linux-kernel - Re: [PATCH v1 1/2] rust: add udelay() function

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: <20250827.114327.709618113348118831.fujita.tomonori@gmail.com>
Date: Wed, 27 Aug 2025 11:43:27 +0900 (JST)
From: FUJITA Tomonori <fujita.tomonori@...il.com>
To: daniel.almeida@...labora.com
Cc: fujita.tomonori@...il.com, a.hindborg@...nel.org,
 alex.gaynor@...il.com, ojeda@...nel.org, aliceryhl@...gle.com,
 anna-maria@...utronix.de, bjorn3_gh@...tonmail.com, boqun.feng@...il.com,
 dakr@...nel.org, frederic@...nel.org, gary@...yguo.net,
 jstultz@...gle.com, linux-kernel@...r.kernel.org, lossin@...nel.org,
 lyude@...hat.com, rust-for-linux@...r.kernel.org, sboyd@...nel.org,
 tglx@...utronix.de, tmgross@...ch.edu, acourbot@...dia.com
Subject: Re: [PATCH v1 1/2] rust: add udelay() function

On Tue, 26 Aug 2025 09:44:27 -0300
Daniel Almeida <daniel.almeida@...labora.com> wrote:

>> +/// Inserts a delay based on microseconds with busy waiting.
>> +///
>> +/// Equivalent to the C side [`udelay()`], which delays in microseconds.
>> +///
>> +/// `delta` must be within `[0, `MAX_UDELAY_MS`]` in milliseconds;
>> +/// otherwise, it is erroneous behavior. That is, it is considered a bug to
>> +/// call this function with an out-of-range value, in which case the function
>> +/// will insert a delay for at least the maximum value in the range and
>> +/// may warn in the future.
>> +///
>> +/// The behavior above differs from the C side [`udelay()`] for which out-of-range
>> +/// values could lead to an overflow and unexpected behavior.
>> +///
>> +/// [`udelay()`]: https://docs.kernel.org/timers/delay_sleep_functions.html#c.udelay
>> +pub fn udelay(delta: Delta) {
>> +    const MAX_UDELAY_DELTA: Delta = Delta::from_millis(bindings::MAX_UDELAY_MS as i64);
> 
> We should perhaps add a build_assert here to make sure this cast is always valid?

Are you concerned that bindings::MAX_UDELAY_MS might exceed i64::MAX?

If so, such a value seems unrealistic as a delay. While
bindings::MAX_UDELAY_MS is architecture-dependent, its maximum is
currently 10, so I don’t think this will ever become an issue in the
future.

If really necessary, we could add something like the followings?

build_assert!(i128::from(bindings::MAX_UDELAY_MS) < i64::MAX.into());

>> +
>> +    let delta = if (Delta::ZERO..=MAX_UDELAY_DELTA).contains(&delta) {
>> +        delta
>> +    } else {
>> +        // TODO: Add WARN_ONCE() when it's supported.
>> +        MAX_UDELAY_DELTA
>> +    };
>> +
>> +    // SAFETY: It is always safe to call `udelay()` with any duration.
>> +    unsafe {
>> +        // Convert the duration to microseconds and round up to preserve
>> +        // the guarantee; `udelay()` inserts a delay for at least
>> +        // the provided duration, but that it may delay for longer
>> +        // under some circumstances.
>> +        bindings::udelay(delta.as_micros_ceil() as c_ulong)
>> +    }
>> +}
>> -- 
>> 2.43.0
>> 
>> 
> 
> With the change you suggested for the safety comment in udelay:
> 
> Reviewed-by: Daniel Almeida <daniel.almeida@...labora.com>

As Miguel wrote, any duration is safe from Rust’s perspective, and it
won’t invoke UB on the C side either.

So I'm not sure the above safety comment needs to be changed. Let’s
ask for Andreas’s opinion.