Re: [Qemu-devel] [PATCH] 9pfs: deprecate handle backend

[Greg Kurz <groug@kaod.org>]

On Mon, 18 Dec 2017 14:19:46 +0000
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> On Mon, Dec 18, 2017 at 02:46:00PM +0100, Greg Kurz wrote:
> > This backend raise some concerns:
> > 
> > - doesn't support symlinks
> > - fails +100 tests in the PJD POSIX file system test suite [1]
> > - requires the QEMU process to run with the CAP_DAC_READ_SEARCH
> >   capability, which isn't recommended for security reasons
> > 
> > For all these reasons, the handle backend is now deprecated.
> > 
> > [1] https://www.tuxera.com/community/posix-test-suite/
> > 
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> > 
> > Aneesh,
> > 
> > Even if I see the benefit of using file handles in a userspace file
> > server, the handle backend still has flaws that make it hardly usable
> > IMHO. Also I haven't received anything about it in years. All users
> > and contributors seem to stick to the local backend.
> > 
> > My guess is that nobody uses the handle backend, and unless I'm missing
> > something, it wouldn't hurt to drop it. My motivation is to reduce the
> > number of lines that I don't really have time/motivation to maintain,
> > and that could be subject to a CVE in the future.
> > 
> > Any thoughts ?
> > ---
> >  hw/9pfs/9p-handle.c |    2 ++
> >  qemu-doc.texi       |    8 ++++++++
> >  2 files changed, 10 insertions(+)
> > 
> > diff --git a/hw/9pfs/9p-handle.c b/hw/9pfs/9p-handle.c
> > index 9875f1894cc5..1291a2db6782 100644
> > --- a/hw/9pfs/9p-handle.c
> > +++ b/hw/9pfs/9p-handle.c
> > @@ -657,6 +657,8 @@ static int handle_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
> >      const char *sec_model = qemu_opt_get(opts, "security_model");
> >      const char *path = qemu_opt_get(opts, "path");
> >  
> > +    warn_report("handle backend is deprecated");
> > +
> >      if (sec_model) {
> >          error_report("Invalid argument security_model specified with handle fsdriver");
> >          return -1;
> > diff --git a/qemu-doc.texi b/qemu-doc.texi
> > index f7317dfc66cd..bf44e2752cb2 100644
> > --- a/qemu-doc.texi
> > +++ b/qemu-doc.texi
> > @@ -2509,6 +2509,14 @@ default channel subsystem image for guests that do not support multiple
> >  channel subsystems, all devices can be put into the default channel
> >  subsystem image.
> >  
> > +@subsection -fsdev handle (since 2.12.0)
> > +
> > +The ``handle'' fsdev backend does not support symlinks and causes the 9p
> > +filesystem in the guest to fail a fair amount of tests from the PJD POSIX
> > +filesystem test suite. Also it requires the CAP_DAC_READ_SEARCH capability,
> > +which is not the recommended way to run QEMU. This backend should not be
> > +used and it will be removed with no replacement.
> > +  
> 
> I would suggest a slight teak to the last sentance.
> 
>  "This backend should not be used and wil be removed. The 'local' backend
>   is the recommended alternative"
> 

Good idea. I'll just do that.

> Regardless of whether you include this wording change though:
> 
>   Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
> 

Thanks !

> 
> Regards,
> Daniel