Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754133AbbBTLIJ (ORCPT ); Fri, 20 Feb 2015 06:08:09 -0500 Received: from mail-lb0-f170.google.com ([209.85.217.170]:38975 "EHLO mail-lb0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753677AbbBTLIH (ORCPT ); Fri, 20 Feb 2015 06:08:07 -0500 MIME-Version: 1.0 In-Reply-To: References: <20150217161724.65a9ff577f84917497d61b4e@linux-foundation.org> <20150218074442.GC1752@p183.telecom.by> Date: Fri, 20 Feb 2015 14:08:04 +0300 Message-ID: Subject: Re: Fw: [PATCH] lib/kstrtox.c Stop parsing integer on overflow From: Alexey Dobriyan To: Anshul Garg Cc: Andrew Morton , Linux Kernel Mailing List Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2716 Lines: 72 On Thu, Feb 19, 2015 at 8:53 PM, Anshul Garg wrote: > On Wed, Feb 18, 2015 at 1:14 PM, Alexey Dobriyan wrote: >> On Tue, Feb 17, 2015 at 04:17:24PM -0800, Andrew Morton wrote: >>> ? >>> >>> Begin forwarded message: >>> >>> Date: Mon, 16 Feb 2015 10:48:50 -0800 >>> From: Anshul Garg >>> To: linux-kernel@vger.kernel.org >>> Cc: aksgarg1989@gmail.com, anshul.g@samsung.com, torvalds@linux-foundation.org >>> Subject: [PATCH] lib/kstrtox.c Stop parsing integer on overflow >>> >>> >>> From: Anshul Garg >>> >>> While converting string representation to integer >>> break the loop if overflow is detected. >>> >>> Signed-off-by: Anshul Garg >>> --- >>> lib/kstrtox.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/lib/kstrtox.c b/lib/kstrtox.c >>> index ec8da78..6f30209 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++; >> >> The _notion_ of a patch is OK if you want EVERY simple_strtoull() call >> to stop parsing past overflow right now. It SHOULD have done so from day 1, >> but it doesn't do that. >> >> When I wrote kstrto*() code I deliberatedly didn't break this bug >> because of the sheer number of call sites. >> >> If you are OK with changing bug-for-bug compatibility, >> then patch simply need to delete overflow detection code. >> >> Alexey > > I think this patch won't break any existing module using this function. > because this function sets KSTRTOX_OVERFLOW as error status. > > which is checked by calling function to determine whether value is correct > or not. > > If this flag is set we can simply discard the parsed value. Which is my previous email is all about. If you discard, every single simple_strto*() call will change its behaviour. If you're OK with that, patch is incomplete. Alexey -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/