On Fri, 23 Jun 2023 14:41:15 +0200 Christian Schoenebeck <qemu_...@crudebyte.com> 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 (preferably > in conjunction with its 'mapped' security model) instead, but now it is > time to officially deprecate the 'proxy' backend. > > Signed-off-by: Christian Schoenebeck <qemu_...@crudebyte.com> > --- > v3 -> v4: > - MAINTAINERS: also move virtfs-proxy-helper.rst to 'obsolete' section > - deprecated.rst: suggest virtiofsd as alternative. > - deprecated.rst: mention a considerable future reimplementation of > 'proxy' using 'vhost'. > - QEMU runtime warning: merge deprecation warnings of '-virtfs proxy' and > '-fsdev proxy' into a single deprecation warning and mention virtiofsd > as alternative. > - virtfs-proxy-helper daemon: print runtime deprecation warning here as > well. > - commit log: mention 'mapped' security model. > > MAINTAINERS | 9 ++++++++- > docs/about/deprecated.rst | 23 +++++++++++++++++++++++ > docs/tools/virtfs-proxy-helper.rst | 3 +++ > fsdev/qemu-fsdev.c | 8 ++++++++ > fsdev/virtfs-proxy-helper.c | 9 +++++++++ > hw/9pfs/9p-proxy.c | 5 +++++ > hw/9pfs/9p-proxy.h | 5 +++++ > meson.build | 2 +- > qemu-options.hx | 6 +++++- > 9 files changed, 67 insertions(+), 3 deletions(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 436b3f0afe..3aa70b5c21 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/ > -F: docs/tools/virtfs-proxy-helper.rst > +X: fsdev/virtfs-proxy-helper.c > 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 > +F: docs/tools/virtfs-proxy-helper.rst > +S: Obsolete > + > virtio-blk > M: Stefan Hajnoczi <stefa...@redhat.com> > L: qemu-bl...@nongnu.org > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > index 0743459862..4ce75722f3 100644 > --- a/docs/about/deprecated.rst > +++ b/docs/about/deprecated.rst > @@ -343,6 +343,29 @@ 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 (along with its proxy helper daemon) in a future version of QEMU. > Please > +use ``-fsdev local`` or ``-virtfs local`` for using the 9p ``local`` > filesystem > +backend instead, or alternatively consider deploying virtiofsd instead. Maybe drop the first "instead" to avoid repetition ? > + > +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 by using its 'mapped' security model option. > + > +Nowadays it would make sense to reimplement the ``proxy`` backend by using > +QEMU's ``vhost`` feature, which would eliminate the high latency costs under > +which the 9p ``proxy`` backend currently suffers. However as of to date > nobody > +has indicated plans for such kind of reimplemention unfortunately. > + > > Block device options > '''''''''''''''''''' > diff --git a/docs/tools/virtfs-proxy-helper.rst > b/docs/tools/virtfs-proxy-helper.rst > index 6cdeedf8e9..bd310ebb07 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' backend 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..9a50ee370b 100644 > --- a/fsdev/qemu-fsdev.c > +++ b/fsdev/qemu-fsdev.c > @@ -133,6 +133,14 @@ int qemu_fsdev_add(QemuOpts *opts, Error **errp) > } > > if (fsdriver) { > + if (strncmp(fsdriver, "proxy", 5) == 0) { > + warn_report( > + "'-fsdev proxy' and '-virtfs proxy' are deprecated, use " > + "'local' instead of 'proxy, or consider deploying virtiofsd " > + "instead" Ditto. LGTM. Reviewed-by: Greg Kurz <gr...@kaod.org> > + ); > + } > + > 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..144aaf585a 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' backend 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> > @@ -1057,6 +1062,10 @@ int main(int argc, char **argv) > struct statfs st_fs; > #endif > > + fprintf(stderr, "NOTE: The 9p 'proxy' backend is deprecated (since " > + "QEMU 8.1) and will be removed in a future version of " > + "QEMU!\n"); > + > prog_name = g_path_get_basename(argv[0]); > > is_daemon = true; > diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c > index 99d115ff0d..905cae6992 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' backend 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..9be4718d3e 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' backend is deprecated (since QEMU 8.1) and will be > + * removed in a future version of QEMU! > + */ > + > #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. -- Greg