From c7e587b73ebac05943df78f5f37d80d32ff47d3d Mon Sep 17 00:00:00 2001 From: "Aneesh Kumar K.V" Date: Mon, 20 May 2013 11:29:52 +0530 Subject: [PATCH 1/4] hw/9pfs: Fix segfault with 9p2000.u When guest tries to chmod a block or char device file over 9pfs, the qemu process segfaults. With 9p2000.u protocol we use wstat to change mode bits and client don't send extension information for chmod. We need to check for size field to check whether extension info is present or not. Reported-by: Michael Tokarev Acked-by: Michael Tokarev Reviewed-by: Stefan Hajnoczi Signed-off-by: Aneesh Kumar K.V --- hw/9pfs/virtio-9p.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c index 296f66f293..8cbb8ae32a 100644 --- a/hw/9pfs/virtio-9p.c +++ b/hw/9pfs/virtio-9p.c @@ -658,7 +658,7 @@ static mode_t v9mode_to_mode(uint32_t mode, V9fsString *extension) ret |= S_IFIFO; } if (mode & P9_STAT_MODE_DEVICE) { - if (extension && extension->data[0] == 'c') { + if (extension->size && extension->data[0] == 'c') { ret |= S_IFCHR; } else { ret |= S_IFBLK; From aed858ce10ef09c7bdf03f73e75e772f567c74cd Mon Sep 17 00:00:00 2001 From: "Aneesh Kumar K.V" Date: Mon, 20 May 2013 17:28:29 +0530 Subject: [PATCH 2/4] hw/9pfs: use O_NOFOLLOW for mapped readlink operation With mapped security models like mapped-xattr and mapped-file, we save the symlink target as file contents. Now if we ever expose a normal directory with mapped security model and find real symlinks in export path, never follow them and return proper error. Reviewed-by: Stefan Hajnoczi Signed-off-by: Aneesh Kumar K.V --- hw/9pfs/virtio-9p-local.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c index 6ece6f7d1c..87aa75d923 100644 --- a/hw/9pfs/virtio-9p-local.c +++ b/hw/9pfs/virtio-9p-local.c @@ -284,7 +284,7 @@ static ssize_t local_readlink(FsContext *fs_ctx, V9fsPath *fs_path, if ((fs_ctx->export_flags & V9FS_SM_MAPPED) || (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE)) { int fd; - fd = open(rpath(fs_ctx, path, buffer), O_RDONLY); + fd = open(rpath(fs_ctx, path, buffer), O_RDONLY | O_NOFOLLOW); if (fd == -1) { return -1; } From 0ceb092e35f9d8793d64d86194ec9463718dffa3 Mon Sep 17 00:00:00 2001 From: "Aneesh Kumar K.V" Date: Mon, 20 May 2013 19:43:15 +0530 Subject: [PATCH 3/4] hw/9pfs: Use O_NOFOLLOW when opening files on server 9p server should never follow a symlink. So use O_NOFOLLOW with all open syscall Tested-by: "M. Mohan Kumar" Signed-off-by: Aneesh Kumar K.V --- hw/9pfs/virtio-9p-local.c | 48 +++++++++++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 7 deletions(-) diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c index 87aa75d923..fc93e9e6e8 100644 --- a/hw/9pfs/virtio-9p-local.c +++ b/hw/9pfs/virtio-9p-local.c @@ -59,6 +59,33 @@ static const char *local_mapped_attr_path(FsContext *ctx, return buffer; } +static FILE *local_fopen(const char *path, const char *mode) +{ + int fd, o_mode = 0; + FILE *fp; + int flags = O_NOFOLLOW; + /* + * only supports two modes + */ + if (mode[0] == 'r') { + flags |= O_RDONLY; + } else if (mode[0] == 'w') { + flags |= O_WRONLY | O_TRUNC | O_CREAT; + o_mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH; + } else { + return NULL; + } + fd = open(path, flags, o_mode); + if (fd == -1) { + return NULL; + } + fp = fdopen(fd, mode); + if (!fp) { + close(fd); + } + return fp; +} + #define ATTR_MAX 100 static void local_mapped_file_attr(FsContext *ctx, const char *path, struct stat *stbuf) @@ -68,7 +95,7 @@ static void local_mapped_file_attr(FsContext *ctx, const char *path, char attr_path[PATH_MAX]; local_mapped_attr_path(ctx, path, attr_path); - fp = fopen(attr_path, "r"); + fp = local_fopen(attr_path, "r"); if (!fp) { return; } @@ -152,7 +179,7 @@ static int local_set_mapped_file_attr(FsContext *ctx, char attr_path[PATH_MAX]; int uid = -1, gid = -1, mode = -1, rdev = -1; - fp = fopen(local_mapped_attr_path(ctx, path, attr_path), "r"); + fp = local_fopen(local_mapped_attr_path(ctx, path, attr_path), "r"); if (!fp) { goto create_map_file; } @@ -179,7 +206,7 @@ create_map_file: } update_map_file: - fp = fopen(attr_path, "w"); + fp = local_fopen(attr_path, "w"); if (!fp) { ret = -1; goto err_out; @@ -316,7 +343,7 @@ static int local_open(FsContext *ctx, V9fsPath *fs_path, char buffer[PATH_MAX]; char *path = fs_path->data; - fs->fd = open(rpath(ctx, path, buffer), flags); + fs->fd = open(rpath(ctx, path, buffer), flags | O_NOFOLLOW); return fs->fd; } @@ -601,6 +628,11 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, V9fsString fullname; char buffer[PATH_MAX]; + /* + * Mark all the open to not follow symlinks + */ + flags |= O_NOFOLLOW; + v9fs_string_init(&fullname); v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name); path = fullname.data; @@ -676,8 +708,9 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, if (fs_ctx->export_flags & V9FS_SM_MAPPED) { int fd; ssize_t oldpath_size, write_size; - fd = open(rpath(fs_ctx, newpath, buffer), O_CREAT|O_EXCL|O_RDWR, - SM_LOCAL_MODE_BITS); + fd = open(rpath(fs_ctx, newpath, buffer), + O_CREAT|O_EXCL|O_RDWR|O_NOFOLLOW, + SM_LOCAL_MODE_BITS); if (fd == -1) { err = fd; goto out; @@ -705,7 +738,8 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, } else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) { int fd; ssize_t oldpath_size, write_size; - fd = open(rpath(fs_ctx, newpath, buffer), O_CREAT|O_EXCL|O_RDWR, + fd = open(rpath(fs_ctx, newpath, buffer), + O_CREAT|O_EXCL|O_RDWR|O_NOFOLLOW, SM_LOCAL_MODE_BITS); if (fd == -1) { err = fd; From db431f6adc881a0758512cd765b3108209013512 Mon Sep 17 00:00:00 2001 From: Gabriel de Perthuis Date: Fri, 10 May 2013 19:53:28 +0200 Subject: [PATCH 4/4] hw/9pfs: Be robust against paths without FS_IOC_GETVERSION 9P optionally uses the FS_IOC_GETVERSION ioctl to get information about a file's version (sometimes called generation number). The code checks for supported filesystems at mount time, but some paths may come from other mounted filesystems. Change it to treat unsupported paths the same as unsupported filesystems, returning 0 in both cases. Note: ENOTTY is the error code for an unsupported ioctl. This fix allows booting a linux kernel with the same / filesystem as the host; otherwise the boot fails when mounting devtmpfs. Signed-off-by: Gabriel de Perthuis Reviewed-by: Aneesh Kumar K.V Signed-off-by: Aneesh Kumar K.V --- hw/9pfs/cofile.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c index 2efebf3571..194c1306c6 100644 --- a/hw/9pfs/cofile.c +++ b/hw/9pfs/cofile.c @@ -38,6 +38,10 @@ int v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t st_mode, }); v9fs_path_unlock(s); } + /* The ioctl may not be supported depending on the path */ + if (err == -ENOTTY) { + err = 0; + } return err; }