All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 0/6] virtiofs queue
@ 2020-05-01 19:14 Dr. David Alan Gilbert (git)
  2020-05-01 19:14 ` [PULL 1/6] virtiofsd: add --rlimit-nofile=NUM option Dr. David Alan Gilbert (git)
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-05-01 19:14 UTC (permalink / raw)
  To: qemu-devel, stefanha, yavrahami, mszeredi, mreitz

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The following changes since commit 1c47613588ccff44422d4bdeea0dc36a0a308ec7:

  Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2020-04-30 19:25:41 +0100)

are available in the Git repository at:

  https://gitlab.com/dagrh/qemu.git tags/pull-virtiofs-20200501

for you to fetch changes up to 66502bbca37ca7a3bfa57e82cfc03b89a7a11eae:

  virtiofsd: drop all capabilities in the wait parent process (2020-05-01 20:05:37 +0100)

----------------------------------------------------------------
virtiofsd: Pull 2020-05-01 (includes CVE fix)

This set includes a security fix, other fixes and improvements.

Security fix:
The security fix is for CVE-2020-10717 where, on low RAM hosts,
the guest can potentially exceed the maximum fd limit.
This fix adds some more configuration so that the user
can explicitly set the limit.
Thank you to Yuval Avrahami for reporting this.

Fixes:

Recursive mounting of the exported directory is now used in
the sandbox, such that if there was a mount underneath present at
the time the virtiofsd was started, that mount is also
visible to the guest; in the existing code, only mounts that
happened after startup were visible.

Security improvements:

The jailing for /proc/self/fd is improved - but it's something
that shouldn't be accessible anyway.

Most capabilities are now dropped at startup; again this shouldn't
change any behaviour but is extra protection.

----------------------------------------------------------------
Max Reitz (1):
      virtiofsd: Show submounts

Miklos Szeredi (1):
      virtiofsd: jail lo->proc_self_fd

Stefan Hajnoczi (4):
      virtiofsd: add --rlimit-nofile=NUM option
      virtiofsd: stay below fs.file-max sysctl value (CVE-2020-10717)
      virtiofsd: only retain file system capabilities
      virtiofsd: drop all capabilities in the wait parent process

 tools/virtiofsd/fuse_lowlevel.h  |   1 +
 tools/virtiofsd/helper.c         |  47 ++++++++++++++++++
 tools/virtiofsd/passthrough_ll.c | 102 ++++++++++++++++++++++++++++++++-------
 3 files changed, 133 insertions(+), 17 deletions(-)



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PULL 1/6] virtiofsd: add --rlimit-nofile=NUM option
  2020-05-01 19:14 [PULL 0/6] virtiofs queue Dr. David Alan Gilbert (git)
@ 2020-05-01 19:14 ` Dr. David Alan Gilbert (git)
  2020-05-01 19:14 ` [PULL 2/6] virtiofsd: stay below fs.file-max sysctl value (CVE-2020-10717) Dr. David Alan Gilbert (git)
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-05-01 19:14 UTC (permalink / raw)
  To: qemu-devel, stefanha, yavrahami, mszeredi, mreitz

From: Stefan Hajnoczi <stefanha@redhat.com>

Make it possible to specify the RLIMIT_NOFILE on the command-line.
Users running multiple virtiofsd processes should allocate a certain
number to each process so that the system-wide limit can never be
exhausted.

When this option is set to 0 the rlimit is left at its current value.
This is useful when a management tool wants to configure the rlimit
itself.

The default behavior remains unchanged: try to set the limit to
1,000,000 file descriptors if the current rlimit is lower.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20200501140644.220940-2-stefanha@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 tools/virtiofsd/fuse_lowlevel.h  |  1 +
 tools/virtiofsd/helper.c         | 23 +++++++++++++++++++++++
 tools/virtiofsd/passthrough_ll.c | 22 ++++++++--------------
 3 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/tools/virtiofsd/fuse_lowlevel.h b/tools/virtiofsd/fuse_lowlevel.h
index 8f6d705b5c..562fd5241e 100644
--- a/tools/virtiofsd/fuse_lowlevel.h
+++ b/tools/virtiofsd/fuse_lowlevel.h
@@ -1777,6 +1777,7 @@ struct fuse_cmdline_opts {
     int syslog;
     int log_level;
     unsigned int max_idle_threads;
+    unsigned long rlimit_nofile;
 };
 
 /**
diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c
index 819c2bc13c..dc59f38af0 100644
--- a/tools/virtiofsd/helper.c
+++ b/tools/virtiofsd/helper.c
@@ -23,6 +23,8 @@
 #include <stdlib.h>
 #include <string.h>
 #include <sys/param.h>
+#include <sys/time.h>
+#include <sys/resource.h>
 #include <unistd.h>
 
 #define FUSE_HELPER_OPT(t, p)                       \
@@ -53,6 +55,7 @@ static const struct fuse_opt fuse_helper_opts[] = {
     FUSE_HELPER_OPT("subtype=", nodefault_subtype),
     FUSE_OPT_KEY("subtype=", FUSE_OPT_KEY_KEEP),
     FUSE_HELPER_OPT("max_idle_threads=%u", max_idle_threads),
+    FUSE_HELPER_OPT("--rlimit-nofile=%lu", rlimit_nofile),
     FUSE_HELPER_OPT("--syslog", syslog),
     FUSE_HELPER_OPT_VALUE("log_level=debug", log_level, FUSE_LOG_DEBUG),
     FUSE_HELPER_OPT_VALUE("log_level=info", log_level, FUSE_LOG_INFO),
@@ -171,6 +174,9 @@ void fuse_cmdline_help(void)
            "                               default: no_writeback\n"
            "    -o xattr|no_xattr          enable/disable xattr\n"
            "                               default: no_xattr\n"
+           "    --rlimit-nofile=<num>      set maximum number of file descriptors\n"
+           "                               (0 leaves rlimit unchanged)\n"
+           "                               default: 1,000,000 if the current rlimit is lower\n"
            );
 }
 
@@ -191,11 +197,28 @@ static int fuse_helper_opt_proc(void *data, const char *arg, int key,
     }
 }
 
+static unsigned long get_default_rlimit_nofile(void)
+{
+    rlim_t max_fds = 1000000; /* our default RLIMIT_NOFILE target */
+    struct rlimit rlim;
+
+    if (getrlimit(RLIMIT_NOFILE, &rlim) < 0) {
+        fuse_log(FUSE_LOG_ERR, "getrlimit(RLIMIT_NOFILE): %m\n");
+        exit(1);
+    }
+
+    if (rlim.rlim_cur >= max_fds) {
+        return 0; /* we have more fds available than required! */
+    }
+    return max_fds;
+}
+
 int fuse_parse_cmdline(struct fuse_args *args, struct fuse_cmdline_opts *opts)
 {
     memset(opts, 0, sizeof(struct fuse_cmdline_opts));
 
     opts->max_idle_threads = 10;
+    opts->rlimit_nofile = get_default_rlimit_nofile();
     opts->foreground = 1;
 
     if (fuse_opt_parse(args, opts, fuse_helper_opts, fuse_helper_opt_proc) ==
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 4c35c95b25..f7b9c1d20c 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -2707,24 +2707,18 @@ static void setup_sandbox(struct lo_data *lo, struct fuse_session *se,
     setup_seccomp(enable_syslog);
 }
 
-/* Raise the maximum number of open file descriptors */
-static void setup_nofile_rlimit(void)
+/* Set the maximum number of open file descriptors */
+static void setup_nofile_rlimit(unsigned long rlimit_nofile)
 {
-    const rlim_t max_fds = 1000000;
-    struct rlimit rlim;
-
-    if (getrlimit(RLIMIT_NOFILE, &rlim) < 0) {
-        fuse_log(FUSE_LOG_ERR, "getrlimit(RLIMIT_NOFILE): %m\n");
-        exit(1);
-    }
+    struct rlimit rlim = {
+        .rlim_cur = rlimit_nofile,
+        .rlim_max = rlimit_nofile,
+    };
 
-    if (rlim.rlim_cur >= max_fds) {
+    if (rlimit_nofile == 0) {
         return; /* nothing to do */
     }
 
-    rlim.rlim_cur = max_fds;
-    rlim.rlim_max = max_fds;
-
     if (setrlimit(RLIMIT_NOFILE, &rlim) < 0) {
         /* Ignore SELinux denials */
         if (errno == EPERM) {
@@ -2977,7 +2971,7 @@ int main(int argc, char *argv[])
 
     fuse_daemonize(opts.foreground);
 
-    setup_nofile_rlimit();
+    setup_nofile_rlimit(opts.rlimit_nofile);
 
     /* Must be before sandbox since it wants /proc */
     setup_capng();
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 2/6] virtiofsd: stay below fs.file-max sysctl value (CVE-2020-10717)
  2020-05-01 19:14 [PULL 0/6] virtiofs queue Dr. David Alan Gilbert (git)
  2020-05-01 19:14 ` [PULL 1/6] virtiofsd: add --rlimit-nofile=NUM option Dr. David Alan Gilbert (git)
