[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] 9pfs: deprecate 'proxy' backend
From: |
Christian Schoenebeck |
Subject: |
Re: [PATCH v2] 9pfs: deprecate 'proxy' backend |
Date: |
Sat, 10 Jun 2023 14:24:56 +0200 |
On Saturday, June 10, 2023 1:07:27 PM CEST Christian Schoenebeck wrote:
> As recent CVE-2023-2861 once again showed, the 9p 'proxy' fs driver is in
> bad shape. Using the 'proxy' backend was already discouraged for safety
> reasons before and we recommended to use the 'local' backend instead,
> but now it is time to officially deprecate the 'proxy' backend.
>
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> ---
> v1 -> v2:
> * Add deprecation notice also to virtfs-proxy-helper.rst,
> qemu-options.hx and to the 'proxy' source files
> (virtfs-proxy-helper.c, 9p-proxy.c, 9p-proxy.h).
>
> MAINTAINERS | 7 +++++++
> docs/about/deprecated.rst | 17 +++++++++++++++++
> docs/tools/virtfs-proxy-helper.rst | 3 +++
> fsdev/qemu-fsdev.c | 5 +++++
> fsdev/virtfs-proxy-helper.c | 5 +++++
> hw/9pfs/9p-proxy.c | 5 +++++
> hw/9pfs/9p-proxy.h | 5 +++++
> meson.build | 2 +-
> qemu-options.hx | 6 +++++-
> softmmu/vl.c | 5 +++++
> 10 files changed, 58 insertions(+), 2 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 436b3f0afe..185d694b2e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2118,13 +2118,20 @@ S: Odd Fixes
> W: https://wiki.qemu.org/Documentation/9p
> F: hw/9pfs/
> X: hw/9pfs/xen-9p*
> +X: hw/9pfs/9p-proxy*
> F: fsdev/
> +X: fsdev/virtfs-proxy-helper.c
> F: docs/tools/virtfs-proxy-helper.rst
> F: tests/qtest/virtio-9p-test.c
> F: tests/qtest/libqos/virtio-9p*
> T: git https://gitlab.com/gkurz/qemu.git 9p-next
> T: git https://github.com/cschoenebeck/qemu.git 9p.next
>
> +virtio-9p-proxy
> +F: hw/9pfs/9p-proxy*
> +F: fsdev/virtfs-proxy-helper.c
> +S: Obsolete
> +
> virtio-blk
> M: Stefan Hajnoczi <stefanha@redhat.com>
> L: qemu-block@nongnu.org
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 0743459862..9b2c780365 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -343,6 +343,23 @@ the addition of volatile memory support, it is now
> necessary to distinguish
> between persistent and volatile memory backends. As such, memdev is
> deprecated
> in favor of persistent-memdev.
>
> +``-fsdev proxy`` and ``-virtfs proxy`` (since 8.1)
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +The 9p ``proxy`` filesystem backend driver has been deprecated and will be
> +removed in a future version of QEMU. Please use ``-fsdev local`` or
> +``-virtfs local`` for using the ``local`` 9p filesystem backend instead.
> +
> +The 9p ``proxy`` backend was originally developed as an alternative to the 9p
> +``local`` backend. The idea was to enhance security by dispatching actual low
> +level filesystem operations from 9p server (QEMU process) over to a separate
> +process (the virtfs-proxy-helper binary). However this alternative never
> gained
> +momentum. The proxy backend is much slower than the local backend, hasn't
> seen
> +any development in years, and showed to be less secure, especially due to the
> +fact that its helper daemon must be run as root, whereas with the local
> backend
> +QEMU is typically run as unprivileged user and allows to tighten behaviour by
> +mapping permissions et al.
> +
>
> Block device options
> ''''''''''''''''''''
> diff --git a/docs/tools/virtfs-proxy-helper.rst
> b/docs/tools/virtfs-proxy-helper.rst
> index 6cdeedf8e9..f5051130e2 100644
> --- a/docs/tools/virtfs-proxy-helper.rst
> +++ b/docs/tools/virtfs-proxy-helper.rst
> @@ -9,6 +9,9 @@ Synopsis
> Description
> -----------
>
> +NOTE: The 9p 'proxy' nackend is deprecated (since QEMU 8.1) and will be
> +removed, along with this daemon, in a future version of QEMU!
> +
> Pass-through security model in QEMU 9p server needs root privilege to do
> few file operations (like chown, chmod to any mode/uid:gid). There are two
> issues in pass-through security model:
> diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
> index 3da64e9f72..242f54ab49 100644
> --- a/fsdev/qemu-fsdev.c
> +++ b/fsdev/qemu-fsdev.c
> @@ -133,6 +133,11 @@ int qemu_fsdev_add(QemuOpts *opts, Error **errp)
> }
>
> if (fsdriver) {
> + if (strncmp(fsdriver, "proxy", 5) == 0) {
> + warn_report("'-fsdev proxy' is deprecated, use '-fsdev local' "
> + "instead");
> + }
> +
> for (i = 0; i < ARRAY_SIZE(FsDrivers); i++) {
> if (strcmp(FsDrivers[i].name, fsdriver) == 0) {
> break;
> diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
> index d9511f429c..87e358376a 100644
> --- a/fsdev/virtfs-proxy-helper.c
> +++ b/fsdev/virtfs-proxy-helper.c
> @@ -9,6 +9,11 @@
> * the COPYING file in the top-level directory.
> */
>
> +/*
> + * NOTE: The 9p 'proxy' nackend is deprecated (since QEMU 8.1) and will be
> + * removed in a future version of QEMU!
> + */
> +
> #include "qemu/osdep.h"
> #include <glib/gstdio.h>
> #include <sys/resource.h>
> diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
> index 99d115ff0d..ab489cdd40 100644
> --- a/hw/9pfs/9p-proxy.c
> +++ b/hw/9pfs/9p-proxy.c
> @@ -15,6 +15,11 @@
> * https://wiki.qemu.org/Documentation/9p
> */
>
> +/*
> + * NOTE: The 9p 'proxy' nackend is deprecated (since QEMU 8.1) and will be
> + * removed in a future version of QEMU!
> + */
> +
> #include "qemu/osdep.h"
> #include <sys/socket.h>
> #include <sys/un.h>
> diff --git a/hw/9pfs/9p-proxy.h b/hw/9pfs/9p-proxy.h
> index b84301d001..4c224927b0 100644
> --- a/hw/9pfs/9p-proxy.h
> +++ b/hw/9pfs/9p-proxy.h
> @@ -10,6 +10,11 @@
> * the COPYING file in the top-level directory.
> */
>
> +/*
> + * NOTE: The 9p 'proxy' nackend is deprecated (since QEMU 8.1) and will be
> + * removed in a future version of QEMU!
> + */
> +
Grrr, typo and copy waste. :)
> #ifndef QEMU_9P_PROXY_H
> #define QEMU_9P_PROXY_H
>
> diff --git a/meson.build b/meson.build
> index 34306a6205..05c01b72bb 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -4170,7 +4170,7 @@ if have_block
> summary_info += {'Block whitelist (ro)':
> get_option('block_drv_ro_whitelist')}
> summary_info += {'Use block whitelist in tools':
> get_option('block_drv_whitelist_in_tools')}
> summary_info += {'VirtFS (9P) support': have_virtfs}
> - summary_info += {'VirtFS (9P) Proxy Helper support':
> have_virtfs_proxy_helper}
> + summary_info += {'VirtFS (9P) Proxy Helper support (deprecated)':
> have_virtfs_proxy_helper}
> summary_info += {'Live block migration':
> config_host_data.get('CONFIG_LIVE_BLOCK_MIGRATION')}
> summary_info += {'replication support':
> config_host_data.get('CONFIG_REPLICATION')}
> summary_info += {'bochs support': get_option('bochs').allowed()}
> diff --git a/qemu-options.hx b/qemu-options.hx
> index b57489d7ca..3a6c7d3ef9 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1735,7 +1735,9 @@ SRST
> Accesses to the filesystem are done by QEMU.
>
> ``proxy``
> - Accesses to the filesystem are done by virtfs-proxy-helper(1).
> + Accesses to the filesystem are done by virtfs-proxy-helper(1). This
> + option is deprecated (since QEMU 8.1) and will be removed in a future
> + version of QEMU. Use ``local`` instead.
>
> ``synth``
> Synthetic filesystem, only used by QTests.
> @@ -1867,6 +1869,8 @@ SRST
>
> ``proxy``
> Accesses to the filesystem are done by virtfs-proxy-helper(1).
> + This option is deprecated (since QEMU 8.1) and will be removed in a
> + future version of QEMU. Use ``local`` instead.
>
> ``synth``
> Synthetic filesystem, only used by QTests.
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index b0b96f67fa..e60648b591 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -3102,6 +3102,11 @@ void qemu_init(int argc, char **argv)
> error_report("Usage: -virtfs fsdriver,mount_tag=tag");
> exit(1);
> }
> + if (strncmp(qemu_opt_get(opts, "fsdriver"), "proxy", 5) ==
> 0) {
> + warn_report("'-virtfs proxy' is deprecated, use "
> + "'-virtfs local' instead");
> + }
> +
> fsdev = qemu_opts_create(qemu_find_opts("fsdev"),
> qemu_opts_id(opts) ?:
> qemu_opt_get(opts, "mount_tag"),
>