[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v2 01/42] exec: Fix memory allocation when memory pa
From: |
Markus Armbruster |
Subject: |
[Qemu-devel] [PATCH v2 01/42] exec: Fix memory allocation when memory path names new file |
Date: |
Mon, 7 Mar 2016 20:25:13 +0100 |
Commit 8d31d6b extended file_ram_alloc() to accept file names in
addition to directory names. Even though it passes O_CREAT to open(),
it actually works only for existing files. Reproducer adapted from
the commit's qemu-doc.texi update:
$ qemu-system-x86_64 -object
memory-backend-file,size=2M,mem-path=/dev/hugepages/my-shmem-file,id=mb1
qemu-system-x86_64: -object
memory-backend-file,size=2M,mem-path=/dev/hugepages/my-shmem-file,id=mb1:
failed to get page size of file /dev/hugepages/my-shmem-file: No such file or
directory
This is because we first get the page size for @path, then open the
actual file. Unwise even before the flawed commit, because the
directory could change in between, invalidating the page size.
Unlikely to bite in practice.
Rearrange the code to create the file (if necessary) before getting
its page size. Carefully avoid TOCTTOU conditions with a method
suggested by Paolo Bonzini.
While there, replace "hugepages" by "guest RAM" in error messages,
because host memory backends can be used for purposes other than huge
pages, e.g. /dev/shm/ shared memory. Help text of -mem-path agrees.
Cc: Paolo Bonzini <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>
---
exec.c | 115 ++++++++++++++++++++++++++++++++++++++---------------------------
1 file changed, 68 insertions(+), 47 deletions(-)
diff --git a/exec.c b/exec.c
index c62c439..5275ff4 100644
--- a/exec.c
+++ b/exec.c
@@ -1212,19 +1212,17 @@ void qemu_mutex_unlock_ramlist(void)
#define HUGETLBFS_MAGIC 0x958458f6
-static long gethugepagesize(const char *path, Error **errp)
+static long gethugepagesize(int fd)
{
struct statfs fs;
int ret;
do {
- ret = statfs(path, &fs);
+ ret = fstatfs(fd, &fs);
} while (ret != 0 && errno == EINTR);
if (ret != 0) {
- error_setg_errno(errp, errno, "failed to get page size of file %s",
- path);
- return 0;
+ return -1;
}
return fs.f_bsize;
@@ -1235,63 +1233,82 @@ static void *file_ram_alloc(RAMBlock *block,
const char *path,
Error **errp)
{
- struct stat st;
+ bool unlink_on_error = false;
char *filename;
char *sanitized_name;
char *c;
void *area;
int fd;
- uint64_t hpagesize;
- Error *local_err = NULL;
+ int64_t hpagesize;
- hpagesize = gethugepagesize(path, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
+ if (kvm_enabled() && !kvm_has_sync_mmu()) {
+ error_setg(errp,
+ "host lacks kvm mmu notifiers, -mem-path unsupported");
+ return NULL;
+ }
+
+ for (;;) {
+ fd = open(path, O_RDWR);
+ if (fd >= 0) {
+ /* @path names an existing file, use it */
+ break;
+ }
+ if (errno == ENOENT) {
+ /* @path names a file that doesn't exist, create it */
+ fd = open(path, O_RDWR | O_CREAT | O_EXCL, 0644);
+ if (fd >= 0) {
+ unlink_on_error = true;
+ break;
+ }
+ } else if (errno == EISDIR) {
+ /* @path names a directory, create a file there */
+ /* Make name safe to use with mkstemp by replacing '/' with '_'. */
+ sanitized_name = g_strdup(memory_region_name(block->mr));
+ for (c = sanitized_name; *c != '\0'; c++) {
+ if (*c == '/') {
+ *c = '_';
+ }
+ }
+
+ filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path,
+ sanitized_name);
+ g_free(sanitized_name);
+
+ fd = mkstemp(filename);
+ if (fd >= 0) {
+ unlink(filename);
+ g_free(filename);
+ break;
+ }
+ g_free(filename);
+ }
+ if (errno != EEXIST && errno != EINTR) {
+ error_setg_errno(errp, errno,
+ "can't open backing store %s for guest RAM",
+ path);
+ goto error;
+ }
+ /*
+ * Try again on EINTR and EEXIST. The latter happens when
+ * something else creates the file between our two open().
+ */
+ }
+
+ hpagesize = gethugepagesize(fd);
+ if (hpagesize < 0) {
+ error_setg_errno(errp, errno, "can't get page size for %s",
+ path);
goto error;
}
block->mr->align = hpagesize;
if (memory < hpagesize) {
error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to "
- "or larger than huge page size 0x%" PRIx64,
+ "or larger than page size 0x%" PRIx64,
memory, hpagesize);
goto error;
}
- if (kvm_enabled() && !kvm_has_sync_mmu()) {
- error_setg(errp,
- "host lacks kvm mmu notifiers, -mem-path unsupported");
- goto error;
- }
-
- if (!stat(path, &st) && S_ISDIR(st.st_mode)) {
- /* Make name safe to use with mkstemp by replacing '/' with '_'. */
- sanitized_name = g_strdup(memory_region_name(block->mr));
- for (c = sanitized_name; *c != '\0'; c++) {
- if (*c == '/') {
- *c = '_';
- }
- }
-
- filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path,
- sanitized_name);
- g_free(sanitized_name);
-
- fd = mkstemp(filename);
- if (fd >= 0) {
- unlink(filename);
- }
- g_free(filename);
- } else {
- fd = open(path, O_RDWR | O_CREAT, 0644);
- }
-
- if (fd < 0) {
- error_setg_errno(errp, errno,
- "unable to create backing store for hugepages");
- goto error;
- }
-
memory = ROUND_UP(memory, hpagesize);
/*
@@ -1307,7 +1324,7 @@ static void *file_ram_alloc(RAMBlock *block,
area = qemu_ram_mmap(fd, memory, hpagesize, block->flags & RAM_SHARED);
if (area == MAP_FAILED) {
error_setg_errno(errp, errno,
- "unable to map backing store for hugepages");
+ "unable to map backing store for guest RAM");
close(fd);
goto error;
}
@@ -1320,6 +1337,10 @@ static void *file_ram_alloc(RAMBlock *block,
return area;
error:
+ if (unlink_on_error) {
+ unlink(path);
+ }
+ close(fd);
return NULL;
}
#endif
--
2.4.3
- [Qemu-devel] [PATCH v2 00/42] ivshmem: Fixes, cleanups, device model split, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 10/42] ivshmem-test: Clean up wait for devices to become operational, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 11/42] ivshmem-test: Improve test cases /ivshmem/server-*, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 04/42] ivshmem-server: Fix and clean up command line help, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 13/42] ivshmem: Add missing newlines to debug printfs, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 01/42] exec: Fix memory allocation when memory path names new file,
Markus Armbruster <=
- [Qemu-devel] [PATCH v2 09/42] ivshmem-test: Improve test case /ivshmem/single, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 14/42] ivshmem: Compile debug prints unconditionally to prevent bit-rot, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 08/42] tests/libqos/pci-pc: Fix qpci_pc_iomap() to map BARs aligned, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 19/42] ivshmem: Failed realize() can leave migration blocker behind, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 02/42] exec: Fix memory allocation when memory path isn't on hugetlbfs, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 21/42] ivshmem: Clean up MSI-X conditions, Markus Armbruster, 2016/03/07