[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 4/4] 9pfs: local: metadata file for the VirtF
From: |
Greg Kurz |
Subject: |
Re: [Qemu-devel] [PATCH v2 4/4] 9pfs: local: metadata file for the VirtFS root |
Date: |
Tue, 23 May 2017 20:47:38 +0200 |
On Tue, 23 May 2017 12:13:17 -0500
Eric Blake <address@hidden> wrote:
> On 05/23/2017 09:32 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 refers
> > 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.
> >
> > @@ -478,7 +504,8 @@ static off_t local_telldir(FsContext *ctx,
> > V9fsFidOpenState *fs)
> >
> > static bool local_is_mapped_file_metadata(FsContext *fs_ctx, const char
> > *name)
> > {
> > - return !strcmp(name, VIRTFS_META_DIR);
> > + return
> > + !strcmp(name, VIRTFS_META_DIR) || !strcmp(name,
> > VIRTFS_META_ROOT_FILE);
>
> We have to block VIRTFS_META_DIR at any depth in the hierarchy, but
> can/should we change the blocking of VIRTFS_META_ROOT_FILE to only
> happen at the root directory, rather than at all directories? On the
> other hand, if you can simultaneously map /path/to/a for one mount
> point, and /path/to/a/b for another, then the root file for B is visible
> at a lower depth than the root file for A, and blocking the metafile
> name everywhere means that the mount A can't influence the behavior on
> the mount for B.
>
I must confess I hadn't this scenario in mind but it's safer from a
security standpoint indeed.
> Not tested, but looks sane, so:
> Reviewed-by: Eric Blake <address@hidden>
>
Thanks again for the review, Eric.
Cheers,
--
Greg
pgpuNEz5R155B.pgp
Description: OpenPGP digital signature