lkml.org 
[lkml]   [2011]   [Mar]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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


    \
     
     \ /
      Last update: 2011-03-18 01:57    [W:0.028 / U:0.248 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site