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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4D7AA086.3030500@canonical.com>
Date:	Fri, 11 Mar 2011 22:21:58 +0000
From:	Tim Gardner <tim.gardner@...onical.com>
To:	Greg KH <gregkh@...e.de>, alan@...rguk.ukuu.org.uk,
	Roel Kluin <roel.kluin@...il.com>,
	"J. Bruce Fields" <bfields@...hat.com>
CC:	linux-kernel@...r.kernel.org, stable@...nel.org,
	stable-review@...nel.org, torvalds@...ux-foundation.org,
	akpm@...ux-foundation.org
Subject: Re: [14/17] nfsd: wrong index used in inner loop

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@...il.com>
>
> commit 3ec07aa9522e3d5e9d5ede7bef946756e623a0a0 upstream.
>
> Index i was already used in the outer loop
>
> Signed-off-by: Roel Kluin<roel.kluin@...il.com>
> Signed-off-by: J. Bruce Fields<bfields@...hat.com>
> Signed-off-by: Greg Kroah-Hartman<gregkh@...e.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?

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