@ 2020-05-01 19:14 ` Dr. David Alan Gilbert (git)
  2020-05-01 19:14 ` [PULL 3/6] virtiofsd: jail lo->proc_self_fd Dr. David Alan Gilbert (git)
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-05-01 19:14 UTC (permalink / raw)
  To: qemu-devel, stefanha, yavrahami, mszeredi, mreitz

From: Stefan Hajnoczi <stefanha@redhat.com>

The system-wide fs.file-max sysctl value determines how many files can
be open.  It defaults to a value calculated based on the machine's RAM
size.  Previously virtiofsd would try to set RLIMIT_NOFILE to 1,000,000
and this allowed the FUSE client to exhaust the number of open files
system-wide on Linux hosts with less than 10 GB of RAM!

Take fs.file-max into account when choosing the default RLIMIT_NOFILE
value.

Fixes: CVE-2020-10717
Reported-by: Yuval Avrahami <yavrahami@paloaltonetworks.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20200501140644.220940-3-stefanha@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 tools/virtiofsd/helper.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c
index dc59f38af0..00a1ef666a 100644
--- a/tools/virtiofsd/helper.c
+++ b/tools/virtiofsd/helper.c
@@ -176,7 +176,8 @@ void fuse_cmdline_help(void)
            "                               default: no_xattr\n"
            "    --rlimit-nofile=<num>      set maximum number of file descriptors\n"
            "                               (0 leaves rlimit unchanged)\n"
