Re: [Qemu-devel] [PATCH 4/5] 9pfs: local: metadata file for the VirtFS root

[Greg Kurz <groug@kaod.org>]

[-- Attachment #1: Type: text/plain, Size: 2369 bytes --]

On Fri, 5 May 2017 12:11:14 -0500
Eric Blake <eblake@redhat.com> wrote:

> On 05/05/2017 09:37 AM, Greg Kurz wrote:
> > When using the mapped-file security, credentials are stored in a metadata
> > directory located in the parent directory. This is okay for all paths with
> > the notable exception of the root path, since we don't want and probably
> > can't create a metadata directory above the virtfs directory on the host.
> > 
> > This patch introduces a dedicated metadata file, sitting in the virtfs root
> > for this purpose. It relies on the fact that the "." name necessarily refer  
> 
> s/refer/refers/
> 

I'll fix this.

> > to the virtfs root.
> > 
> > As for the metadata directory, we don't want the client to see this file.
> > The current code only cares for readdir() but there are many other places
> > to fix actually. The filtering logic is hence put in a separate function.
> > 
> > Before:
> > 
> > # ls -ld
> > drwxr-xr-x. 3 greg greg 4096 May  5 12:49 .
> > # chown root.root .
> > chown: changing ownership of '.': Is a directory
> > # ls -ld
> > drwxr-xr-x. 3 greg greg 4096 May  5 12:49 .
> > 
> > After:
> > 
> > # ls -ld
> > drwxr-xr-x. 3 greg greg 4096 May  5 12:49 .
> > # chown bin.bin .
> > # ls -ld
> > drwxr-xr-x. 3 root root 4096 May  5 12:50 .  
> 
> Huh? How does chown bin.bin result in root.root? Copy-and-paste error?
> 

Oops, I had first tested with bin.bin, then I decided that root.root would
make more sense but I fumbled when updating the changelog... :-\

> > 
> > and from the host:
> > 
> > ls -al .virtfs_metadata_root
> > -rwx------. 1 greg greg 26 May  5 12:50 .virtfs_metadata_root
> > $ cat .virtfs_metadata_root
> > virtfs.uid=0
> > virtfs.gid=0  
> 
> So what happens if the guest tries to manipulate
> ./.virtfs_metadata_root? I know you are filtering it so that readdir()
> and such can't see it, but do you have sane errors for the guest's
> attempt to circumvent those hidden names?
> 

This is addressed by patch 5.

> > 
> > Reported-by: Léo Gaspard <leo@gaspard.io>
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> >  hw/9pfs/9p-local.c |   88 ++++++++++++++++++++++++++++++++++++----------------
> >  1 file changed, 61 insertions(+), 27 deletions(-)
> > 
> > diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c  
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]