| Message ID | 20230807190736.572665-3-david@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | softmmu/physmem: file_ram_open() readonly improvements | expand |
diff --git a/softmmu/physmem.c b/softmmu/physmem.c index d1ae694b20..9580567608 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -1304,6 +1304,13 @@ static int file_ram_open(const char *path, break; } if (errno == ENOENT) { + if (readonly) { + /* + * O_RDONLY would later make ftruncate() fail, leading to + * SIGBUS after mmap(). + */ + return -ENOENT; + } /* @path names a file that doesn't exist, create it */ fd = open(path, O_RDWR | O_CREAT | O_EXCL, 0644); if (fd >= 0) {
Currently, if a file does not exist yet, file_ram_open() will create new empty file and open it writable. However, it even does that when readonly=true was specified. So instead of opening the file read-only, we'll open it writable, implying that later fallocate() or ftruncate() could succeed. Specifying O_RDONLY instead would theoretically work, however, ftruncate() will refuse to resize the new empty file and we'll get a warning: ftruncate: Invalid argument mmap() will succeed, but any later access to that memory would fail with SIGBUS. Undesirable. If someone intends to let QEMU open+mmap a file read-only, better create+resize+fill that file ahead of time outside of QEMU context. We'll now fail with: ./qemu-system-x86_64 \ -object memory-backend-file,id=ram0,mem-path=tmp/ls,readonly=true,size=1g qemu-system-x86_64: can't open backing store tmp/ls for guest RAM: No such file or directory It's unlikely that this will harm existing users: especially R/O NVDIMMs better expose some reasonable data that already exists. Everything else would just hide user errors when accidentally specifying a non-existent file. Note that the only memory-backend-file will end up calling memory_region_init_ram_from_file() -> qemu_ram_alloc_from_file() -> file_ram_open(). Signed-off-by: David Hildenbrand <david@redhat.com> --- softmmu/physmem.c | 7 +++++++ 1 file changed, 7 insertions(+)