Re: [Qemu-devel] [PATCH v4 1/3] 9pfs: forbid illegal path names

[Greg Kurz]

On Tue, 30 Aug 2016 13:03:40 -0500
Eric Blake <address@hidden> wrote:

> On 08/30/2016 12:11 PM, Greg Kurz wrote:
> > Empty path components don't make sense for most commands and may cause
> > undefined behavior, depending on the backend.
> > 
> > Also, the walk request described in the 9P spec [1] clearly shows that
> > the client is supposed to send individual path components: the official
> > linux client never sends portions of path containing the / character for
> > example.
> > 
> > Moreover, the 9P spec [2] also states that a system can decide to restrict
> > the set of supported characters used in path components, with an explicit
> > mention "to remove slashes from name components".
> > 
> > This patch introduces a new name_is_illegal() helper that checks the
> > names sent by the client are not empty and don't contain unwanted chars.
> > Since 9pfs is only supported on linux hosts, only the / character is
> > checked at the moment. When support for other hosts (AKA. win32) is added,
> > other chars may need to be blacklisted as well.
> > 
> > If a client sends an illegal path component, the request will fail and
> > ENOENT is returned to the client.
> > 
> > [1] http://man.cat-v.org/plan_9/5/walk
> > [2] http://man.cat-v.org/plan_9/5/intro
> > 
> > Suggested-by: Peter Maydell <address@hidden>
> > Signed-off-by: Greg Kurz <address@hidden>
> > ---
> > v4: dropped the checking of the symbolic link target name: because a target
> >     can be a full path and thus contain '/' and linux already complains if
> >     it is an empty string. When the symlink gets dereferenced, slashes are
> >     interpreted as the usual path component separator.  
> 
> Can a symlink to "/foo" be used to escape the root (by being absolute

No it can't because the target isn't a actually a file name but a string that
will be translated to a path when the link is dereferenced. And all other
requests with a file name argument that could have some unwanted effect don't
allow '/' in file names.

> instead of relative)?  However, if the answer to that question requires
> more code, I'm fine with it being a separate patch.  So for this email,
> 
> Reviewed-by: Eric Blake <address@hidden>
>

pgpBJwOMiUwxv.pgp
Description: OpenPGP digital signature