diff mbox

[1/1] Add support for custom fmasks/dmasks in 9ps mapped mode

Message ID 20170618082813.8091-2-tobleminer@gmail.com
State New
Headers show

Commit Message

Tobias Schramm June 18, 2017, 8:28 a.m. UTC 
Signed-off-by: Tobias Schramm <tobleminer@gmail.com>
---
 fsdev/file-op-9p.h      |  4 ++++
 fsdev/qemu-fsdev-opts.c | 12 ++++++++++++
 hw/9pfs/9p-local.c      | 33 +++++++++++++++++++++++++++++----
 hw/9pfs/9p.c            |  3 +++
 4 files changed, 48 insertions(+), 4 deletions(-)

Comments

Manos Pitsidianakis June 18, 2017, 1:57 p.m. UTC | #1 
On Sun, Jun 18, 2017 at 10:28:13AM +0200, Tobias Schramm wrote:
>@@ -1469,6 +1472,28 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
>
>     fse->path = g_strdup(path);
>
>+    fse->fmask = SM_LOCAL_MODE_BITS;
>+    if (fmask) {
>+        mask = strtol(fmask, NULL, 0);

(Use qemu_strtol(), or maybe parse_uint() since it has to be positive)

If a mode without the '0' prefix is supplied (as required by strtol to 
parse an octal), the input will be parsed as a decimal and will result 
in a wrong value. Also, maybe you should check for mask > 0777 and for 
negative input as well instead of &ing.


>+        if((!mask || mask == LONG_MIN || mask == LONG_MAX) && errno)
>+        {
>+            error_report("Invalid fmask %s specified", fmask);
>+            return -1;
>+	}
>+        fse->fmask = ((mode_t)mask) & 0777;
>+    }
>+
>+    fse->dmask = SM_LOCAL_DIR_MODE_BITS;
>+    if (dmask) {
>+        mask = strtol(dmask, NULL, 0);
>+        if((!mask || mask == LONG_MIN || mask == LONG_MAX) && errno)
>+        {
>+            error_report("Invalid dmask %s specified", dmask);
>+            return -1;
>+	}
>+        fse->dmask = ((mode_t)mask) & 0777;

Same here.

>+    }
>+
>     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
>
>
>
Eric Blake June 27, 2017, 8:01 p.m. UTC | #2 
On 06/18/2017 03:28 AM, Tobias Schramm wrote:
> Signed-off-by: Tobias Schramm <tobleminer@gmail.com>
> ---
>  fsdev/file-op-9p.h      |  4 ++++
>  fsdev/qemu-fsdev-opts.c | 12 ++++++++++++
>  hw/9pfs/9p-local.c      | 33 +++++++++++++++++++++++++++++----
>  hw/9pfs/9p.c            |  3 +++
>  4 files changed, 48 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;
>  

> +++ 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,

No documentation of what these represent (other than the cover letter).
I'd at least expect a comment of 'default creation mask for files' and
'default creation mask for directories'; it would also be nice to state
if the mask is positive or negative logic (compare the third argument
passed to open() which specifies bits to set, vs. the argument to umask
which specifies bits to keep clear).

Also, does this need a QAPI counterpart (or can you even create a 9pfs
device through QMP)?
diff mbox

Patch

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..5c312298e8 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");
+    long mask;
     Error *err = NULL;
 
     if (!sec_model) {
@@ -1469,6 +1472,28 @@  static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
 
     fse->path = g_strdup(path);
 
+    fse->fmask = SM_LOCAL_MODE_BITS;
+    if (fmask) {
+        mask = strtol(fmask, NULL, 0);
+        if((!mask || mask == LONG_MIN || mask == LONG_MAX) && errno)
+        {
+            error_report("Invalid fmask %s specified", fmask);
+            return -1;
+	}
+        fse->fmask = ((mode_t)mask) & 0777;
+    }
+
+    fse->dmask = SM_LOCAL_DIR_MODE_BITS;
+    if (dmask) {
+        mask = strtol(dmask, NULL, 0);
+        if((!mask || mask == LONG_MIN || mask == LONG_MAX) && errno)
+        {
+            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);