-           "                               default: 1,000,000 if the current rlimit is lower\n"
+           "                               default: min(1000000, fs.file-max - 16384)\n"
+           "                                        if the current rlimit is lower\n"
            );
 }
 
@@ -199,9 +200,32 @@ static int fuse_helper_opt_proc(void *data, const char *arg, int key,
 
 static unsigned long get_default_rlimit_nofile(void)
 {
+    g_autofree gchar *file_max_str = NULL;
+    const rlim_t reserved_fds = 16384; /* leave at least this many fds free */
     rlim_t max_fds = 1000000; /* our default RLIMIT_NOFILE target */
+    rlim_t file_max;
     struct rlimit rlim;
 
+    /*
+     * Reduce max_fds below the system-wide maximum, if necessary.  This
+     * ensures there are fds available for other processes so we don't
+     * cause resource exhaustion.
+     */
+    if (!g_file_get_contents("/proc/sys/fs/file-max", &file_max_str,
+                             NULL, NULL)) {
+        fuse_log(FUSE_LOG_ERR, "can't read /proc/sys/fs/file-max\n");
+        exit(1);
+    }
+    file_max = g_ascii_strtoull(file_max_str, NULL, 10);
+    if (file_max < 2 * reserved_fds) {
+        fuse_log(FUSE_LOG_ERR,
+                 "The fs.file-max sysctl is too low (%lu) to allow a "
+                 "reasonable number of open files.\n",
+                 (unsigned long)file_max);
+        exit(1);
+    }
+    max_fds = MIN(file_max - reserved_fds, max_fds);
+
     if (getrlimit(RLIMIT_NOFILE, &rlim) < 0) {
         fuse_log(FUSE_LOG_ERR, "getrlimit(RLIMIT_NOFILE): %m\n");
         exit(1);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 3/6] virtiofsd: jail lo->proc_self_fd
  2020-05-01 19:14 [PULL 0/6] virtiofs queue Dr. David Alan Gilbert (git)
  2020-05-01 19:14 ` [PULL 1/6] virtiofsd: add --rlimit-nofile=NUM option Dr. David Alan Gilbert (git)
  2020-05-01 19:14 ` [PULL 2/6] virtiofsd: stay below fs.file-max sysctl value (CVE-2020-10717) Dr. David Alan Gilbert (git)
@ 2020-05-01 19:14 ` Dr. David Alan Gilbert (git)
  2020-05-01 19:14 ` [PULL 4/6] virtiofsd: Show submounts Dr. David Alan Gilbert (git)
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-05-01 19:14 UTC (permalink / raw)
  To: qemu-devel, stefanha, yavrahami, mszeredi, mreitz

From: Miklos Szeredi <mszeredi@redhat.com>

While it's not possible to escape the proc filesystem through
lo->proc_self_fd, it is possible to escape to the root of the proc
filesystem itself through "../..".

Use a temporary mount for opening lo->proc_self_fd, that has it's root at
/proc/self/fd/, preventing access to the ancestor directories.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Message-Id: <20200429124733.22488-1-mszeredi@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 tools/virtiofsd/passthrough_ll.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index f7b9c1d20c..d7a6474b6e 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -2536,6 +2536,8 @@ static void print_capabilities(void)
 static void setup_namespaces(struct lo_data *lo, struct fuse_session *se)
 {
     pid_t child;
+    char template[] = "virtiofsd-XXXXXX";
+    char *tmpdir;
 
     /*
      * Create a new pid namespace for *child* processes.  We'll have to
@@ -2597,12 +2599,33 @@ static void setup_namespaces(struct lo_data *lo, struct fuse_session *se)
         exit(1);
     }
 
+    tmpdir = mkdtemp(template);
+    if (!tmpdir) {
+        fuse_log(FUSE_LOG_ERR, "tmpdir(%s): %m\n", template);
+        exit(1);
+    }
+
+    if (mount("/proc/self/fd", tmpdir, NULL, MS_BIND, NULL) < 0) {
+        fuse_log(FUSE_LOG_ERR, "mount(/proc/self/fd, %s, MS_BIND): %m\n",
+                 tmpdir);
+        exit(1);
+    }
+
     /* Now we can get our /proc/self/fd directory file descriptor */
-    lo->proc_self_fd = open("/proc/self/fd", O_PATH);
+    lo->proc_self_fd = open(tmpdir, O_PATH);
     if (lo->proc_self_fd == -1) {
-        fuse_log(FUSE_LOG_ERR, "open(/proc/self/fd, O_PATH): %m\n");
+        fuse_log(FUSE_LOG_ERR, "open(%s, O_PATH): %m\n", tmpdir);
         exit(1);
     }
+
+    if (umount2(tmpdir, MNT_DETACH) < 0) {
+        fuse_log(FUSE_LOG_ERR, "umount2(%s, MNT_DETACH): %m\n", tmpdir);
+        exit(1);
+    }
+
+    if (rmdir(tmpdir) < 0) {
+        fuse_log(FUSE_LOG_ERR, "rmdir(%s): %m\n", tmpdir);
+    }
 }
 
 /*
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 4/6] virtiofsd: Show submounts
  2020-05-01 19:14 [PULL 0/6] virtiofs queue Dr. David Alan Gilbert (git)
                   ` (2 preceding siblings ...)
  2020-05-01 19:14 ` [PULL 3/6] virtiofsd: jail lo->proc_self_fd Dr. David Alan Gilbert (git)
@ 2020-05-01 19:14 ` Dr. David Alan Gilbert (git)
  2020-05-01 19:14 ` [PULL 5/6] virtiofsd: only retain file system capabilities Dr. David Alan Gilbert (git)
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-05-01 19:14 UTC (permalink / raw)
  To: qemu-devel, stefanha, yavrahami, mszeredi, mreitz

From: Max Reitz <mreitz@redhat.com>

Currently, setup_mounts() bind-mounts the shared directory without
MS_REC.  This makes all submounts disappear.

Pass MS_REC so that the guest can see submounts again.

Fixes: 5baa3b8e95064c2434bd9e2f312edd5e9ae275dc
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200424133516.73077-1-mreitz@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
  Changed Fixes to point to the commit with the problem rather than
          the commit that turned it on
---
 tools/virtiofsd/passthrough_ll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index d7a6474b6e..7873692168 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -2666,7 +2666,7 @@ static void setup_mounts(const char *source)
     int oldroot;
     int newroot;
 
-    if (mount(source, source, NULL, MS_BIND, NULL) < 0) {
+    if (mount(source, source, NULL, MS_BIND | MS_REC, NULL) < 0) {
         fuse_log(FUSE_LOG_ERR, "mount(%s, %s, MS_BIND): %m\n", source, source);
         exit(1);
     }
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 5/6] virtiofsd: only retain file system capabilities
  2020-05-01 19:14 [PULL 0/6] virtiofs queue Dr. David Alan Gilbert (git)
                   ` (3 preceding siblings ...)
  2020-05-01 19:14 ` [PULL 4/6] virtiofsd: Show submounts Dr. David Alan Gilbert (git)
@ 2020-05-01 19:14 ` Dr. David Alan Gilbert (git)
  2020-05-01 19:15 ` [PULL 6/6] virtiofsd: drop all capabilities in the wait parent process Dr. David Alan Gilbert (git)
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-05-01 19:14 UTC (permalink / raw)
  To: qemu-devel, stefanha, yavrahami, mszeredi, mreitz

From: Stefan Hajnoczi <stefanha@redhat.com>

virtiofsd runs as root but only needs a subset of root's Linux
capabilities(7).  As a file server its purpose is to create and access
files on behalf of a client.  It needs to be able to access files with
arbitrary uid/gid owners.  It also needs to be create device nodes.

Introduce a Linux capabilities(7) whitelist and drop all capabilities
that we don't need, making the virtiofsd process less powerful than a
regular uid root process.

  # cat /proc/PID/status
  ...
          Before           After
  CapInh: 0000000000000000 0000000000000000
  CapPrm: 0000003fffffffff 00000000880000df
  CapEff: 0000003fffffffff 00000000880000df
  CapBnd: 0000003fffffffff 0000000000000000
  CapAmb: 0000000000000000 0000000000000000

Note that file capabilities cannot be used to achieve the same effect on
the virtiofsd executable because mount is used during sandbox setup.
Therefore we drop capabilities programmatically at the right point
during startup.

This patch only affects the sandboxed child process.  The parent process
that sits in waitpid(2) still has full root capabilities and will be
addressed in the next patch.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20200416164907.244868-2-stefanha@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 tools/virtiofsd/passthrough_ll.c | 38 ++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 7873692168..e49650b63d 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -2718,6 +2718,43 @@ static void setup_mounts(const char *source)
     close(oldroot);
 }
 
+/*
+ * Only keep whitelisted capabilities that are needed for file system operation
+ */
+static void setup_capabilities(void)
+{
+    pthread_mutex_lock(&cap.mutex);
+    capng_restore_state(&cap.saved);
+
+    /*
+     * Whitelist file system-related capabilities that are needed for a file
+     * server to act like root.  Drop everything else like networking and
+     * sysadmin capabilities.
+     *
+     * Exclusions:
+     * 1. CAP_LINUX_IMMUTABLE is not included because it's only used via ioctl
+     *    and we don't support that.
+     * 2. CAP_MAC_OVERRIDE is not included because it only seems to be
+     *    used by the Smack LSM.  Omit it until there is demand for it.
+     */
+    capng_setpid(syscall(SYS_gettid));
+    capng_clear(CAPNG_SELECT_BOTH);
+    capng_updatev(CAPNG_ADD, CAPNG_PERMITTED | CAPNG_EFFECTIVE,
+            CAP_CHOWN,
+            CAP_DAC_OVERRIDE,
+            CAP_DAC_READ_SEARCH,
+            CAP_FOWNER,
+            CAP_FSETID,
+            CAP_SETGID,
+            CAP_SETUID,
+            CAP_MKNOD,
+            CAP_SETFCAP);
+    capng_apply(CAPNG_SELECT_BOTH);
+
+    cap.saved = capng_save_state();
+    pthread_mutex_unlock(&cap.mutex);
+}
+
 /*
  * Lock down this process to prevent access to other processes or files outside
  * source directory.  This reduces the impact of arbitrary code execution bugs.
@@ -2728,6 +2765,7 @@ 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();
 }
 
 /* Set the maximum number of open file descriptors */
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 6/6] virtiofsd: drop all capabilities in the wait parent process
  2020-05-01 19:14 [PULL 0/6] virtiofs queue Dr. David Alan Gilbert (git)
                   ` (4 preceding siblings ...)
  2020-05-01 19:14 ` [PULL 5/6] virtiofsd: only retain file system capabilities Dr. David Alan Gilbert (git)
@ 2020-05-01 19:15 ` Dr. David Alan Gilbert (git)
  2020-05-01 19:28 ` [PULL 0/6] virtiofs queue Dr. David Alan Gilbert
  2020-05-03 13:11 ` Peter Maydell
  7 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-05-01 19:15 UTC (permalink / raw)
  To: qemu-devel, stefanha, yavrahami, mszeredi, mreitz

From: Stefan Hajnoczi <stefanha@redhat.com>

All this process does is wait for its child.  No capabilities are
needed.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 tools/virtiofsd/passthrough_ll.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index e49650b63d..3ba1d90984 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -2530,6 +2530,17 @@ static void print_capabilities(void)
     printf("}\n");
 }
 
+/*
+ * Drop all Linux capabilities because the wait parent process only needs to
+ * sit in waitpid(2) and terminate.
+ */
+static void setup_wait_parent_capabilities(void)
+{
+    capng_setpid(syscall(SYS_gettid));
+    capng_clear(CAPNG_SELECT_BOTH);
+    capng_apply(CAPNG_SELECT_BOTH);
+}
+
 /*
  * Move to a new mount, net, and pid namespaces to isolate this process.
  */
@@ -2563,6 +2574,8 @@ static void setup_namespaces(struct lo_data *lo, struct fuse_session *se)
         pid_t waited;
         int wstatus;
 
+        setup_wait_parent_capabilities();
+
         /* The parent waits for the child */
         do {
             waited = waitpid(child, &wstatus, 0);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PULL 0/6] virtiofs queue
  2020-05-01 19:14 [PULL 0/6] virtiofs queue Dr. David Alan Gilbert (git)
                   ` (5 preceding siblings ...)
  2020-05-01 19:15 ` [PULL 6/6] virtiofsd: drop all capabilities in the wait parent process Dr. David Alan Gilbert (git)
@ 2020-05-01 19:28 ` Dr. David Alan Gilbert
  2020-05-03 13:11 ` Peter Maydell
  7 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert @ 2020-05-01 19:28 UTC (permalink / raw)
  To: qemu-stable, qemu-devel, stefanha, yavrahami, mszeredi, mreitz

Dear Stable,
  From this series, the fixes:

       virtiofsd: add --rlimit-nofile=NUM option
       virtiofsd: stay below fs.file-max sysctl value (CVE-2020-10717)

and
       virtiofsd: Show submounts

should probably be backported.

Dave

* Dr. David Alan Gilbert (git) (dgilbert@redhat.com) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> The following changes since commit 1c47613588ccff44422d4bdeea0dc36a0a308ec7:
> 
>   Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2020-04-30 19:25:41 +0100)
> 
> are available in the Git repository at:
> 
>   https://gitlab.com/dagrh/qemu.git tags/pull-virtiofs-20200501
> 
> for you to fetch changes up to 66502bbca37ca7a3bfa57e82cfc03b89a7a11eae:
> 
>   virtiofsd: drop all capabilities in the wait parent process (2020-05-01 20:05:37 +0100)
> 
> ----------------------------------------------------------------
> virtiofsd: Pull 2020-05-01 (includes CVE fix)
> 
> This set includes a security fix, other fixes and improvements.
> 
> Security fix:
> The security fix is for CVE-2020-10717 where, on low RAM hosts,
> the guest can potentially exceed the maximum fd limit.
> This fix adds some more configuration so that the user
> can explicitly set the limit.
> Thank you to Yuval Avrahami for reporting this.
> 
> Fixes:
> 
> Recursive mounting of the exported directory is now used in
> the sandbox, such that if there was a mount underneath present at
> the time the virtiofsd was started, that mount is also
> visible to the guest; in the existing code, only mounts that
> happened after startup were visible.
> 
> Security improvements:
> 
> The jailing for /proc/self/fd is improved - but it's something
> that shouldn't be accessible anyway.
> 
> Most capabilities are now dropped at startup; again this shouldn't
> change any behaviour but is extra protection.
> 
> ----------------------------------------------------------------
> Max Reitz (1):
>       virtiofsd: Show submounts
> 
> Miklos Szeredi (1):
>       virtiofsd: jail lo->proc_self_fd
> 
> Stefan Hajnoczi (4):
>       virtiofsd: add --rlimit-nofile=NUM option
>       virtiofsd: stay below fs.file-max sysctl value (CVE-2020-10717)
>       virtiofsd: only retain file system capabilities
>       virtiofsd: drop all capabilities in the wait parent process
> 
>  tools/virtiofsd/fuse_lowlevel.h  |   1 +
>  tools/virtiofsd/helper.c         |  47 ++++++++++++++++++
>  tools/virtiofsd/passthrough_ll.c | 102 ++++++++++++++++++++++++++++++++-------
>  3 files changed, 133 insertions(+), 17 deletions(-)
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PULL 0/6] virtiofs queue
  2020-05-01 19:14 [PULL 0/6] virtiofs queue Dr. David Alan Gilbert (git)
                   ` (6 preceding siblings ...)
  2020-05-01 19:28 ` [PULL 0/6] virtiofs queue Dr. David Alan Gilbert
@ 2020-05-03 13:11 ` Peter Maydell
  2020-05-04  8:13   ` Dr. David Alan Gilbert
  7 siblings, 1 reply; 14+ messages in thread
From: Peter Maydell @ 2020-05-03 13:11 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git)
  Cc: mszeredi, yavrahami, QEMU Developers, Stefan Hajnoczi, Max Reitz

On Fri, 1 May 2020 at 20:16, Dr. David Alan Gilbert (git)
<dgilbert@redhat.com> wrote:
>
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> The following changes since commit 1c47613588ccff44422d4bdeea0dc36a0a308ec7:
>
>   Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2020-04-30 19:25:41 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/dagrh/qemu.git tags/pull-virtiofs-20200501
>
> for you to fetch changes up to 66502bbca37ca7a3bfa57e82cfc03b89a7a11eae:
>
>   virtiofsd: drop all capabilities in the wait parent process (2020-05-01 20:05:37 +0100)
>
> ----------------------------------------------------------------
> virtiofsd: Pull 2020-05-01 (includes CVE fix)
>
> This set includes a security fix, other fixes and improvements.
>
> Security fix:
> The security fix is for CVE-2020-10717 where, on low RAM hosts,
> the guest can potentially exceed the maximum fd limit.
> This fix adds some more configuration so that the user
> can explicitly set the limit.
> Thank you to Yuval Avrahami for reporting this.
>
> Fixes:
>
> Recursive mounting of the exported directory is now used in
> the sandbox, such that if there was a mount underneath present at
> the time the virtiofsd was started, that mount is also
> visible to the guest; in the existing code, only mounts that
> happened after startup were visible.
>
> Security improvements:
>
> The jailing for /proc/self/fd is improved - but it's something
> that shouldn't be accessible anyway.
>
> Most capabilities are now dropped at startup; again this shouldn't
> change any behaviour but is extra protection.
>
> ----------------------------------------------------------------


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
for any user-visible changes.

I notice you didn't include the usual Cc: qemu-stable@nongnu.org
lines in the commits to be backported, but I think the stable
branch maintainers can deal with the occasional manual notification.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PULL 0/6] virtiofs queue
  2020-05-03 13:11 ` Peter Maydell
@ 2020-05-04  8:13   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert @ 2020-05-04  8:13 UTC (permalink / raw)
  To: Peter Maydell
  Cc: mszeredi, yavrahami, QEMU Developers, Stefan Hajnoczi, Max Reitz

* Peter Maydell (peter.maydell@linaro.org) wrote:
> On Fri, 1 May 2020 at 20:16, Dr. David Alan Gilbert (git)
> <dgilbert@redhat.com> wrote:
> >
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > The following changes since commit 1c47613588ccff44422d4bdeea0dc36a0a308ec7:
> >
> >   Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2020-04-30 19:25:41 +0100)
> >
> > are available in the Git repository at:
> >
> >   https://gitlab.com/dagrh/qemu.git tags/pull-virtiofs-20200501
> >
> > for you to fetch changes up to 66502bbca37ca7a3bfa57e82cfc03b89a7a11eae:
> >
> >   virtiofsd: drop all capabilities in the wait parent process (2020-05-01 20:05:37 +0100)
> >
> > ----------------------------------------------------------------
> > virtiofsd: Pull 2020-05-01 (includes CVE fix)
> >
> > This set includes a security fix, other fixes and improvements.
> >
> > Security fix:
> > The security fix is for CVE-2020-10717 where, on low RAM hosts,
> > the guest can potentially exceed the maximum fd limit.
> > This fix adds some more configuration so that the user
> > can explicitly set the limit.
> > Thank you to Yuval Avrahami for reporting this.
> >
> > Fixes:
> >
> > Recursive mounting of the exported directory is now used in
> > the sandbox, such that if there was a mount underneath present at
> > the time the virtiofsd was started, that mount is also
> > visible to the guest; in the existing code, only mounts that
> > happened after startup were visible.
> >
> > Security improvements:
> >
> > The jailing for /proc/self/fd is improved - but it's something
> > that shouldn't be accessible anyway.
> >
> > Most capabilities are now dropped at startup; again this shouldn't
> > change any behaviour but is extra protection.
> >
> > ----------------------------------------------------------------
> 
> 
> Applied, thanks.
> 
> Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
> for any user-visible changes.
> 
> I notice you didn't include the usual Cc: qemu-stable@nongnu.org
> lines in the commits to be backported, but I think the stable
> branch maintainers can deal with the occasional manual notification.

