* [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).