linux-kernel - Re: [PATCH v12 1/3] rust: str: add radix prefixed integer parsing functions

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] [day] [month] [year] [list]

Message-Id: <D9PW6AGXCWM8.31IPHEJLS9SNZ@kernel.org>
Date: Wed, 07 May 2025 13:29:03 +0200
From: "Benno Lossin" <lossin@...nel.org>
To: "Andreas Hindborg" <a.hindborg@...nel.org>
Cc: "Miguel Ojeda" <ojeda@...nel.org>, "Alex Gaynor"
 <alex.gaynor@...il.com>, "Boqun Feng" <boqun.feng@...il.com>, "Gary Guo"
 <gary@...yguo.net>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, "Benno Lossin" <benno.lossin@...ton.me>, "Alice
 Ryhl" <aliceryhl@...gle.com>, "Masahiro Yamada" <masahiroy@...nel.org>,
 "Nathan Chancellor" <nathan@...nel.org>, "Luis Chamberlain"
 <mcgrof@...nel.org>, "Danilo Krummrich" <dakr@...nel.org>, "Nicolas Schier"
 <nicolas.schier@...ux.dev>, "Trevor Gross" <tmgross@...ch.edu>, "Adam
 Bratschi-Kaye" <ark.email@...il.com>, <rust-for-linux@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>, <linux-kbuild@...r.kernel.org>, "Petr
 Pavlu" <petr.pavlu@...e.com>, "Sami Tolvanen" <samitolvanen@...gle.com>,
 "Daniel Gomez" <da.gomez@...sung.com>, "Simona Vetter"
 <simona.vetter@...ll.ch>, "Greg KH" <gregkh@...uxfoundation.org>, "Fiona
 Behrens" <me@...enk.dev>, "Daniel Almeida" <daniel.almeida@...labora.com>,
 <linux-modules@...r.kernel.org>
Subject: Re: [PATCH v12 1/3] rust: str: add radix prefixed integer parsing
 functions

On Wed May 7, 2025 at 11:15 AM CEST, Andreas Hindborg wrote:
> "Benno Lossin" <lossin@...nel.org> writes:
>> On Tue May 6, 2025 at 3:02 PM CEST, Andreas Hindborg wrote:
>>> +    pub trait ParseInt: private::FromStrRadix + TryFrom<u64> {
>>> +        /// Parse a string according to the description in [`Self`].
>>> +        fn from_str(src: &BStr) -> Result<Self> {
>>> +            match src.deref() {
>>> +                [b'-', rest @ ..] => {
>>> +                    let (radix, digits) = strip_radix(rest.as_ref());
>>> +                    // 2's complement values range from -2^(b-1) to 2^(b-1)-1.
>>> +                    // So if we want to parse negative numbers as positive and
>>> +                    // later multiply by -1, we have to parse into a larger
>>> +                    // integer. We choose `u64` as sufficiently large.
>>> +                    //
>>> +                    // NOTE: 128 bit integers are not available on all
>>> +                    // platforms, hence the choice of 64 bits.
>>> +                    let val = u64::from_str_radix(
>>> +                        core::str::from_utf8(digits).map_err(|_| EINVAL)?,
>>> +                        radix,
>>> +                    )
>>> +                    .map_err(|_| EINVAL)?;
>>> +
>>> +                    if val > Self::abs_min() {
>>> +                        return Err(EINVAL);
>>> +                    }
>>> +
>>> +                    if val == Self::abs_min() {
>>> +                        return Ok(Self::MIN);
>>> +                    }
>>> +
>>> +                    // SAFETY: We checked that `val` will fit in `Self` above.
>>> +                    let val: Self = unsafe { val.try_into().unwrap_unchecked() };
>>> +
>>> +                    Ok(val.complement())
>>
>> You're allowing to parse `u32` with a leading `-`? I'd expect an error
>> in that case. Maybe `complement` should be named `negate` and return a
>> `Result`?
>
> You would get `Err(EINVAL)` in that case, hitting this:
>
>   if val > Self::abs_min() {
>       return Err(EINVAL);
>   }

Ah, I think I asked this in a previous version already... Thanks.

---
Cheers,
Benno