Messages in this thread Patch in this message | | | Date | Thu, 17 Mar 2011 19:00:12 -0400 | From | "J. Bruce Fields" <> | Subject | Re: [14/17] nfsd: wrong index used in inner loop |
| |
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@gmail.com> > > > >commit 3ec07aa9522e3d5e9d5ede7bef946756e623a0a0 upstream. > > > >Index i was already used in the outer loop > > > >Signed-off-by: Roel Kluin<roel.kluin@gmail.com> > >Signed-off-by: J. Bruce Fields<bfields@redhat.com> > >Signed-off-by: Greg Kroah-Hartman<gregkh@suse.de> > > > >--- > > 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.
| |