On Fri, 26 Aug 2016 13:41:23 -0500 Eric Blake <ebl...@redhat.com> wrote:
> On 08/26/2016 10:07 AM, Greg Kurz wrote: > > According to the 9P spec at http://man.cat-v.org/plan_9/5/intro : > > > > Data items of larger or variable lengths are represented by a > > two-byte field specifying a count, n, followed by n bytes of > > data. Text strings are represented this way, with the text > > itself stored as a UTF-8 encoded sequence of Unicode charac- > > ters (see utf(6)). Text strings in 9P messages are not NUL- > > terminated: n counts the bytes of UTF-8 data, which include > > no final zero byte. The NUL character is illegal in all > > text strings in 9P, and is therefore excluded from file > > names, user names, and so on. > > > > With this patch, if a 9P client sends a text string containing a NUL > > character, the request will fail and the client is returned EINVAL. > > > > The checking is done in v9fs_iov_vunmarshal() because it is a convenient > > place to check all client originated strings. > > > > Suggested-by: Peter Maydell <peter.mayd...@linaro.org> > > Signed-off-by: Greg Kurz <gr...@kaod.org> > > --- > > fsdev/9p-iov-marshal.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/fsdev/9p-iov-marshal.c b/fsdev/9p-iov-marshal.c > > index 663cad542900..9bcdc370231d 100644 > > --- a/fsdev/9p-iov-marshal.c > > +++ b/fsdev/9p-iov-marshal.c > > @@ -127,7 +127,12 @@ ssize_t v9fs_iov_vunmarshal(struct iovec *out_sg, int > > out_num, size_t offset, > > str->size); > > if (copied > 0) { > > str->data[str->size] = 0; > > - } else { > > + /* 9P forbids NUL characters in all text strings */ > > + if (strlen(str->data) != str->size) { > > If this were glibc, we could micro-optimize and do: > > if (rawmemchr(str->data, 0) != str->data + str->size) > > so that strlen() doesn't have to visit the tail end of the string if a > NUL is present early. But your code is just fine as-is, and doesn't Hmmm... if a NUL is present early, why would strlen() visit the tail end of the string ? Looking at the glibc sources (string/strlen.c and string/rawmemchr.c), both calls share the same implementation: handle initial characters 1 by 1 and then test a longword at a time... and FWIW, strlen() knows at compile time it looks for 0 instead of a runtime character for rawmemchr(). I have the feeling that strlen() is the more optimized actually. > have to worry about rawmemchr() being present. > > Reviewed-by: Eric Blake <ebl...@redhat.com> >
pgpQdezjOPJzi.pgp
Description: OpenPGP digital signature