[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8095c6ae5f8d412d8e6ff95707961a08@AcuMS.aculab.com>
Date: Tue, 19 Dec 2023 16:42:11 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'David Disseldorp' <ddiss@...e.de>, Qu Wenruo <wqu@...e.com>
CC: "linux-btrfs@...r.kernel.org" <linux-btrfs@...r.kernel.org>, Andrew Morton
<akpm@...ux-foundation.org>, Christophe JAILLET
<christophe.jaillet@...adoo.fr>, Andy Shevchenko
<andriy.shevchenko@...ux.intel.com>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 1/2] lib/strtox: introduce kstrtoull_suffix() helper
From: David Disseldorp
> Sent: 18 December 2023 13:00
>
> On Fri, 15 Dec 2023 19:09:23 +1030, Qu Wenruo wrote:
>
> > Just as mentioned in the comment of memparse(), the simple_stroull()
> > usage can lead to overflow all by itself.
> >
> > Furthermore, the suffix calculation is also super overflow prone because
> > that some suffix like "E" itself would eat 60bits, leaving only 4 bits
> > available.
> >
> > And that suffix "E" can also lead to confusion since it's using the same
> > char of hex Ox'E'.
> >
> > One simple example to expose all the problem is to use memparse() on
> > "25E".
> > The correct value should be 28823037615171174400, but the suffix E makes
> > it super simple to overflow, resulting the incorrect value
> > 10376293541461622784 (9E).
Some more bikeshed paint :-)
...
> > + ret = _kstrtoull(s, base, &init_value, &endptr);
> > + /* Either already overflow or no number string at all. */
> > + if (ret < 0)
> > + return ret;
> > + final_value = init_value;
> > + /* No suffixes. */
> > + if (!*endptr)
> > + goto done;
How about:
suffix = *endptr;
if (!strchr(suffixes, suffix))
return -ENIVAL;
shift = strcspn("KkMmGgTtPp", suffix)/2 * 10 + 10;
if (shift > 50)
return -EINVAL;
if (value >> (64 - shift))
return -EOVERFLOW;
value <<= shift;
Although purists might want to multiply by 1000 not 1024.
And SI multipliers are all upper-case - except k.
...
> > + /* Overflow check. */
> > + if (final_value < init_value)
> > + return -EOVERFLOW;
That is just plain wrong.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Powered by blists - more mailing lists