From fde1f3e4a0644feb005894950427b622496769cb Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 7 Jun 2018 12:17:21 +0200 Subject: 9p: proxy: Fix size passed to `connect` The size to pass to the `connect` call is the size of the entire `struct sockaddr_un`. Passing anything shorter than this causes errors on darwin. Signed-off-by: Keno Fischer Signed-off-by: Greg Kurz --- hw/9pfs/9p-proxy.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c index e2e03292de..47a94e088d 100644 --- a/hw/9pfs/9p-proxy.c +++ b/hw/9pfs/9p-proxy.c @@ -1088,7 +1088,7 @@ static int proxy_ioc_getversion(FsContext *fs_ctx, V9fsPath *path, static int connect_namedsocket(const char *path, Error **errp) { - int sockfd, size; + int sockfd; struct sockaddr_un helper; if (strlen(path) >= sizeof(helper.sun_path)) { @@ -1102,8 +1102,7 @@ static int connect_namedsocket(const char *path, Error **errp) } strcpy(helper.sun_path, path); helper.sun_family = AF_UNIX; - size = strlen(helper.sun_path) + sizeof(helper.sun_family); - if (connect(sockfd, (struct sockaddr *)&helper, size) < 0) { + if (connect(sockfd, (struct sockaddr *)&helper, sizeof(helper)) < 0) { error_setg_errno(errp, errno, "failed to connect to '%s'", path); close(sockfd); return -1; -- cgit v1.2.3 From 2306271c381b75b66d76c4231bba3e7452a0d876 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 7 Jun 2018 12:17:21 +0200 Subject: 9p: local: Properly set errp in fstatfs error path In the review of 9p: Avoid warning if FS_IOC_GETVERSION is not defined Grep Kurz noted this error path was failing to set errp. Fix that. Signed-off-by: Keno Fischer [added local: to commit title, Greg Kurz] Signed-off-by: Greg Kurz --- hw/9pfs/9p-local.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c index b37b1db453..7758c38509 100644 --- a/hw/9pfs/9p-local.c +++ b/hw/9pfs/9p-local.c @@ -1418,6 +1418,8 @@ static int local_init(FsContext *ctx, Error **errp) */ if (fstatfs(data->mountfd, &stbuf) < 0) { close_preserve_errno(data->mountfd); + error_setg_errno(errp, errno, + "failed to stat file system at '%s'", ctx->fs_root); goto err; } switch (stbuf.f_type) { -- cgit v1.2.3 From ec70b956fddf628ee2e42521c54362e80115a3c4 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 7 Jun 2018 12:17:22 +0200 Subject: 9p: Move a couple xattr functions to 9p-util These functions will need custom implementations on Darwin. Since the implementation is very similar among all of them, and 9p-util already has the _nofollow version of fgetxattrat, let's move them all there. Signed-off-by: Keno Fischer Signed-off-by: Greg Kurz --- hw/9pfs/9p-util.c | 33 +++++++++++++++++++++++++++++++++ hw/9pfs/9p-util.h | 4 ++++ hw/9pfs/9p-xattr.c | 33 --------------------------------- 3 files changed, 37 insertions(+), 33 deletions(-) diff --git a/hw/9pfs/9p-util.c b/hw/9pfs/9p-util.c index f709c27a1f..614b7fc34d 100644 --- a/hw/9pfs/9p-util.c +++ b/hw/9pfs/9p-util.c @@ -24,3 +24,36 @@ ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char *name, g_free(proc_path); return ret; } + +ssize_t flistxattrat_nofollow(int dirfd, const char *filename, + char *list, size_t size) +{ + char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename); + int ret; + + ret = llistxattr(proc_path, list, size); + g_free(proc_path); + return ret; +} + +ssize_t fremovexattrat_nofollow(int dirfd, const char *filename, + const char *name) +{ + char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename); + int ret; + + ret = lremovexattr(proc_path, name); + g_free(proc_path); + return ret; +} + +int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name, + void *value, size_t size, int flags) +{ + char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename); + int ret; + + ret = lsetxattr(proc_path, name, value, size, flags); + g_free(proc_path); + return ret; +} diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h index dc0d2e29aa..79ed6b233e 100644 --- a/hw/9pfs/9p-util.h +++ b/hw/9pfs/9p-util.h @@ -60,5 +60,9 @@ ssize_t fgetxattrat_nofollow(int dirfd, const char *path, const char *name, void *value, size_t size); int fsetxattrat_nofollow(int dirfd, const char *path, const char *name, void *value, size_t size, int flags); +ssize_t flistxattrat_nofollow(int dirfd, const char *filename, + char *list, size_t size); +ssize_t fremovexattrat_nofollow(int dirfd, const char *filename, + const char *name); #endif diff --git a/hw/9pfs/9p-xattr.c b/hw/9pfs/9p-xattr.c index d05c1a1c1d..c696d8f846 100644 --- a/hw/9pfs/9p-xattr.c +++ b/hw/9pfs/9p-xattr.c @@ -60,17 +60,6 @@ ssize_t pt_listxattr(FsContext *ctx, const char *path, return name_size; } -static ssize_t flistxattrat_nofollow(int dirfd, const char *filename, - char *list, size_t size) -{ - char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename); - int ret; - - ret = llistxattr(proc_path, list, size); - g_free(proc_path); - return ret; -} - /* * Get the list and pass to each layer to find out whether * to send the data or not @@ -196,17 +185,6 @@ ssize_t pt_getxattr(FsContext *ctx, const char *path, const char *name, return local_getxattr_nofollow(ctx, path, name, value, size); } -int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name, - void *value, size_t size, int flags) -{ - char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename); - int ret; - - ret = lsetxattr(proc_path, name, value, size, flags); - g_free(proc_path); - return ret; -} - ssize_t local_setxattr_nofollow(FsContext *ctx, const char *path, const char *name, void *value, size_t size, int flags) @@ -235,17 +213,6 @@ int pt_setxattr(FsContext *ctx, const char *path, const char *name, void *value, return local_setxattr_nofollow(ctx, path, name, value, size, flags); } -static ssize_t fremovexattrat_nofollow(int dirfd, const char *filename, - const char *name) -{ - char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename); - int ret; - - ret = lremovexattr(proc_path, name); - g_free(proc_path); - return ret; -} - ssize_t local_removexattr_nofollow(FsContext *ctx, const char *path, const char *name) { -- cgit v1.2.3 From a647502c582981c395b5d16e52a22ac7aff0fb2b Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 7 Jun 2018 12:17:22 +0200 Subject: 9p: xattr: Fix crashes due to free of uninitialized value If the size returned from llistxattr/lgetxattr is 0, we skipped the malloc call, leaving xattr.value uninitialized. However, this value is later passed to `g_free` without any further checks, causing an error. Fix that by always calling g_malloc unconditionally. If `size` is 0, it will return NULL, which is safe to pass to g_free. Signed-off-by: Keno Fischer Signed-off-by: Greg Kurz --- hw/9pfs/9p.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index d74302deeb..4386d69817 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -3256,8 +3256,8 @@ static void coroutine_fn v9fs_xattrwalk(void *opaque) xattr_fidp->fs.xattr.len = size; xattr_fidp->fid_type = P9_FID_XATTR; xattr_fidp->fs.xattr.xattrwalk_fid = true; + xattr_fidp->fs.xattr.value = g_malloc0(size); if (size) { - xattr_fidp->fs.xattr.value = g_malloc0(size); err = v9fs_co_llistxattr(pdu, &xattr_fidp->path, xattr_fidp->fs.xattr.value, xattr_fidp->fs.xattr.len); @@ -3289,8 +3289,8 @@ static void coroutine_fn v9fs_xattrwalk(void *opaque) xattr_fidp->fs.xattr.len = size; xattr_fidp->fid_type = P9_FID_XATTR; xattr_fidp->fs.xattr.xattrwalk_fid = true; + xattr_fidp->fs.xattr.value = g_malloc0(size); if (size) { - xattr_fidp->fs.xattr.value = g_malloc0(size); err = v9fs_co_lgetxattr(pdu, &xattr_fidp->path, &name, xattr_fidp->fs.xattr.value, xattr_fidp->fs.xattr.len); -- cgit v1.2.3 From 5b7b2f9a85bcc44485da713f60e371dd66a644b1 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 7 Jun 2018 12:17:22 +0200 Subject: 9p: local: Avoid warning if FS_IOC_GETVERSION is not defined Both `stbuf` and `local_ioc_getversion` where unused when FS_IOC_GETVERSION was not defined, causing a compiler warning. Reorganize the code to avoid this warning. Signed-off-by: Keno Fischer Signed-off-by: Greg Kurz --- hw/9pfs/9p-local.c | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c index 7758c38509..5721eff1e1 100644 --- a/hw/9pfs/9p-local.c +++ b/hw/9pfs/9p-local.c @@ -1373,10 +1373,10 @@ static int local_unlinkat(FsContext *ctx, V9fsPath *dir, return ret; } +#ifdef FS_IOC_GETVERSION static int local_ioc_getversion(FsContext *ctx, V9fsPath *path, mode_t st_mode, uint64_t *st_gen) { -#ifdef FS_IOC_GETVERSION int err; V9fsFidOpenState fid_open; @@ -1395,32 +1395,21 @@ static int local_ioc_getversion(FsContext *ctx, V9fsPath *path, err = ioctl(fid_open.fd, FS_IOC_GETVERSION, st_gen); local_close(ctx, &fid_open); return err; -#else - errno = ENOTTY; - return -1; -#endif } +#endif -static int local_init(FsContext *ctx, Error **errp) +static int local_ioc_getversion_init(FsContext *ctx, LocalData *data, Error **errp) { +#ifdef FS_IOC_GETVERSION struct statfs stbuf; - LocalData *data = g_malloc(sizeof(*data)); - data->mountfd = open(ctx->fs_root, O_DIRECTORY | O_RDONLY); - if (data->mountfd == -1) { - error_setg_errno(errp, errno, "failed to open '%s'", ctx->fs_root); - goto err; - } - -#ifdef FS_IOC_GETVERSION /* * use ioc_getversion only if the ioctl is definied */ if (fstatfs(data->mountfd, &stbuf) < 0) { - close_preserve_errno(data->mountfd); error_setg_errno(errp, errno, - "failed to stat file system at '%s'", ctx->fs_root); - goto err; + "failed to stat file system at '%s'", ctx->fs_root); + return -1; } switch (stbuf.f_type) { case EXT2_SUPER_MAGIC: @@ -1431,6 +1420,23 @@ static int local_init(FsContext *ctx, Error **errp) break; } #endif + return 0; +} + +static int local_init(FsContext *ctx, Error **errp) +{ + LocalData *data = g_malloc(sizeof(*data)); + + data->mountfd = open(ctx->fs_root, O_DIRECTORY | O_RDONLY); + if (data->mountfd == -1) { + error_setg_errno(errp, errno, "failed to open '%s'", ctx->fs_root); + goto err; + } + + if (local_ioc_getversion_init(ctx, data, errp) < 0) { + close(data->mountfd); + goto err; + } if (ctx->export_flags & V9FS_SM_PASSTHROUGH) { ctx->xops = passthrough_xattr_ops; -- cgit v1.2.3 From 67e87345744ac96d6c9560827ea094264c88fbff Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 7 Jun 2018 12:17:22 +0200 Subject: 9p: Properly check/translate flags in unlinkat The 9p-local code previously relied on P9_DOTL_AT_REMOVEDIR and AT_REMOVEDIR having the same numerical value and deferred any errorchecking to the syscall itself. However, while the former assumption is true on Linux, it is not true in general. 9p-handle did this properly however. Move the translation code to the generic 9p server code and add an error if unrecognized flags are passed. Signed-off-by: Keno Fischer Signed-off-by: Greg Kurz --- hw/9pfs/9p-handle.c | 8 +------- hw/9pfs/9p.c | 13 +++++++++++-- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/hw/9pfs/9p-handle.c b/hw/9pfs/9p-handle.c index 4dc0d2bed1..f3641dbe4a 100644 --- a/hw/9pfs/9p-handle.c +++ b/hw/9pfs/9p-handle.c @@ -559,19 +559,13 @@ static int handle_unlinkat(FsContext *ctx, V9fsPath *dir, { int dirfd, ret; HandleData *data = (HandleData *) ctx->private; - int rflags; dirfd = open_by_handle(data->mountfd, dir->data, O_PATH); if (dirfd < 0) { return dirfd; } - rflags = 0; - if (flags & P9_DOTL_AT_REMOVEDIR) { - rflags |= AT_REMOVEDIR; - } - - ret = unlinkat(dirfd, name, rflags); + ret = unlinkat(dirfd, name, flags); close(dirfd); return ret; diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index 4386d69817..c842ec555e 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -2522,7 +2522,7 @@ static void coroutine_fn v9fs_unlinkat(void *opaque) { int err = 0; V9fsString name; - int32_t dfid, flags; + int32_t dfid, flags, rflags = 0; size_t offset = 7; V9fsPath path; V9fsFidState *dfidp; @@ -2549,6 +2549,15 @@ static void coroutine_fn v9fs_unlinkat(void *opaque) goto out_nofid; } + if (flags & ~P9_DOTL_AT_REMOVEDIR) { + err = -EINVAL; + goto out_nofid; + } + + if (flags & P9_DOTL_AT_REMOVEDIR) { + rflags |= AT_REMOVEDIR; + } + dfidp = get_fid(pdu, dfid); if (dfidp == NULL) { err = -EINVAL; @@ -2567,7 +2576,7 @@ static void coroutine_fn v9fs_unlinkat(void *opaque) if (err < 0) { goto out_err; } - err = v9fs_co_unlinkat(pdu, &dfidp->path, &name, flags); + err = v9fs_co_unlinkat(pdu, &dfidp->path, &name, rflags); if (!err) { err = offset; } -- cgit v1.2.3 From aca6897fba149a2a650dcdf5a5e1ae828371f4aa Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 7 Jun 2018 12:17:22 +0200 Subject: 9p: xattr: Properly translate xattrcreate flags As with unlinkat, these flags come from the client and need to be translated to their host values. The protocol values happen to match linux, but that need not be true in general. Signed-off-by: Keno Fischer Signed-off-by: Greg Kurz --- hw/9pfs/9p.c | 17 +++++++++++++++-- hw/9pfs/9p.h | 4 ++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index c842ec555e..eef289e394 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -3327,7 +3327,7 @@ out_nofid: static void coroutine_fn v9fs_xattrcreate(void *opaque) { - int flags; + int flags, rflags = 0; int32_t fid; uint64_t size; ssize_t err = 0; @@ -3344,6 +3344,19 @@ static void coroutine_fn v9fs_xattrcreate(void *opaque) } trace_v9fs_xattrcreate(pdu->tag, pdu->id, fid, name.data, size, flags); + if (flags & ~(P9_XATTR_CREATE | P9_XATTR_REPLACE)) { + err = -EINVAL; + goto out_nofid; + } + + if (flags & P9_XATTR_CREATE) { + rflags |= XATTR_CREATE; + } + + if (flags & P9_XATTR_REPLACE) { + rflags |= XATTR_REPLACE; + } + if (size > XATTR_SIZE_MAX) { err = -E2BIG; goto out_nofid; @@ -3365,7 +3378,7 @@ static void coroutine_fn v9fs_xattrcreate(void *opaque) xattr_fidp->fs.xattr.copied_len = 0; xattr_fidp->fs.xattr.xattrwalk_fid = false; xattr_fidp->fs.xattr.len = size; - xattr_fidp->fs.xattr.flags = flags; + xattr_fidp->fs.xattr.flags = rflags; v9fs_string_init(&xattr_fidp->fs.xattr.name); v9fs_string_copy(&xattr_fidp->fs.xattr.name, &name); xattr_fidp->fs.xattr.value = g_malloc0(size); diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h index bad8ee719c..8883761b2c 100644 --- a/hw/9pfs/9p.h +++ b/hw/9pfs/9p.h @@ -169,6 +169,10 @@ typedef struct V9fsConf char *fsdev_id; } V9fsConf; +/* 9p2000.L xattr flags (matches Linux values) */ +#define P9_XATTR_CREATE 1 +#define P9_XATTR_REPLACE 2 + typedef struct V9fsXattr { uint64_t copied_len; -- cgit v1.2.3