linux-kernel - Re: [PATCH v4 1/2] rust: kernel: create `overflow

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: <CAJ-ks9mXgEsZbm9yyBBj-z_nYtxB1BcCpEToj5rQp+G1Nmdiag@mail.gmail.com>
Date: Sun, 20 Jul 2025 12:23:40 -0400
From: Tamir Duberstein <tamird@...il.com>
To: Antonio Hickey <contact@...oniohickey.com>
Cc: a.hindborg@...nel.org, alex.gaynor@...il.com, aliceryhl@...gle.com, 
	bjorn3_gh@...tonmail.com, boqun.feng@...il.com, dakr@...nel.org, 
	danielstonecote@...il.com, gary@...yguo.net, linux-kernel@...r.kernel.org, 
	lossin@...nel.org, ojeda@...nel.org, rust-for-linux@...r.kernel.org, 
	tmgross@...ch.edu
Subject: Re: [PATCH v4 1/2] rust: kernel: create `overflow_assert!` macro

On Sun, Jul 20, 2025 at 12:17 PM Antonio Hickey
<contact@...oniohickey.com> wrote:
>
> Tamir Duberstein wrote:
> > On Sat, Jun 28, 2025 at 10:43 PM Antonio Hickey
> > <contact@...oniohickey.com> wrote:
> > > +//! Overflow assertion.
> > > +
> > > +/// Verifies at runtime that an expression is within an expected bound.
> >
> > This is a strange comment, imo. The only thing special about this
> > macro relative to a plain `assert` is that it disabled when
> > CONFIG_RUST_OVERFLOW_CHECKS=n.
>
> Special yes, but it also documents the intent of the `assert`.
> I did include a comment about the conditional behaviour with
> CONFIG_RUST_OVERFLOW_CHECKS being enabled/disabled.
>
> I'm open to rewording this doc comment to be better though.
> I should also add information about the optional panic message
> like the `static_assert` does as well, what about something like:
>
> ```
> /// Overflow assertion.
> ///
> /// Runtime assertion that an expression is within an expected bounds.

I still disprefer this phrasing because there's nothing stopping
someone from writing

overflow_assert!(sky_is_blue());

In other words the type of the expression is bool while the comment
implies that the type of the expression is numeric.

> ///
> /// This macro is only enabled when `CONFIG_RUST_OVERFLOW_CHECKS` is true.
> ///
> /// An optional panic message can be supplied after the expression.
> /// Currently only a string literal without formatting is supported
> /// due to constness limitations of the [`assert!`] macro.

I don't think this is correct; in a non-const context you could use
formatting with this macro, no? IMO the doc comment on `debug_assert`
is good to use as a guide here.

https://doc.rust-lang.org/stable/std/macro.debug_assert.html

Consider also using the same implementation as that macro; there's no
need to separate `cond` from the other arguments.

> ```
>
> Thanks
>
> >
> > > +///
> > > +/// This macro is only active when `CONFIG_RUST_OVERFLOW_CHECKS` is enabled.
>