Thanks, yes I sent a mail to qemu-stable as a reply to the series
saying which patches I thought should be for stable.

Dave

> thanks
> -- PMM
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PULL 0/6] virtiofs queue
  2021-02-16 18:37 Dr. David Alan Gilbert (git)
@ 2021-02-17 19:18 ` Peter Maydell
  0 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2021-02-17 19:18 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git)
  Cc: QEMU Developers, Wainer dos Santos Moschetta, Greg Kurz,
	virtio-fs, Philippe Mathieu-Daudé,
	vgoyal

On Tue, 16 Feb 2021 at 18:45, Dr. David Alan Gilbert (git)
<dgilbert@redhat.com> wrote:
>
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> The following changes since commit 18543229fd7a2c79dcd6818c7b1f0f62512b5220:
>
>   Merge remote-tracking branch 'remotes/cleber-gitlab/tags/python-next-pull-request' into staging (2021-02-16 14:37:57 +0000)
>
> are available in the Git repository at:
>
>   https://gitlab.com/dagrh/qemu.git tags/pull-virtiofs-20210216
>
> for you to fetch changes up to 26ec1909648e0c06ff06ebc3ddb2f88ebeeaa6a9:
>
>   virtiofsd: Do not use a thread pool by default (2021-02-16 17:54:18 +0000)
>
> ----------------------------------------------------------------
> virtiofsd pull 2021-02-16
>
> Vivek's support for new FUSE KILLPRIV_V2
> and some smaller cleanups.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PULL 0/6] virtiofs queue
@ 2021-02-16 18:37 Dr. David Alan Gilbert (git)
  2021-02-17 19:18 ` Peter Maydell
  0 siblings, 1 reply; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2021-02-16 18:37 UTC (permalink / raw)
  To: qemu-devel, wainersm, groug, philmd, vgoyal; +Cc: virtio-fs

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The following changes since commit 18543229fd7a2c79dcd6818c7b1f0f62512b5220:

  Merge remote-tracking branch 'remotes/cleber-gitlab/tags/python-next-pull-request' into staging (2021-02-16 14:37:57 +0000)

