Re: [PATCH] nfs client: handle long symlinks properly

[Assar <assar@permabit.com>]

Trond Myklebust <trond.myklebust@fys.uio.no> writes:
> > diff --git a/fs/nfs/nfs2xdr.c b/fs/nfs/nfs2xdr.c
> > --- a/fs/nfs/nfs2xdr.c
> > +++ b/fs/nfs/nfs2xdr.c
> > @@ -571,8 +571,11 @@ nfs_xdr_readlinkres(struct rpc_rqst *req
> >         strlen = (u32*)kmap(rcvbuf->pages[0]);
> >         /* Convert length of symlink */
> >         len = ntohl(*strlen);
> > -       if (len > rcvbuf->page_len)
> > -               len = rcvbuf->page_len;
> > +       if (len >= rcvbuf->page_len - sizeof(u32) || len > NFS2_MAXPATHLEN) {
> 
> Shouldn't that be
> 
> 	if (len > rcvbuf->page_len - sizeof(u32) || len > NFS2_MAXPATHLEN)
> 
> ? As long as we use page_len == PAGE_SIZE, we probably don't care, but
> if someone some day decides to set a different value for page_len, then
> we want to make sure that we don't end up overflowing the buffer when we
> NUL-terminate.

Wouldn't len == rcvbuf->page_len - sizeof(u32) mean that there isn't
room for writing the terminating NUL?