Re: [Qemu-devel] [PATCH v5 1/1] 9pfs: local: Add support for custom fmode/dmode in 9ps mapped security modes

[Greg Kurz]

On Tue, 20 Jun 2017 01:37:18 +0200
Tobias Schramm  wrote:

> In mapped security modes, files are created with very restrictive
> permissions (600 for files and 700 for directories). This makes
> file sharing between virtual machines and users on the host rather
> complicated. Imagine eg. a group of users that need to access data
> produced by processes on a virtual machine. Giving those users access
> to the data will be difficult since the group access mode is always 0.
> 
> This patch makes the default mode for both files and directories
> configurable. Existing setups that don't know about the new parameters
> keep using the current secure behavior.
> 
> Signed-off-by: Tobias Schramm 
> ---

Thanks.

Pushed to https://github.com/gkurz/qemu/commits/9p-next


>  v5: Eliminate expandable variables, check mandatory commandline options
>  first
>  v4: Use OPT_NUMBER for file mode arguments, fix back to front naming,
>  fix resource leak and add sanity checking for fmode/dmode arguments
>  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  | 25 +
>  hw/9pfs/9p.c|  3 +++
>  qemu-options.hx | 20 
>  5 files changed, 56 insertions(+), 8 deletions(-)
> 
> diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
> index 0844a403dc..474c79d003 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 fmode;
> +mode_t dmode;
>  } FsDriverEntry;
>  
>  typedef struct FsContext
> @@ -88,6 +90,8 @@ typedef struct FsContext
>  FsThrottle *fst;
>  /* fs driver specific data */
>  void *private;
> +mode_t fmode;
> +mode_t dmode;
>  } FsContext;
>  
>  typedef struct V9fsPath {
> diff --git a/fsdev/qemu-fsdev-opts.c b/fsdev/qemu-fsdev-opts.c
> index bf5713008a..7c31af 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 = "fmode",
> +.type = QEMU_OPT_NUMBER,
> +}, {
> +.name = "dmode",
> +.type = QEMU_OPT_NUMBER,
>  },
>  
>  THROTTLE_OPTS,
> @@ -75,6 +81,12 @@ static QemuOptsList qemu_virtfs_opts = {
>  }, {
>  .name = "sock_fd",
>  .type = QEMU_OPT_NUMBER,
> +}, {
> +.name = "fmode",
> +.type = QEMU_OPT_NUMBER,
> +}, {
> +.name = "dmode",
> +.type = QEMU_OPT_NUMBER,
>  },
>  
>  { /*End of list */ }
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index 1e78b7c9e9..f1ce03b06a 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->fmode | 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->dmode);
>  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->fmode);
>  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->fmode);
>  if (fd == -1) {
>  goto out;
>  }
> @@ -1467,6 +1467,23 @@ static int local_parse_opts(QemuOpts *opts, struct 
> FsDriverEntry *fse)
>  return -1;
>  }
>  
> +if (fse->export_flags & V9FS_SM_MAPPED ||
> +fse->export_flags & V9FS_SM_MAPPED_FILE) {
> +fse->fmode =
> +qemu_opt_get_number(opts, "fmode", 

[Qemu-devel] [PATCH v5 1/1] 9pfs: local: Add support for custom fmode/dmode in 9ps mapped security modes

[Tobias Schramm]

In mapped security modes, files are created with very restrictive
permissions (600 for files and 700 for directories). This makes
file sharing between virtual machines and users on the host rather
complicated. Imagine eg. a group of users that need to access data
produced by processes on a virtual machine. Giving those users access
to the data will be difficult since the group access mode is always 0.

This patch makes the default mode for both files and directories
configurable. Existing setups that don't know about the new parameters
keep using the current secure behavior.

Signed-off-by: Tobias Schramm 
---
 v5: Eliminate expandable variables, check mandatory commandline options
 first
 v4: Use OPT_NUMBER for file mode arguments, fix back to front naming,
 fix resource leak and add sanity checking for fmode/dmode arguments
 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  | 25 +
 hw/9pfs/9p.c|  3 +++
 qemu-options.hx | 20 
 5 files changed, 56 insertions(+), 8 deletions(-)

diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index 0844a403dc..474c79d003 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 fmode;
+mode_t dmode;
 } FsDriverEntry;
 
 typedef struct FsContext
@@ -88,6 +90,8 @@ typedef struct FsContext
 FsThrottle *fst;
 /* fs driver specific data */
 void *private;
+mode_t fmode;
+mode_t dmode;
 } FsContext;
 
 typedef struct V9fsPath {
diff --git a/fsdev/qemu-fsdev-opts.c b/fsdev/qemu-fsdev-opts.c
index bf5713008a..7c31af 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 = "fmode",
+.type = QEMU_OPT_NUMBER,
+}, {
+.name = "dmode",
+.type = QEMU_OPT_NUMBER,
 },
 
 THROTTLE_OPTS,
@@ -75,6 +81,12 @@ static QemuOptsList qemu_virtfs_opts = {
 }, {
 .name = "sock_fd",
 .type = QEMU_OPT_NUMBER,
+}, {
+.name = "fmode",
+.type = QEMU_OPT_NUMBER,
+}, {
+.name = "dmode",
+.type = QEMU_OPT_NUMBER,
 },
 
 { /*End of list */ }
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 1e78b7c9e9..f1ce03b06a 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->fmode | 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->dmode);
 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->fmode);
 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->fmode);
 if (fd == -1) {
 goto out;
 }
@@ -1467,6 +1467,23 @@ static int local_parse_opts(QemuOpts *opts, struct 
FsDriverEntry *fse)
 return -1;
 }
 
+if (fse->export_flags & V9FS_SM_MAPPED ||
+fse->export_flags & V9FS_SM_MAPPED_FILE) {
+fse->fmode =
+qemu_opt_get_number(opts, "fmode", SM_LOCAL_MODE_BITS) & 0777;
+fse->dmode =
+qemu_opt_get_number(opts, "dmode", SM_LOCAL_DIR_MODE_BITS) & 0777;
+} else {
+if (qemu_opt_find(opts, "fmode")) {
+error_report("fmode is only valid for mapped 9p modes");
+return -1;
+}
+if (qemu_opt_find(opts, "dmode")) {
+error_report("dmode is only valid for mapped 9p modes");
+return