are available in the Git repository at:

  https://gitlab.com/dagrh/qemu.git tags/pull-virtiofs-20210216

for you to fetch changes up to 26ec1909648e0c06ff06ebc3ddb2f88ebeeaa6a9:

  virtiofsd: Do not use a thread pool by default (2021-02-16 17:54:18 +0000)

----------------------------------------------------------------
virtiofsd pull 2021-02-16

Vivek's support for new FUSE KILLPRIV_V2
and some smaller cleanups.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

----------------------------------------------------------------
Greg Kurz (1):
      virtiofsd: vu_dispatch locking should never fail

Philippe Mathieu-Daudé (1):
      tools/virtiofsd: Replace the word 'whitelist'

Vivek Goyal (3):
      virtiofsd: Save error code early at the failure callsite
      viriofsd: Add support for FUSE_HANDLE_KILLPRIV_V2
      virtiofsd: Do not use a thread pool by default

Wainer dos Santos Moschetta (1):
      virtiofsd: Allow to build it without the tools

 tools/meson.build                     |  7 ++-
 tools/virtiofsd/fuse_common.h         | 15 ++++++
 tools/virtiofsd/fuse_lowlevel.c       | 13 ++++-
 tools/virtiofsd/fuse_lowlevel.h       |  1 +
 tools/virtiofsd/fuse_virtio.c         | 49 ++++++++++++-----
 tools/virtiofsd/passthrough_ll.c      | 99 ++++++++++++++++++++++++++++++-----
 tools/virtiofsd/passthrough_seccomp.c | 12 ++---
 7 files changed, 158 insertions(+), 38 deletions(-)



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PULL 0/6] virtiofs queue
  2020-02-21 13:25 Dr. David Alan Gilbert (git)
