Re: [14/17] nfsd: wrong index used in inner loop

[Tim Gardner]

On 03/17/2011 05:00 PM, J. Bruce Fields wrote:
> On Fri, Mar 11, 2011 at 10:21:58PM +0000, Tim Gardner wrote:
> > On 03/11/2011 08:40 PM, Greg KH wrote:
> > > 2.6.32-longterm review patch.  If anyone has any objections, please let us know.
> > >
> > > ------------------
> > >
> > > From: roel<roel.kluin@xxxxxxxxx>
> > >
> > > commit 3ec07aa9522e3d5e9d5ede7bef946756e623a0a0 upstream.
> > >
> > > Index i was already used in the outer loop
> > >
> > > Signed-off-by: Roel Kluin<roel.kluin@xxxxxxxxx>
> > > Signed-off-by: J. Bruce Fields<bfields@xxxxxxxxxx>
> > > Signed-off-by: Greg Kroah-Hartman<gregkh@xxxxxxx>
> > >
> > > ---
> > >   fs/nfsd/nfs4xdr.c |    4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > --- a/fs/nfsd/nfs4xdr.c
> > > +++ b/fs/nfsd/nfs4xdr.c
> > > @@ -1114,7 +1114,7 @@ nfsd4_decode_create_session(struct nfsd4
> > >
> > >   	u32 dummy;
> > >   	char *machine_name;
> > > -	int i;
> > > +	int i, j;
> > >   	int nr_secflavs;
> > >
> > >   	READ_BUF(16);
> > > @@ -1187,7 +1187,7 @@ nfsd4_decode_create_session(struct nfsd4
> > >   			READ_BUF(4);
> > >   			READ32(dummy);
> > >   			READ_BUF(dummy * 4);
> > > -			for (i = 0; i<   dummy; ++i)
> > > +			for (j = 0; j<   dummy; ++j)
> > >   				READ32(dummy);
> > >   			break;
> > >   		case RPC_AUTH_GSS:
> > >
> > >
> > > --
> >
> > I agree that fixing the index in this loop is a good thing, but its
> > caused me to look at the result:
> >
> > for (j = 0; j<   dummy; ++j)
> > 	READ32(dummy);
> >
> > It seems to me that this loop might never terminate if the original
> > buffer is maliciously constructed, e.g., 0, 1, 2, 3, ... Is the data
> > in this buffer really that well vetted?
>
> Agreed, the code's still clearly bogus.  In fact, we can just delete
> that loop entirely; I have a patch queued up to send to Linus soon.
>
> (But go ahead and apply this anyway, and then you'll get the followup
> patch when it lands.)
>
> --b.

Will do. Thanks for the update.

rtg
--
Tim Gardner tim.gardner@xxxxxxxxxxxxx
--
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/