linux-kernel - Re: [PATCH] lib/kstrtox.c break if overflow is detected

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: <20150121193909.GA15481@linux.com>
Date:	Wed, 21 Jan 2015 20:39:09 +0100
From:	Levente Kurusa <levex@...ux.com>
To:	Anshul Garg <aksgarg1989@...il.com>
Cc:	akpm@...ux-foundation.org, felipe.contreras@...il.com,
	linux-kernel@...r.kernel.org, anshul.g@...sung.com
Subject: Re: [PATCH] lib/kstrtox.c break if overflow is detected

Hi,

On Wed, Jan 21, 2015 at 11:26:26AM -0800, Anshul Garg wrote:
> From: Anshul Garg <aksgarg1989@...il.com>
> 
> 1. While converting string representation to integer
> break the loop if overflow is detected.
> 2. Clean kstrtoll function
> 
> Signed-off-by: Anshul Garg <aksgarg1989@...il.com>
> ---
>  lib/kstrtox.c |   28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/lib/kstrtox.c b/lib/kstrtox.c
> index ec8da78..8cbe5ca 100644
> --- a/lib/kstrtox.c
> +++ b/lib/kstrtox.c
> @@ -70,8 +70,10 @@ unsigned int _parse_integer(const char *s, unsigned int base, unsigned long long
>  		 * it in the max base we support (16)
>  		 */
>  		if (unlikely(res & (~0ull << 60))) {
> -			if (res > div_u64(ULLONG_MAX - val, base))
> +			if (res > div_u64(ULLONG_MAX - val, base)) {
>  				overflow = 1;
> +				break;
> +			}
>  		}
>  		res = res * base + val;
>  		rv++;
> @@ -146,23 +148,19 @@ EXPORT_SYMBOL(kstrtoull);
>  int kstrtoll(const char *s, unsigned int base, long long *res)
>  {
>  	unsigned long long tmp;
> -	int rv;
> +	int rv, sign = 1;
>  
>  	if (s[0] == '-') {
> -		rv = _kstrtoull(s + 1, base, &tmp);
> -		if (rv < 0)
> -			return rv;
> -		if ((long long)(-tmp) >= 0)
> -			return -ERANGE;
> -		*res = -tmp;
> -	} else {
> -		rv = kstrtoull(s, base, &tmp);
> -		if (rv < 0)
> -			return rv;
> -		if ((long long)tmp < 0)
> -			return -ERANGE;
> -		*res = tmp;
> +		sign = -1;
> +		s++;
>  	}
> +
> +	rv = kstrtoull(s, base, &tmp);
> +	if (rv < 0)
> +		return rv;
> +	if ((long long)tmp < 0)
> +		return -ERANGE;
> +	*res = sign * tmp;
>  	return 0;
>  }
>  EXPORT_SYMBOL(kstrtoll);

Looks correct to me, so:

Reviewed-by: Levente Kurusa <levex@...ux.com>

But I believe the two hunks are completely unrelated to
eachother and hence should split into a patch series.

Could you please do that and resend? Or if Andrew is OK
with picking it up as is, then it's fine.

Thanks,
    Levente.

Download attachment "signature.asc" of type "application/pgp-signature" (491 bytes)