@ 2020-02-21 18:37 ` Peter Maydell
  0 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2020-02-21 18:37 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git)
  Cc: Miroslav Rezanina, Philippe Mathieu-Daudé,
	QEMU Developers, yangx.jy

On Fri, 21 Feb 2020 at 13:52, Dr. David Alan Gilbert (git)
<dgilbert@redhat.com> wrote:
>
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> The following changes since commit b651b80822fa8cb66ca30087ac7fbc75507ae5d2:
>
>   Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-5.0-pull-request' into staging (2020-02-20 17:35:42 +0000)
>
> are available in the Git repository at:
>
>   https://gitlab.com/dagrh/qemu.git tags/pull-virtiofs-20200221
>
> for you to fetch changes up to 5bb8e8beedb47fc0d0a44957a154918c4f4afc80:
>
>   docs: Fix virtiofsd.1 location (2020-02-21 13:05:27 +0000)
>
> ----------------------------------------------------------------
> virtiofs pull 20200221
>
> Mostly minor cleanups.
> Miroslav's fixes a make install corner case.
> Philippe's set includes an error corner case fix.
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.0
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PULL 0/6] virtiofs queue
@ 2020-02-21 13:25 Dr. David Alan Gilbert (git)
  2020-02-21 18:37 ` Peter Maydell
  0 siblings, 1 reply; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-02-21 13:25 UTC (permalink / raw)
  To: qemu-devel, philmd, yangx.jy, mrezanin

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The following changes since commit b651b80822fa8cb66ca30087ac7fbc75507ae5d2:

  Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-5.0-pull-request' into staging (2020-02-20 17:35:42 +0000)

