From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Qu Wenruo <wqu@suse.com>
Cc: linux-btrfs@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>,
Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
David Laight <David.Laight@aculab.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] lib/strtox: introduce kstrtoull_suffix() helper
Date: Wed, 20 Dec 2023 16:17:23 +0200 [thread overview]
Message-ID: <ZYL3c1S7B9PHPEwI@smile.fi.intel.com> (raw)
In-Reply-To: <e042f40ea5cf7fa8251713d5bb7a485f42c5615b.1703030510.git.wqu@suse.com>
On Wed, Dec 20, 2023 at 10:40:00AM +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).
>
> So here we introduce a new helper to address the problem,
> kstrtoull_suffix():
>
> - Enhance _kstrtoull()
> This allow _kstrtoull() to return even if it hits an invalid char, as
> long as the optional parameter @retptr is provided.
>
> If @retptr is provided, _kstrtoull() would try its best to parse the
> valid part, and leave the remaining to be handled by the caller.
>
> If @retptr is not provided, the behavior is not altered.
>
> - New kstrtoull_suffix() helper
> This new helper utilize the new @retptr capability of _kstrtoull(),
> and provides 2 new ability:
>
> * Allow certain suffixes to be chosen
> The recommended suffix list is "KMGTP" (using the new unit_suffix
> enum as a bitmap), excluding the overflow prone "E".
> Undermost cases there is really no need to use "E" suffix anyway.
> And for those who really need that exabytes suffix, they can enable
> that suffix pretty easily.
>
> * Add overflow checks for the suffixes
> If the original number string is fine, but with the extra left
> shift overflow happens, then -EOVERFLOW is returned.
NAK. Read the v1 discussion why.
--
With Best Regards,
Andy Shevchenko
next prev parent reply other threads:[~2023-12-20 14:20 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-20 0:09 [PATCH v2 0/2] lib/kstrtox: introduce kstrtoull_suffix() helper Qu Wenruo
2023-12-20 0:10 ` [PATCH v2 1/2] lib/strtox: " Qu Wenruo
2023-12-20 5:38 ` David Disseldorp
2023-12-23 9:57 ` Qu Wenruo
2023-12-27 5:38 ` David Disseldorp
2023-12-20 14:17 ` Andy Shevchenko [this message]
2023-12-20 0:10 ` [PATCH v2 2/2] btrfs: sysfs: use kstrtoull_suffix() to replace memparse() Qu Wenruo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZYL3c1S7B9PHPEwI@smile.fi.intel.com \
--to=andriy.shevchenko@linux.intel.com \
--cc=David.Laight@aculab.com \
--cc=akpm@linux-foundation.org \
--cc=christophe.jaillet@wanadoo.fr \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=wqu@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox