Re: [PATCH] virtiofsd: Allow capability restriction to be turned off

2020-06-19 Thread no-reply 
Patchew URL: 
https://patchew.org/QEMU/20200619171809.30984-1-dgilb...@redhat.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

  GEN docs/interop/qemu-qmp-ref.txt
  GEN docs/interop/qemu-qmp-ref.7
  CC  qga/commands.o
/usr/bin/ld: 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o):
 warning: common of `__interception::real_vfork' overridden by definition from 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  CC  qga/guest-agent-command-state.o
  CC  qga/main.o
  CC  qga/commands-posix.o
---
  GEN docs/interop/qemu-ga-ref.html
  GEN docs/interop/qemu-ga-ref.txt
  GEN docs/interop/qemu-ga-ref.7
/usr/bin/ld: 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o):
 warning: common of `__interception::real_vfork' overridden by definition from 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINKqemu-keymap
  LINKivshmem-client
  LINKivshmem-server
/usr/bin/ld: 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o):
 warning: common of `__interception::real_vfork' overridden by definition from 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o):
 warning: common of `__interception::real_vfork' overridden by definition from 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  AS  pc-bios/optionrom/multiboot.o
  AS  pc-bios/optionrom/linuxboot.o
  CC  pc-bios/optionrom/linuxboot_dma.o
  AS  pc-bios/optionrom/kvmvapic.o
/usr/bin/ld: 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o):
 warning: common of `__interception::real_vfork' overridden by definition from 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINKqemu-nbd
  LINKqemu-storage-daemon
  AS  pc-bios/optionrom/pvh.o
  LINKqemu-img
/usr/bin/ld: 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o):
 warning: common of `__interception::real_vfork' overridden by definition from 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o):
 warning: common of `__interception::real_vfork' overridden by definition from 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINKqemu-io
  CC  pc-bios/optionrom/pvh_main.o
  BUILD   pc-bios/optionrom/multiboot.img
/usr/bin/ld: 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o):
 warning: common of `__interception::real_vfork' overridden by definition from 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o):
 warning: common of `__interception::real_vfork' overridden by definition from 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  BUILD   pc-bios/optionrom/linuxboot.img
  BUILD   pc-bios/optionrom/kvmvapic.img
  LINKqemu-edid
  BUILD   pc-bios/optionrom/multiboot.raw
/usr/bin/ld: 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o):
 warning: common of `__interception::real_vfork' overridden by definition from 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  BUILD   pc-bios/optionrom/linuxboot.raw
  BUILD   pc-bios/optionrom/kvmvapic.raw
  SIGNpc-bios/optionrom/multiboot.bin
  LINKfsdev/virtfs-proxy-helper
  SIGNpc-bios/optionrom/linuxboot.bin
/usr/bin/ld: 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o):
 warning: common of `__interception::real_vfork' overridden by definition from 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  BUILD   pc-bios/optionrom/linuxboot_dma.img
  SIGNpc-bios/optionrom/kvmvapic.bin
  BUILD   pc-bios/optionrom/pvh.img
---
  SIGNpc-bios/optionrom/linuxboot_dma.bin
  SIGNpc-bios/optionrom/pvh.bin
  LINKscsi/qemu-pr-helper
/usr/bin/ld: 
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o):
 warning: common of `__interception::real_vfork' overridden by definition from

Re: [PATCH] virtiofsd: Allow capability restriction to be turned off

2020-06-19 Thread Vivek Goyal 
On Fri, Jun 19, 2020 at 06:18:09PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> Commit a59feb483b8fae24d043 dropped most capabilities at startup,
> since in most cases virtiofsd doesn't need them.  Unfortunately we found
> a case that needs CAP_SYS_ADMIN - setting trusted extended attributes.
> It's rare to need it, and the clean fix for it is going to be more
> complicated, but as a way for existing users to continue, allow the
> capability restriction to be turned off by adding
> 
>   -o no_restrictcaps
> 
> We still drop the capabilities by default since this seems much safer;
> even if we don't have an explicit problem to fix.
> 

Hi David,

In one of the emails, Daniel suggested option "--trusted-xattrs=on|off". 
How about we use that and if that option is specified, we automatically
give CAP_SYS_ADMIN as well. Something to consider.

This probably could be extended later to "--trusted-xattrs=on|off|remapped"
when we have the capability to remap. And in case of "off/remap" we can take
away CAP_SYS_ADMIN. 

Vivek

> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  docs/tools/virtiofsd.rst | 3 +++
>  tools/virtiofsd/helper.c | 3 +++
>  tools/virtiofsd/passthrough_ll.c | 8 +++-
>  3 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst
> index 378594c422..8819d7d19e 100644
> --- a/docs/tools/virtiofsd.rst
> +++ b/docs/tools/virtiofsd.rst
> @@ -81,6 +81,9 @@ Options
>  Enable/disable extended attributes (xattr) on files and directories.  The
>  default is ``no_xattr``.
>  
> +  * restrictcaps|no_restrictcaps\n"
> +Restrict capabilities at startup.  The default is ``restrictcaps``.
> +
>  .. option:: --socket-path=PATH
>  
>Listen on vhost-user UNIX domain socket at PATH.
> diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c
> index 00a1ef666a..51ed9fbed0 100644
> --- a/tools/virtiofsd/helper.c
> +++ b/tools/virtiofsd/helper.c
> @@ -174,6 +174,9 @@ void fuse_cmdline_help(void)
> "   default: no_writeback\n"
> "-o xattr|no_xattr  enable/disable xattr\n"
> "   default: no_xattr\n"
> +   "-o restrictcaps|no_restrictcaps\n"
> +   "   restrict capabilities at 
> startup\n"
> +   "   default: restrictcaps\n"
> "--rlimit-nofile=  set maximum number of file 
> descriptors\n"
> "   (0 leaves rlimit unchanged)\n"
> "   default: min(100, fs.file-max 
> - 16384)\n"
> diff --git a/tools/virtiofsd/passthrough_ll.c 
> b/tools/virtiofsd/passthrough_ll.c
> index 2ce7c96085..8dee657b19 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -144,6 +144,7 @@ struct lo_data {
>  int flock;
>  int posix_lock;
>  int xattr;
> +int restrictcaps;
>  char *source;
>  double timeout;
>  int cache;
> @@ -170,6 +171,8 @@ static const struct fuse_opt lo_opts[] = {
>  { "no_posix_lock", offsetof(struct lo_data, posix_lock), 0 },
>  { "xattr", offsetof(struct lo_data, xattr), 1 },
>  { "no_xattr", offsetof(struct lo_data, xattr), 0 },
> +{ "restrictcaps", offsetof(struct lo_data, restrictcaps), 1 },
> +{ "no_restrictcaps", offsetof(struct lo_data, restrictcaps), 0 },
>  { "timeout=%lf", offsetof(struct lo_data, timeout), 0 },
>  { "timeout=", offsetof(struct lo_data, timeout_set), 1 },
>  { "cache=none", offsetof(struct lo_data, cache), CACHE_NONE },
> @@ -2615,7 +2618,9 @@ static void setup_sandbox(struct lo_data *lo, struct 
> fuse_session *se,
>  setup_namespaces(lo, se);
>  setup_mounts(lo->source);
>  setup_seccomp(enable_syslog);
> -setup_capabilities();
> +if (lo->restrictcaps) {
> +setup_capabilities();
> +}
>  }
>  
>  /* Set the maximum number of open file descriptors */
> @@ -2764,6 +2769,7 @@ int main(int argc, char *argv[])
>  .writeback = 0,
>  .posix_lock = 1,
>  .proc_self_fd = -1,
> +.restrictcaps = 1,
>  };
>  struct lo_map_elem *root_elem;
>  int ret = -1;
> -- 
> 2.26.2
>

[PATCH] virtiofsd: Allow capability restriction to be turned off

2020-06-19 Thread Dr. David Alan Gilbert (git) 
From: "Dr. David Alan Gilbert" 

Commit a59feb483b8fae24d043 dropped most capabilities at startup,
since in most cases virtiofsd doesn't need them.  Unfortunately we found
a case that needs CAP_SYS_ADMIN - setting trusted extended attributes.
It's rare to need it, and the clean fix for it is going to be more
complicated, but as a way for existing users to continue, allow the
capability restriction to be turned off by adding

  -o no_restrictcaps

We still drop the capabilities by default since this seems much safer;
even if we don't have an explicit problem to fix.

Signed-off-by: Dr. David Alan Gilbert 
---
 docs/tools/virtiofsd.rst | 3 +++
 tools/virtiofsd/helper.c | 3 +++
 tools/virtiofsd/passthrough_ll.c | 8 +++-
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst
index 378594c422..8819d7d19e 100644
--- a/docs/tools/virtiofsd.rst
+++ b/docs/tools/virtiofsd.rst
@@ -81,6 +81,9 @@ Options
 Enable/disable extended attributes (xattr) on files and directories.  The
 default is ``no_xattr``.
 
+  * restrictcaps|no_restrictcaps\n"
+Restrict capabilities at startup.  The default is ``restrictcaps``.
+
 .. option:: --socket-path=PATH
 
   Listen on vhost-user UNIX domain socket at PATH.
diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c
index 00a1ef666a..51ed9fbed0 100644
--- a/tools/virtiofsd/helper.c
+++ b/tools/virtiofsd/helper.c
@@ -174,6 +174,9 @@ void fuse_cmdline_help(void)
"   default: no_writeback\n"
"-o xattr|no_xattr  enable/disable xattr\n"
"   default: no_xattr\n"
+   "-o restrictcaps|no_restrictcaps\n"
+   "   restrict capabilities at startup\n"
+   "   default: restrictcaps\n"
"--rlimit-nofile=  set maximum number of file 
descriptors\n"
"   (0 leaves rlimit unchanged)\n"
"   default: min(100, fs.file-max - 
16384)\n"
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 2ce7c96085..8dee657b19 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -144,6 +144,7 @@ struct lo_data {
 int flock;
 int posix_lock;
 int xattr;
+int restrictcaps;
 char *source;
 double timeout;
 int cache;
@@ -170,6 +171,8 @@ static const struct fuse_opt lo_opts[] = {
 { "no_posix_lock", offsetof(struct lo_data, posix_lock), 0 },
 { "xattr", offsetof(struct lo_data, xattr), 1 },
 { "no_xattr", offsetof(struct lo_data, xattr), 0 },
+{ "restrictcaps", offsetof(struct lo_data, restrictcaps), 1 },
+{ "no_restrictcaps", offsetof(struct lo_data, restrictcaps), 0 },
 { "timeout=%lf", offsetof(struct lo_data, timeout), 0 },
 { "timeout=", offsetof(struct lo_data, timeout_set), 1 },
 { "cache=none", offsetof(struct lo_data, cache), CACHE_NONE },
@@ -2615,7 +2618,9 @@ static void setup_sandbox(struct lo_data *lo, struct 
fuse_session *se,
 setup_namespaces(lo, se);
 setup_mounts(lo->source);
 setup_seccomp(enable_syslog);
-setup_capabilities();
+if (lo->restrictcaps) {
+setup_capabilities();
+}
 }
 
 /* Set the maximum number of open file descriptors */
@@ -2764,6 +2769,7 @@ int main(int argc, char *argv[])
 .writeback = 0,
 .posix_lock = 1,
 .proc_self_fd = -1,
+.restrictcaps = 1,
 };
 struct lo_map_elem *root_elem;
 int ret = -1;
-- 
2.26.2