Re: [PATCH] nfs client, kernel 2.4.31: readlink result overflow

[Marcelo Tosatti]

On Tue, Sep 13, 2005 at 02:52:44PM -0400, Assar wrote:
> Hi, Marcelo.
> 
> Marcelo Tosatti <marcelo.tosatti@xxxxxxxxxxxx> writes:
> > > diff -u linux-2.4.31.orig/fs/nfs/nfs2xdr.c linux-2.4.31/fs/nfs/nfs2xdr.c
> > > --- linux-2.4.31.orig/fs/nfs/nfs2xdr.c	2002-11-28 18:53:15.000000000 -0500
> > > +++ linux-2.4.31/fs/nfs/nfs2xdr.c	2005-09-12 16:12:30.000000000 -0400
> > > @@ -571,8 +571,8 @@
> > >  	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(*strlen) - 1)
> > > +		len = rcvbuf->page_len - sizeof(*strlen) - 1;
> > 
> > So the problem is that the "len" variable encapsulated in (u32 *)rcvbuf->pages[0]
> > does not account for its own length (4 bytes)? 
> 
> That's one problem.
> 
> > If thats the reason, you don't need the "-1" there?
> 
> It also writes a 0 byte.  I think it looks like this:
> 
> ---- ------------ -
> len  string...    0

If an overflow happens (len > rcvbuf->page_len) the last character will get 
truncated anyway, so there is no need for the "-1" AFAICS.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/