On Fri, 5 May 2017 11:59:15 -0500 Eric Blake <ebl...@redhat.com> wrote:
> 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/
>
I'll fix this.
> > that it can be passed to "*at()" syscalls ('/' would mean the host root).
> >
> > Signed-off-by: Greg Kurz <gr...@kaod.org>
> > ---
> > 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 '..'?
>
My understanding is that symlinks are supposed to be resolved by the client,
and we shouldn't follow them in the server.
> > + 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;
> > }
> >
> >
>
pgpYKiH5CO5Ju.pgp
Description: OpenPGP digital signature
