* [Qemu-devel] [PATCH v3 0/1] Add fmask/dmask option for 9p mapped mode @ 2017-06-18 16:17 Tobias Schramm 2017-06-18 16:17 ` [Qemu-devel] [PATCH v3 1/1] Add support for custom fmasks/dmasks in 9ps " Tobias Schramm 0 siblings, 1 reply; 3+ messages in thread From: Tobias Schramm @ 2017-06-18 16:17 UTC (permalink / raw) To: qemu-devel; +Cc: aneesh.kumar, groug, el13635, Tobias Schramm Hi, as per Manos suggestion I'm now parsing the fmask/dmask as unsigned rather than signed longs. Tobias Schramm Tobias Schramm (1): Add support for custom fmasks/dmasks in 9ps mapped mode fsdev/file-op-9p.h | 4 ++++ fsdev/qemu-fsdev-opts.c | 12 ++++++++++++ hw/9pfs/9p-local.c | 29 +++++++++++++++++++++++++---- hw/9pfs/9p.c | 3 +++ 4 files changed, 44 insertions(+), 4 deletions(-) -- 2.13.1 ^ permalink raw reply [flat|nested] 3+ messages in thread
* [Qemu-devel] [PATCH v3 1/1] Add support for custom fmasks/dmasks in 9ps mapped mode 2017-06-18 16:17 [Qemu-devel] [PATCH v3 0/1] Add fmask/dmask option for 9p mapped mode Tobias Schramm @ 2017-06-18 16:17 ` Tobias Schramm 2017-06-19 9:22 ` Greg Kurz 0 siblings, 1 reply; 3+ messages in thread From: Tobias Schramm @ 2017-06-18 16:17 UTC (permalink / raw) To: qemu-devel; +Cc: aneesh.kumar, groug, el13635, Tobias Schramm Signed-off-by: Tobias Schramm <tobleminer@gmail.com> --- v3: Use unsigned types for umask v2: Adjust patch to QEMU code style fsdev/file-op-9p.h | 4 ++++ fsdev/qemu-fsdev-opts.c | 12 ++++++++++++ hw/9pfs/9p-local.c | 29 +++++++++++++++++++++++++---- hw/9pfs/9p.c | 3 +++ 4 files changed, 44 insertions(+), 4 deletions(-) diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h index 0844a403dc..0ecb1d392b 100644 --- a/fsdev/file-op-9p.h +++ b/fsdev/file-op-9p.h @@ -76,6 +76,8 @@ typedef struct FsDriverEntry { int export_flags; FileOperations *ops; FsThrottle fst; + mode_t fmask; + mode_t dmask; } FsDriverEntry; typedef struct FsContext @@ -88,6 +90,8 @@ typedef struct FsContext FsThrottle *fst; /* fs driver specific data */ void *private; + mode_t fmask; + mode_t dmask; } FsContext; typedef struct V9fsPath { diff --git a/fsdev/qemu-fsdev-opts.c b/fsdev/qemu-fsdev-opts.c index bf5713008a..dd6391edb0 100644 --- a/fsdev/qemu-fsdev-opts.c +++ b/fsdev/qemu-fsdev-opts.c @@ -38,6 +38,12 @@ static QemuOptsList qemu_fsdev_opts = { }, { .name = "sock_fd", .type = QEMU_OPT_NUMBER, + }, { + .name = "fmask", + .type = QEMU_OPT_STRING, + }, { + .name = "dmask", + .type = QEMU_OPT_STRING, }, THROTTLE_OPTS, @@ -75,6 +81,12 @@ static QemuOptsList qemu_virtfs_opts = { }, { .name = "sock_fd", .type = QEMU_OPT_NUMBER, + }, { + .name = "fmask", + .type = QEMU_OPT_STRING, + }, { + .name = "dmask", + .type = QEMU_OPT_STRING, }, { /*End of list */ } diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c index 1e78b7c9e9..ef7282e294 100644 --- a/hw/9pfs/9p-local.c +++ b/hw/9pfs/9p-local.c @@ -633,7 +633,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, if (fs_ctx->export_flags & V9FS_SM_MAPPED || fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) { - err = mknodat(dirfd, name, SM_LOCAL_MODE_BITS | S_IFREG, 0); + err = mknodat(dirfd, name, fs_ctx->fmask | S_IFREG, 0); if (err == -1) { goto out; } @@ -685,7 +685,7 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path, if (fs_ctx->export_flags & V9FS_SM_MAPPED || fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) { - err = mkdirat(dirfd, name, SM_LOCAL_DIR_MODE_BITS); + err = mkdirat(dirfd, name, fs_ctx->dmask); if (err == -1) { goto out; } @@ -786,7 +786,7 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, /* Determine the security model */ if (fs_ctx->export_flags & V9FS_SM_MAPPED || fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) { - fd = openat_file(dirfd, name, flags, SM_LOCAL_MODE_BITS); + fd = openat_file(dirfd, name, flags, fs_ctx->fmask); if (fd == -1) { goto out; } @@ -849,7 +849,7 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, ssize_t oldpath_size, write_size; fd = openat_file(dirfd, name, O_CREAT | O_EXCL | O_RDWR, - SM_LOCAL_MODE_BITS); + fs_ctx->fmask); if (fd == -1) { goto out; } @@ -1431,6 +1431,9 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse) { const char *sec_model = qemu_opt_get(opts, "security_model"); const char *path = qemu_opt_get(opts, "path"); + const char *fmask = qemu_opt_get(opts, "fmask"); + const char *dmask = qemu_opt_get(opts, "dmask"); + unsigned long mask; Error *err = NULL; if (!sec_model) { @@ -1469,6 +1472,24 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse) fse->path = g_strdup(path); + fse->fmask = SM_LOCAL_MODE_BITS; + if (fmask) { + if (qemu_strtoul(fmask, NULL, 0, &mask)) { + error_report("Invalid fmask %s specified", fmask); + return -1; + } + fse->fmask = ((mode_t)mask) & 0777; + } + + fse->dmask = SM_LOCAL_DIR_MODE_BITS; + if (dmask) { + if (qemu_strtoul(dmask, NULL, 0, &mask)) { + error_report("Invalid dmask %s specified", dmask); + return -1; + } + fse->dmask = ((mode_t)mask) & 0777; + } + return 0; } diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index 96d2683348..40290dbade 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -3533,6 +3533,9 @@ int v9fs_device_realize_common(V9fsState *s, Error **errp) s->ops = fse->ops; + s->ctx.fmask = fse->fmask; + s->ctx.dmask = fse->dmask; + s->fid_list = NULL; qemu_co_rwlock_init(&s->rename_lock); -- 2.13.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/1] Add support for custom fmasks/dmasks in 9ps mapped mode 2017-06-18 16:17 ` [Qemu-devel] [PATCH v3 1/1] Add support for custom fmasks/dmasks in 9ps " Tobias Schramm @ 2017-06-19 9:22 ` Greg Kurz 0 siblings, 0 replies; 3+ messages in thread From: Greg Kurz @ 2017-06-19 9:22 UTC (permalink / raw) To: Tobias Schramm; +Cc: qemu-devel, aneesh.kumar, el13635 [-- Attachment #1: Type: text/plain, Size: 6168 bytes --] On Sun, 18 Jun 2017 18:17:27 +0200 Tobias Schramm <tobleminer@gmail.com> wrote: Hi, The patch titles are usually prefixed by the component name. Something like: 9pfs: local: add support for custom fmask/dmask in mapped security modes Also, it is always nice to write a changelog, even if the feature looks simple. At least, explain why we need that. > Signed-off-by: Tobias Schramm <tobleminer@gmail.com> > --- > v3: Use unsigned types for umask > v2: Adjust patch to QEMU code style > > fsdev/file-op-9p.h | 4 ++++ > fsdev/qemu-fsdev-opts.c | 12 ++++++++++++ > hw/9pfs/9p-local.c | 29 +++++++++++++++++++++++++---- > hw/9pfs/9p.c | 3 +++ This patch changes the command line. Please update the documentation accordingly (qemu-options.hx). > 4 files changed, 44 insertions(+), 4 deletions(-) > > diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h > index 0844a403dc..0ecb1d392b 100644 > --- a/fsdev/file-op-9p.h > +++ b/fsdev/file-op-9p.h > @@ -76,6 +76,8 @@ typedef struct FsDriverEntry { > int export_flags; > FileOperations *ops; > FsThrottle fst; > + mode_t fmask; > + mode_t dmask; > } FsDriverEntry; > > typedef struct FsContext > @@ -88,6 +90,8 @@ typedef struct FsContext > FsThrottle *fst; > /* fs driver specific data */ > void *private; > + mode_t fmask; > + mode_t dmask; > } FsContext; > > typedef struct V9fsPath { > diff --git a/fsdev/qemu-fsdev-opts.c b/fsdev/qemu-fsdev-opts.c > index bf5713008a..dd6391edb0 100644 > --- a/fsdev/qemu-fsdev-opts.c > +++ b/fsdev/qemu-fsdev-opts.c > @@ -38,6 +38,12 @@ static QemuOptsList qemu_fsdev_opts = { > }, { > .name = "sock_fd", > .type = QEMU_OPT_NUMBER, > + }, { > + .name = "fmask", > + .type = QEMU_OPT_STRING, Why not using QEMU_OPT_NUMBER ? > + }, { > + .name = "dmask", > + .type = QEMU_OPT_STRING, > }, > > THROTTLE_OPTS, > @@ -75,6 +81,12 @@ static QemuOptsList qemu_virtfs_opts = { > }, { > .name = "sock_fd", > .type = QEMU_OPT_NUMBER, > + }, { > + .name = "fmask", > + .type = QEMU_OPT_STRING, > + }, { > + .name = "dmask", > + .type = QEMU_OPT_STRING, > }, > > { /*End of list */ } > diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c > index 1e78b7c9e9..ef7282e294 100644 > --- a/hw/9pfs/9p-local.c > +++ b/hw/9pfs/9p-local.c > @@ -633,7 +633,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, > > if (fs_ctx->export_flags & V9FS_SM_MAPPED || > fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) { > - err = mknodat(dirfd, name, SM_LOCAL_MODE_BITS | S_IFREG, 0); > + err = mknodat(dirfd, name, fs_ctx->fmask | S_IFREG, 0); > if (err == -1) { > goto out; > } > @@ -685,7 +685,7 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path, > > if (fs_ctx->export_flags & V9FS_SM_MAPPED || > fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) { > - err = mkdirat(dirfd, name, SM_LOCAL_DIR_MODE_BITS); > + err = mkdirat(dirfd, name, fs_ctx->dmask); > if (err == -1) { > goto out; > } > @@ -786,7 +786,7 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, > /* Determine the security model */ > if (fs_ctx->export_flags & V9FS_SM_MAPPED || > fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) { > - fd = openat_file(dirfd, name, flags, SM_LOCAL_MODE_BITS); > + fd = openat_file(dirfd, name, flags, fs_ctx->fmask); > if (fd == -1) { > goto out; > } > @@ -849,7 +849,7 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, > ssize_t oldpath_size, write_size; > > fd = openat_file(dirfd, name, O_CREAT | O_EXCL | O_RDWR, > - SM_LOCAL_MODE_BITS); > + fs_ctx->fmask); > if (fd == -1) { > goto out; > } > @@ -1431,6 +1431,9 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse) > { > const char *sec_model = qemu_opt_get(opts, "security_model"); > const char *path = qemu_opt_get(opts, "path"); > + const char *fmask = qemu_opt_get(opts, "fmask"); > + const char *dmask = qemu_opt_get(opts, "dmask"); > + unsigned long mask; > Error *err = NULL; > > if (!sec_model) { > @@ -1469,6 +1472,24 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse) > > fse->path = g_strdup(path); > > + fse->fmask = SM_LOCAL_MODE_BITS; > + if (fmask) { > + if (qemu_strtoul(fmask, NULL, 0, &mask)) { > + error_report("Invalid fmask %s specified", fmask); > + return -1; If the user passes an invalid fmask, then fse->path is leaked. Sanity checks should happen before, or you have to free allocated resources. In the present case, I guess the g_strdup() line should be the last thing this function does. > + } > + fse->fmask = ((mode_t)mask) & 0777; > + } > + > + fse->dmask = SM_LOCAL_DIR_MODE_BITS; > + if (dmask) { > + if (qemu_strtoul(dmask, NULL, 0, &mask)) { > + error_report("Invalid dmask %s specified", dmask); > + return -1; > + } > + fse->dmask = ((mode_t)mask) & 0777; > + } > + All of this is for mapped modes only. It should fail and print an error when used with other modes. > return 0; > } > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > index 96d2683348..40290dbade 100644 > --- a/hw/9pfs/9p.c > +++ b/hw/9pfs/9p.c > @@ -3533,6 +3533,9 @@ int v9fs_device_realize_common(V9fsState *s, Error **errp) > > s->ops = fse->ops; > > + s->ctx.fmask = fse->fmask; > + s->ctx.dmask = fse->dmask; > + > s->fid_list = NULL; > qemu_co_rwlock_init(&s->rename_lock); > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-06-19 9:22 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-06-18 16:17 [Qemu-devel] [PATCH v3 0/1] Add fmask/dmask option for 9p mapped mode Tobias Schramm 2017-06-18 16:17 ` [Qemu-devel] [PATCH v3 1/1] Add support for custom fmasks/dmasks in 9ps " Tobias Schramm 2017-06-19 9:22 ` Greg Kurz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).