When using the mapped-file security mode, we shouldn't let the client
mess with the metadata. The current code already hides it but the
client can still access the metadata through several operations.

This patch fixes the issue by:
- preventing the creation of fids pointing to the metadata (name_to_path)
- failing various operations taking a dirpath and a name arguments if
  name is a metadata reserved name

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/9pfs/9p-local.c |   41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index b427d2928800..93cadac302c9 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -588,6 +588,11 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
     int err = -1;
     int dirfd;
 
+    if (local_must_skip_metadata(fs_ctx, name)) {
+        errno = EINVAL;
+        return -1;
+    }
+
     dirfd = local_opendir_nofollow(fs_ctx, dir_path->data);
     if (dirfd == -1) {
         return -1;
@@ -634,6 +639,11 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path,
     int err = -1;
     int dirfd;
 
+    if (local_must_skip_metadata(fs_ctx, name)) {
+        errno = EINVAL;
+        return -1;
+    }
+
     dirfd = local_opendir_nofollow(fs_ctx, dir_path->data);
     if (dirfd == -1) {
         return -1;
@@ -723,6 +733,11 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name,
     int err = -1;
     int dirfd;
 
+    if (local_must_skip_metadata(fs_ctx, name)) {
+        errno = EINVAL;
+        return -1;
+    }
+
     /*
      * Mark all the open to not follow symlinks
      */
@@ -781,6 +796,11 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath,
     int err = -1;
     int dirfd;
 
+    if (local_must_skip_metadata(fs_ctx, name)) {
+        errno = EINVAL;
+        return -1;
+    }
+
     dirfd = local_opendir_nofollow(fs_ctx, dir_path->data);
     if (dirfd == -1) {
         return -1;
@@ -855,6 +875,11 @@ static int local_link(FsContext *ctx, V9fsPath *oldpath,
     int ret = -1;
     int odirfd, ndirfd;
 
+    if (local_must_skip_metadata(ctx, name)) {
+        errno = EINVAL;
+        return -1;
+    }
+
     odirfd = local_opendir_nofollow(ctx, odirpath);
     if (odirfd == -1) {
         goto out;
@@ -983,6 +1008,11 @@ static int local_unlinkat_common(FsContext *ctx, int dirfd, const char *name,
 {
     int ret = -1;
 
+    if (local_must_skip_metadata(ctx, name)) {
+        errno = EINVAL;
+        return -1;
+    }
+
     if (ctx->export_flags & V9FS_SM_MAPPED_FILE) {
         int map_dirfd;
 
@@ -1125,6 +1155,11 @@ static int local_lremovexattr(FsContext *ctx, V9fsPath *fs_path,
 static int local_name_to_path(FsContext *ctx, V9fsPath *dir_path,
                               const char *name, V9fsPath *target)
 {
+    if (local_must_skip_metadata(ctx, name)) {
+        errno = EINVAL;
+        return -1;
+    }
+
     if (dir_path) {
         if (!strcmp(name, ".")) {
             /* "." relative to "foo/bar" is "foo/bar" */
@@ -1161,6 +1196,12 @@ static int local_renameat(FsContext *ctx, V9fsPath *olddir,
     int ret;
     int odirfd, ndirfd;
 
+    if (local_must_skip_metadata(ctx, old_name) ||
+        local_must_skip_metadata(ctx, new_name)) {
+        errno = EINVAL;
+        return -1;
+    }
+
     odirfd = local_opendir_nofollow(ctx, olddir->data);
     if (odirfd == -1) {
         return -1;

On 05/05/2017 09:37 AM, Greg Kurz wrote:
> When using the mapped-file security mode, we shouldn't let the client
> mess with the metadata. The current code already hides it but the
> client can still access the metadata through several operations.
> 
> This patch fixes the issue by:
> - preventing the creation of fids pointing to the metadata (name_to_path)
> - failing various operations taking a dirpath and a name arguments if
>   name is a metadata reserved name
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  hw/9pfs/9p-local.c |   41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index b427d2928800..93cadac302c9 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -588,6 +588,11 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
>      int err = -1;
>      int dirfd;
>  
> +    if (local_must_skip_metadata(fs_ctx, name)) {
> +        errno = EINVAL;
> +        return -1;
> +    }
> +

I don't know if EINVAL is the best error, but it seems reasonable enough.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

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

> On 05/05/2017 09:37 AM, Greg Kurz wrote:
> > When using the mapped-file security mode, we shouldn't let the client
> > mess with the metadata. The current code already hides it but the
> > client can still access the metadata through several operations.
> > 
> > This patch fixes the issue by:
> > - preventing the creation of fids pointing to the metadata (name_to_path)
> > - failing various operations taking a dirpath and a name arguments if
> >   name is a metadata reserved name
> > 
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> >  hw/9pfs/9p-local.c |   41 +++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> > 
> > diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> > index b427d2928800..93cadac302c9 100644
> > --- a/hw/9pfs/9p-local.c
> > +++ b/hw/9pfs/9p-local.c
> > @@ -588,6 +588,11 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
> >      int err = -1;
> >      int dirfd;
> >  
> > +    if (local_must_skip_metadata(fs_ctx, name)) {
> > +        errno = EINVAL;
> > +        return -1;
> > +    }
> > +  
> 
> I don't know if EINVAL is the best error, but it seems reasonable enough.
> 

I admit that I'm not really a big fan of returning EINVAL, but there's
nothing like "this file name is illegal" on Linux and I couldn't come
up with a better error...