Re: [Qemu-devel] [PATCH 2/5] 9pfs: local: resolve special directories in paths

[Eric Blake]

On 05/05/2017 09:37 AM, Greg Kurz wrote:
> When using the mapped-file security mode, the creds of a path /foo/bar
> are stored in the /foo/.virtfs_metadata/bar file. This is okay for all
> paths unless they end with '.' or '..', because we cannot create the
> corresponding file in the metadata directory.
> 
> This patch ensures that '.' and '..' are resolved in all paths.
> 
> The core code only passes path elements (no '/') to the backend, with
> the notable exception of the '/' path, which refers to the virtfs root.
> This patch preserve the current behavior of converting it to '.' so

s/preserve/preserves/

> that it can be passed to "*at()" syscalls ('/' would mean the host root).
> 
> Signed-off-by: Greg Kurz <address@hidden>
> ---
>  hw/9pfs/9p-local.c |   30 +++++++++++++++++++++++-------
>  1 file changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index f3ebca4f7a56..92262f3c3e37 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -1097,14 +1097,30 @@ static int local_name_to_path(FsContext *ctx, 
> V9fsPath *dir_path,
>                                const char *name, V9fsPath *target)
>  {
>      if (dir_path) {
> -        v9fs_path_sprintf(target, "%s/%s", dir_path->data, name);
> -    } else if (strcmp(name, "/")) {
> -        v9fs_path_sprintf(target, "%s", name);
> +        if (!strcmp(name, ".")) {
> +            /* "." relative to "foo/bar" is "foo/bar" */
> +            v9fs_path_copy(target, dir_path);
> +        } else if (!strcmp(name, "..")) {
> +            if (!strcmp(dir_path->data, ".")) {
> +                /* ".." relative to the root is "." */
> +                v9fs_path_sprintf(target, ".");
> +            } else {
> +                char *tmp = g_path_get_dirname(dir_path->data);
> +                /* ".." relative to "foo/bar" is equivalent to "foo" */

True only if bar is not a symlink to some other directory.  What
guarantees do you have that you are not going to be inadvertently
skipping a traversal through symlinks and thereby picking the wrong
location for '..'?

> +                v9fs_path_sprintf(target, "%s", tmp);
> +                g_free(tmp);
> +            }
> +        } else {
> +            assert(!strchr(name, '/'));
> +            v9fs_path_sprintf(target, "%s/%s", dir_path->data, name);
> +        }
> +    } else if (!strcmp(name, "/") || !strcmp(name, ".") ||
> +               !strcmp(name, "..")) {
> +            /* This is the root fid */
> +        v9fs_path_sprintf(target, ".");
>      } else {
> -        /* We want the path of the export root to be relative, otherwise
> -         * "*at()" syscalls would treat it as "/" in the host.
> -         */
> -        v9fs_path_sprintf(target, "%s", ".");
> +        assert(!strchr(name, '/'));
> +        v9fs_path_sprintf(target, "./%s", name);
>      }
>      return 0;
>  }
> 
> 

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

signature.asc
Description: OpenPGP digital signature