On Fri, 26 Aug 2016 13:33:28 -0500 Eric Blake <ebl...@redhat.com> wrote:
> On 08/26/2016 10:07 AM, Greg Kurz wrote: > > Empty path components don't make sense 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 > > EINVAL 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 <peter.mayd...@linaro.org> > > Signed-off-by: Greg Kurz <gr...@kaod.org> > > --- > > hw/9pfs/9p.c | 56 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 56 insertions(+) > > > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > > index b6b02b46a9da..dba11773699b 100644 > > --- a/hw/9pfs/9p.c > > +++ b/hw/9pfs/9p.c > > @@ -1256,6 +1256,11 @@ static int v9fs_walk_marshal(V9fsPDU *pdu, uint16_t > > nwnames, V9fsQID *qids) > > return offset; > > } > > > > +static bool name_is_illegal(const char *name) > > +{ > > + return name == NULL || strchr(name, '/') != NULL; > > Is anyone actually passing NULL? And the commit message says you are > blacklisting the empty string "", but that is not what you did here. > Depending on whether callers can even pass NULL, you may be able to just > s/name == NULL/!*name/ > Oops you're right, v9fs_iov_vunmarshal() always allocates and sets str->data, even for empty strings... so I guess this cannot be call with NULL and your suggestion is the way to go.
pgpXIybw1utFp.pgp
Description: OpenPGP digital signature
