Messages in this thread | | | Date | Thu, 17 Mar 2011 18:55:29 -0600 | From | Tim Gardner <> | Subject | Re: [14/17] nfsd: wrong index used in inner loop |
| |
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@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. >
Will do. Thanks for the update.
rtg -- Tim Gardner tim.gardner@canonical.com
| |