are available in the Git repository at:

  https://gitlab.com/dagrh/qemu.git tags/pull-virtiofs-20200221

for you to fetch changes up to 5bb8e8beedb47fc0d0a44957a154918c4f4afc80:

  docs: Fix virtiofsd.1 location (2020-02-21 13:05:27 +0000)

----------------------------------------------------------------
virtiofs pull 20200221

Mostly minor cleanups.
Miroslav's fixes a make install corner case.
Philippe's set includes an error corner case fix.

----------------------------------------------------------------
Dr. David Alan Gilbert (1):
      virtiofsd: Help message fix for 'seconds'

Miroslav Rezanina (1):
      docs: Fix virtiofsd.1 location

Philippe Mathieu-Daudé (3):
      tools/virtiofsd/passthrough_ll: Remove unneeded variable assignment
      tools/virtiofsd/passthrough_ll: Remove unneeded variable assignment
      tools/virtiofsd/fuse_lowlevel: Fix fuse_out_header::error value

Xiao Yang (1):
      virtiofsd: Remove fuse.h and struct fuse_module

 Makefile                         |    2 +-
 tools/virtiofsd/fuse.h           | 1229 --------------------------------------
 tools/virtiofsd/fuse_i.h         |   16 -
 tools/virtiofsd/fuse_lowlevel.c  |    2 +-
 tools/virtiofsd/helper.c         |    2 +-
 tools/virtiofsd/passthrough_ll.c |    4 -
 6 files changed, 3 insertions(+), 1252 deletions(-)
 delete mode 100644 tools/virtiofsd/fuse.h



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-02-17 19:20 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-01 19:14 [PULL 0/6] virtiofs queue Dr. David Alan Gilbert (git)
2020-05-01 19:14 ` [PULL 1/6] virtiofsd: add --rlimit-nofile=NUM option Dr. David Alan Gilbert (git)
2020-05-01 19:14 ` [PULL 2/6] virtiofsd: stay below fs.file-max sysctl value (CVE-2020-10717) Dr. David Alan Gilbert (git)
2020-05-01 19:14 ` [PULL 3/6] virtiofsd: jail lo->proc_self_fd Dr. David Alan Gilbert (git)
2020-05-01 19:14 ` [PULL 4/6] virtiofsd: Show submounts Dr. David Alan Gilbert (git)
2020-05-01 19:14 ` [PULL 5/6] virtiofsd: only retain file system capabilities Dr. David Alan Gilbert (git)
2020-05-01 19:15 ` [PULL 6/6] virtiofsd: drop all capabilities in the wait parent process Dr. David Alan Gilbert (git)
2020-05-01 19:28 ` [PULL 0/6] virtiofs queue Dr. David Alan Gilbert
2020-05-03 13:11 ` Peter Maydell
2020-05-04  8:13   ` Dr. David Alan Gilbert
  -- strict thread matches above, loose matches on Subject: below --
2021-02-16 18:37 Dr. David Alan Gilbert (git)
2021-02-17 19:18 ` Peter Maydell
2020-02-21 13:25 Dr. David Alan Gilbert (git)
2020-02-21 18:37 ` Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.