[PATCH] rust: time: make Delta division and remainder fail consistently

[PATCH] rust: time: make Delta division and remainder fail consistently

[chenhan]

On 64-bit, Delta division and remainder use Rust operators, which panic
for a zero divisor and for the i64::MIN / -1 overflow case. On 32-bit,
the C helpers do not provide the same behavior, so the same API calls can
return architecture-dependent values or emit a divide-by-zero diagnostic.

Check both invalid inputs before selecting the architecture-specific
implementation. This gives both APIs the same panic conditions as i64's
`/` and `%` operators and documents them in the public API.

Tested on x86-64 and ARMv7 QEMU with CONFIG_SAMPLE_RUST_REPRO=y: zero
divisors and i64::MIN / -1 panic for both APIs, while 10 / 3 returns 3
and 10 % 3 returns 1 on both architectures.

Fixes: 4521438fb076 ("rust: time: Implement basic arithmetic operations for Delta")
Closes: https://github.com/Rust-for-Linux/linux/issues/1254
Assisted-by: LLM
Signed-off-by: chenhan <chenhan0017.work@gmail.com>
---
 rust/kernel/time.rs | 103 +++++++++++++++++++++++++++++++-------------
 1 file changed, 72 insertions(+), 31 deletions(-)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 6c0a5e8090d0..55c90365ba73 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -405,21 +405,65 @@ fn mul_assign(&mut self, rhs: i64) {
     }
 }
 
+#[inline]
+fn div_s64_or_panic(dividend: i64, divisor: i64) -> i64 {
+    if divisor == 0 {
+        panic!("attempt to divide by zero");
+    }
+
+    if dividend == i64::MIN && divisor == -1 {
+        panic!("attempt to divide with overflow");
+    }
+
+    #[cfg(CONFIG_64BIT)]
+    {
+        dividend / divisor
+    }
+
+    #[cfg(not(CONFIG_64BIT))]
+    {
+        // SAFETY: `divisor` is non-zero, and both operands are passed by value.
+        unsafe { bindings::div64_s64(dividend, divisor) }
+    }
+}
+
+#[inline]
+fn rem_s64_or_panic(dividend: i64, divisor: i32) -> i64 {
+    if divisor == 0 {
+        panic!("attempt to calculate the remainder with a divisor of zero");
+    }
+
+    if dividend == i64::MIN && divisor == -1 {
+        panic!("attempt to calculate the remainder with overflow");
+    }
+
+    #[cfg(CONFIG_64BIT)]
+    {
+        dividend % i64::from(divisor)
+    }
+
+    #[cfg(not(CONFIG_64BIT))]
+    {
+        let mut rem = 0;
+
+        // SAFETY: `rem` points to a local variable and `divisor` is non-zero.
+        unsafe { bindings::div_s64_rem(dividend, divisor, &mut rem) };
+
+        i64::from(rem)
+    }
+}
+
+/// # Panics
+///
+/// Panics if `rhs` is zero, or if the quotient overflows, i.e. if `self` is
+/// `Delta::from_nanos(i64::MIN)` and `rhs` is `Delta::from_nanos(-1)`. Both
+/// cases panic on 32-bit as well as on 64-bit; see `div_s64_or_panic()`.
 impl ops::Div for Delta {
     type Output = i64;
 
     #[inline]
     fn div(self, rhs: Self) -> Self::Output {
-        #[cfg(CONFIG_64BIT)]
-        {
-            self.value / rhs.value
-        }
-
-        #[cfg(not(CONFIG_64BIT))]
-        {
-            // SAFETY: This function is always safe to call regardless of the input values
-            unsafe { bindings::div64_s64(self.value, rhs.value) }
-        }
+        div_s64_or_panic(self.value, rhs.value)
     }
 }
 
@@ -554,29 +598,26 @@ pub fn as_millis_ceil(self) -> i64 {
         }
     }
 
-    /// Return `self % dividend` where `dividend` is in nanoseconds.
+    /// Return `self % divisor`, where `divisor` is a number of nanoseconds.
+    ///
+    /// The result has the sign of `self`, and its magnitude is strictly smaller
+    /// than that of `divisor`.
     ///
-    /// The kernel doesn't have any emulation for `s64 % s64` on 32 bit platforms, so this is
-    /// limited to 32 bit dividends.
+    /// `divisor` is a 32-bit integer because the helper called on 32-bit
+    /// platforms, `div_s64_rem()`, takes an `s32` divisor. The dividend (`self`)
+    /// is a full `i64` on every architecture, so the width restriction applies
+    /// to all configurations, not only to 32-bit ones.
+    ///
+    /// # Panics
+    ///
+    /// Panics if `divisor` is zero, or if the division overflows, i.e. if `self`
+    /// is `Delta::from_nanos(i64::MIN)` and `divisor` is `-1`. These are the same
+    /// inputs [`ops::Div`] panics on, and the same ones `i64`'s `%` operator
+    /// panics on. See `rem_s64_or_panic()`.
     #[inline]
-    pub fn rem_nanos(self, dividend: i32) -> Self {
-        #[cfg(CONFIG_64BIT)]
-        {
-            Self {
-                value: self.as_nanos() % i64::from(dividend),
-            }
-        }
-
-        #[cfg(not(CONFIG_64BIT))]
-        {
-            let mut rem = 0;
-
-            // SAFETY: `rem` is in the stack, so we can always provide a valid pointer to it.
-            unsafe { bindings::div_s64_rem(self.as_nanos(), dividend, &mut rem) };
-
-            Self {
-                value: i64::from(rem),
-            }
+    pub fn rem_nanos(self, divisor: i32) -> Self {
+        Self {
+            value: rem_s64_or_panic(self.as_nanos(), divisor),
         }
     }
 }

base-commit: d266640c6c760c9bc215bf5a3ece122ca488b6f5
-- 
2.34.1

Re: [PATCH] rust: time: make Delta division and remainder fail consistently

[Andreas Hindborg]

"chenhan" <chenhan0017.work@gmail.com> writes:

> On 64-bit, Delta division and remainder use Rust operators, which panic
> for a zero divisor and for the i64::MIN / -1 overflow case. On 32-bit,
> the C helpers do not provide the same behavior, so the same API calls can
> return architecture-dependent values or emit a divide-by-zero diagnostic.
>
> Check both invalid inputs before selecting the architecture-specific
> implementation. This gives both APIs the same panic conditions as i64's
> `/` and `%` operators and documents them in the public API.
>
> Tested on x86-64 and ARMv7 QEMU with CONFIG_SAMPLE_RUST_REPRO=y: zero
> divisors and i64::MIN / -1 panic for both APIs, while 10 / 3 returns 3
> and 10 % 3 returns 1 on both architectures.
>
> Fixes: 4521438fb076 ("rust: time: Implement basic arithmetic operations for Delta")
> Closes: https://github.com/Rust-for-Linux/linux/issues/1254
> Assisted-by: LLM
> Signed-off-by: chenhan <chenhan0017.work@gmail.com>
> ---
>  rust/kernel/time.rs | 103 +++++++++++++++++++++++++++++++-------------
>  1 file changed, 72 insertions(+), 31 deletions(-)
>
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> index 6c0a5e8090d0..55c90365ba73 100644
> --- a/rust/kernel/time.rs
> +++ b/rust/kernel/time.rs
> @@ -405,21 +405,65 @@ fn mul_assign(&mut self, rhs: i64) {
>      }
>  }
>
> +#[inline]
> +fn div_s64_or_panic(dividend: i64, divisor: i64) -> i64 {
> +    if divisor == 0 {
> +        panic!("attempt to divide by zero");
> +    }
> +
> +    if dividend == i64::MIN && divisor == -1 {
> +        panic!("attempt to divide with overflow");
> +    }
> +
> +    #[cfg(CONFIG_64BIT)]
> +    {
> +        dividend / divisor
> +    }
> +
> +    #[cfg(not(CONFIG_64BIT))]
> +    {
> +        // SAFETY: `divisor` is non-zero, and both operands are passed by value.
> +        unsafe { bindings::div64_s64(dividend, divisor) }
> +    }
> +}
> +
> +#[inline]
> +fn rem_s64_or_panic(dividend: i64, divisor: i32) -> i64 {
> +    if divisor == 0 {
> +        panic!("attempt to calculate the remainder with a divisor of zero");
> +    }
> +
> +    if dividend == i64::MIN && divisor == -1 {
> +        panic!("attempt to calculate the remainder with overflow");
> +    }
> +
> +    #[cfg(CONFIG_64BIT)]
> +    {
> +        dividend % i64::from(divisor)
> +    }
> +
> +    #[cfg(not(CONFIG_64BIT))]
> +    {
> +        let mut rem = 0;
> +
> +        // SAFETY: `rem` points to a local variable and `divisor` is non-zero.
> +        unsafe { bindings::div_s64_rem(dividend, divisor, &mut rem) };
> +
> +        i64::from(rem)
> +    }
> +}

We should probably find a better place for these. How about
rust/kernel/math.rs? Opinions?


Best regards,
Andreas Hindborg

Re: [PATCH] rust: time: make Delta division and remainder fail consistently

[Miguel Ojeda]

On Mon, Sep 28, 2026 at 8:39 PM chenhan <chenhan0017.work@gmail.com> wrote:
>
> Tested on x86-64 and ARMv7 QEMU with CONFIG_SAMPLE_RUST_REPRO=y: zero
> divisors and i64::MIN / -1 panic for both APIs, while 10 / 3 returns 3
> and 10 % 3 returns 1 on both architectures.

This does not add much information, since we don't have the reproducer
here, i.e. what is `CONFIG_SAMPLE_RUST_REPRO`? I would instead explain
what the reproducer is, if it is important to do so.

> Fixes: 4521438fb076 ("rust: time: Implement basic arithmetic operations for Delta")

This points to something that could be backported, so if it is a fix,
then we probably want:

Cc: stable@vger.kernel.org

However, this changes behavior -- did you check all callers etc.?

> Signed-off-by: chenhan <chenhan0017.work@gmail.com>

The kernel requires a "known identity", is "chenhan" one?

  https://docs.kernel.org/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin

> +        // SAFETY: `divisor` is non-zero, and both operands are passed by value.

What "both operands are passed by value" is trying to say? i.e. what
precondition are you trying to satisfy?

> +        // SAFETY: `rem` points to a local variable and `divisor` is non-zero.
> +        unsafe { bindings::div_s64_rem(dividend, divisor, &mut rem) };

What happens in the `min / -1` case?

> +/// Panics if `rhs` is zero, or if the quotient overflows, i.e. if `self` is
> +/// `Delta::from_nanos(i64::MIN)` and `rhs` is `Delta::from_nanos(-1)`. Both
> +/// cases panic on 32-bit as well as on 64-bit; see `div_s64_or_panic()`.

I think the last sentence is not needed, i.e. if nothing is said, then
it should be understood apply to all cases. Perhaps we could keep the
reference to the function, if it were public, with an intra-doc link,
but it isn't, so I don't think we need it either.

> +    /// platforms, `div_s64_rem()`, takes an `s32` divisor. The dividend (`self`)

Perhaps `i32` with an intra-doc link instead?

> +    /// is a full `i64` on every architecture, so the width restriction applies

Please add intra-doc links where they work, e.g. [`i64`]

Thanks for the patch and welcome!

Cheers,
Miguel

Re: [PATCH] rust: time: make Delta division and remainder fail consistently

[Miguel Ojeda]

On Mon, Sep 28, 2026 at 9:12 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> We should probably find a better place for these. How about
> rust/kernel/math.rs? Opinions?

Yeah, they should not be in `time.rs`.

Cheers,
Miguel

Re: [PATCH] rust: time: make Delta division and remainder fail consistently

[Miguel Ojeda]

On Tue, Sep 29, 2026 at 9:20 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> Yeah, they should not be in `time.rs`.

By the way, chenhan: in the next version, please Cc everyone in the
`DELAY, SLEEP, TIMEKEEPING, TIMERS [RUST]` and `RUST` entries (both
`M:` and `R:` fields).

Thanks!

Cheers,
Miguel

Re: [PATCH] rust: time: make Delta division and remainder fail consistently

[FUJITA Tomonori]

On Tue, 29 Sep 2026 02:39:25 +0800
chenhan <chenhan0017.work@gmail.com> wrote:

> On 64-bit, Delta division and remainder use Rust operators, which panic
> for a zero divisor and for the i64::MIN / -1 overflow case. On 32-bit,
> the C helpers do not provide the same behavior, so the same API calls can
> return architecture-dependent values or emit a divide-by-zero diagnostic.
> 
> Check both invalid inputs before selecting the architecture-specific
> implementation. This gives both APIs the same panic conditions as i64's
> `/` and `%` operators and documents them in the public API.
> 
> Tested on x86-64 and ARMv7 QEMU with CONFIG_SAMPLE_RUST_REPRO=y: zero
> divisors and i64::MIN / -1 panic for both APIs, while 10 / 3 returns 3
> and 10 % 3 returns 1 on both architectures.
> 
> Fixes: 4521438fb076 ("rust: time: Implement basic arithmetic operations for Delta")
> Closes: https://github.com/Rust-for-Linux/linux/issues/1254
> Assisted-by: LLM
> Signed-off-by: chenhan <chenhan0017.work@gmail.com>
> ---
>  rust/kernel/time.rs | 103 +++++++++++++++++++++++++++++++-------------
>  1 file changed, 72 insertions(+), 31 deletions(-)
> 
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> index 6c0a5e8090d0..55c90365ba73 100644
> --- a/rust/kernel/time.rs
> +++ b/rust/kernel/time.rs
> @@ -405,21 +405,65 @@ fn mul_assign(&mut self, rhs: i64) {
>      }
>  }
>  
> +#[inline]
> +fn div_s64_or_panic(dividend: i64, divisor: i64) -> i64 {

Do you have other users in mind? If not, each helper has only one
caller, so they could go directly into div() and rem_nanos().

> +    if divisor == 0 {
> +        panic!("attempt to divide by zero");
> +    }
> +
> +    if dividend == i64::MIN && divisor == -1 {
> +        panic!("attempt to divide with overflow");
> +    }
> +
> +    #[cfg(CONFIG_64BIT)]
> +    {
> +        dividend / divisor
> +    }
> +
> +    #[cfg(not(CONFIG_64BIT))]
> +    {
> +        // SAFETY: `divisor` is non-zero, and both operands are passed by value.
> +        unsafe { bindings::div64_s64(dividend, divisor) }
> +    }
> +}
> +
> +#[inline]
> +fn rem_s64_or_panic(dividend: i64, divisor: i32) -> i64 {

Ditto.

> +    if divisor == 0 {
> +        panic!("attempt to calculate the remainder with a divisor of zero");
> +    }
> +
> +    if dividend == i64::MIN && divisor == -1 {
> +        panic!("attempt to calculate the remainder with overflow");
> +    }
> +
> +    #[cfg(CONFIG_64BIT)]
> +    {
> +        dividend % i64::from(divisor)
> +    }
> +
> +    #[cfg(not(CONFIG_64BIT))]
> +    {
> +        let mut rem = 0;
> +
> +        // SAFETY: `rem` points to a local variable and `divisor` is non-zero.
> +        unsafe { bindings::div_s64_rem(dividend, divisor, &mut rem) };
> +
> +        i64::from(rem)
> +    }
> +}
> +
> +/// # Panics
> +///
> +/// Panics if `rhs` is zero, or if the quotient overflows, i.e. if `self` is
> +/// `Delta::from_nanos(i64::MIN)` and `rhs` is `Delta::from_nanos(-1)`. Both
> +/// cases panic on 32-bit as well as on 64-bit; see `div_s64_or_panic()`.

div_s64_or_panic() is private, so you should not mention it in the
public documentation.

Also, shouldn't the `# Panics` section be right above fn div()?


>  impl ops::Div for Delta {
>      type Output = i64;
>  
>      #[inline]
>      fn div(self, rhs: Self) -> Self::Output {
> -        #[cfg(CONFIG_64BIT)]
> -        {
> -            self.value / rhs.value
> -        }
> -
> -        #[cfg(not(CONFIG_64BIT))]
> -        {
> -            // SAFETY: This function is always safe to call regardless of the input values
> -            unsafe { bindings::div64_s64(self.value, rhs.value) }
> -        }
> +        div_s64_or_panic(self.value, rhs.value)
>      }
>  }
>  
> @@ -554,29 +598,26 @@ pub fn as_millis_ceil(self) -> i64 {
>          }
>      }
>  
> -    /// Return `self % dividend` where `dividend` is in nanoseconds.
> +    /// Return `self % divisor`, where `divisor` is a number of nanoseconds.
> +    ///
> +    /// The result has the sign of `self`, and its magnitude is strictly smaller
> +    /// than that of `divisor`.
>      ///
> -    /// The kernel doesn't have any emulation for `s64 % s64` on 32 bit platforms, so this is
> -    /// limited to 32 bit dividends.
> +    /// `divisor` is a 32-bit integer because the helper called on 32-bit
> +    /// platforms, `div_s64_rem()`, takes an `s32` divisor. The dividend (`self`)
> +    /// is a full `i64` on every architecture, so the width restriction applies
> +    /// to all configurations, not only to 32-bit ones.
> +    ///
> +    /// # Panics
> +    ///
> +    /// Panics if `divisor` is zero, or if the division overflows, i.e. if `self`
> +    /// is `Delta::from_nanos(i64::MIN)` and `divisor` is `-1`. These are the same
> +    /// inputs [`ops::Div`] panics on, and the same ones `i64`'s `%` operator

The inputs are not the same. One takes i64 and the other takes i32.

> +    /// panics on. See `rem_s64_or_panic()`.
>      #[inline]
> -    pub fn rem_nanos(self, dividend: i32) -> Self {
> -        #[cfg(CONFIG_64BIT)]
> -        {
> -            Self {
> -                value: self.as_nanos() % i64::from(dividend),
> -            }
> -        }
> -
> -        #[cfg(not(CONFIG_64BIT))]
> -        {
> -            let mut rem = 0;
> -
> -            // SAFETY: `rem` is in the stack, so we can always provide a valid pointer to it.
> -            unsafe { bindings::div_s64_rem(self.as_nanos(), dividend, &mut rem) };
> -
> -            Self {
> -                value: i64::from(rem),
> -            }
> +    pub fn rem_nanos(self, divisor: i32) -> Self {
> +        Self {
> +            value: rem_s64_or_panic(self.as_nanos(), divisor),
>          }
>      }
>  }
> 
> base-commit: d266640c6c760c9bc215bf5a3ece122ca488b6f5
> -- 
> 2.34.1
> 
> 

Re: [PATCH] rust: time: make Delta division and remainder fail consistently

[Georgios Androutsopoulos]

Thanks for working on this.

Could you please also add:

Reported-by: Georgios Androutsopoulos <georgeandrout13@gmail.com>

to the commit message?

Re: [PATCH] rust: time: make Delta division and remainder fail consistently

[chenhan]

Hi Andreas, Miguel, Tomonori and Georgios,

Thanks for the reviews. Here are the details behind the changes in v2.

Miguel wrote:
> This does not add much information, since we don't have the reproducer
> here, i.e. what is `CONFIG_SAMPLE_RUST_REPRO`? I would instead explain
> what the reproducer is, if it is important to do so.

You're right; CONFIG_SAMPLE_RUST_REPRO was a local Kconfig option for
samples/rust/rust_repro.rs, and neither was included in v1. Setting it to
y built the test module into the kernel. Its init function read a dividend
(i64), a divisor (i32) and an operation selector from module parameters,
then evaluated one of these expressions using kernel::time::Delta:

    Delta::from_nanos(dividend) / Delta::from_nanos(i64::from(divisor))
    Delta::from_nanos(dividend).rem_nanos(divisor).as_nanos()

The inputs came from the kernel command line. Each failing operation ran
in a separate QEMU boot, since a panic stops that test. The expected
results with the fix are:

    dividend    divisor    division        remainder (nanoseconds)
    10          0          panic           panic
    i64::MIN    -1         panic           panic
    10          3          3               1

V2 replaces the unexplained config name with a description of the test.
For v2 I used an expanded test module: it passes operands through
core::hint::black_box to exercise the actual APIs, checks 26 valid
divisions and 21 valid remainders per architecture, and runs the four
panic cases above in separate boots. All passed on x86-64 and ARMv7.
The division tests also cover full-width i64 divisors. Both kernel test
configurations had CONFIG_RUST_OVERFLOW_CHECKS=y; I have not boot-tested
with it disabled. The guards themselves are unconditional, consistent
with the Rust semantics discussed in the issue and cited in v2.

On Andreas' and Miguel's suggestion to move generic helpers to math.rs,
Tomonori wrote:
> Do you have other users in mind? If not, each helper has only one
> caller, so they could go directly into div() and rem_nanos().

I tried the math.rs version, then checked for other users. Each helper
still had only one caller. Time-unit conversions use fixed positive
divisors, to_jiffies_timeout() uses unsigned arithmetic, and
num/bounded.rs operates on generic integer types. None is a concrete
extra user for these fixed-type signed helpers. V2 therefore follows
Tomonori's suggestion: the checks are directly in the two methods, with
no generic free functions in time.rs and no new math module.

Miguel wrote:
> However, this changes behavior -- did you check all callers etc.?

I checked the tracked Rust sources, including drivers, samples and lib,
at timekeeping-next 2ea0119f72db (the v2 base) and rust-next c82c75ae11fa.
I searched named calls and inspected division expressions in Delta and
Instant users, including values obtained through elapsed(). I found no
in-tree calls to either API outside their definitions. Existing users
construct timeouts, compare elapsed times, or convert to scalar units.
Valid-input arithmetic is unchanged; invalid inputs on 32-bit now panic.
The audit does not cover out-of-tree users or all stable branches. I kept
the stable Cc alongside Fixes as suggested; please let me know if this
consistency fix is unsuitable for backporting without an affected caller.

Miguel wrote:
> What "both operands are passed by value" is trying to say? i.e. what
> precondition are you trying to satisfy?
> [...]
> What happens in the `min / -1` case?

Passing by value did not establish a relevant safety precondition. The
comments now state that the divisor is non-zero and the quotient is
representable. div_s64_rem() computes a quotient even though we discard
it. The MIN / -1 guard was already before the C call in v1; v2 keeps it
and makes that condition explicit in the SAFETY comment. Without the
guard, the generic 32-bit C implementation can produce a wrapped quotient
and a zero remainder under the kernel's C arithmetic flags. Rust's signed
remainder operator panics for this input even though the mathematical
remainder is zero, so we reject it to match Rust's semantics. The pointer
comment also explains that &mut rem provides a valid, aligned, writable
i32 for the call.

For the documentation comments from Miguel and Tomonori, I moved
# Panics immediately above fn div(), removed the redundant architecture
sentence and private-helper references, and added intra-doc links for
i32, i64 and i64::MIN. I also removed the claim of identical inputs:
Delta division takes a Delta holding i64 nanoseconds, while rem_nanos()
takes an i32 divisor. Each method documents its own panic conditions.

Miguel wrote:
> The kernel requires a "known identity", is "chenhan" one?

Yes. chenhan is the identity I consistently use, and
chenhan0017.work@gmail.com is my email address. I also participate in
issue #1254 as WindDevil on GitHub. V2 uses that identity consistently in
From and Signed-off-by.

V2 includes all M: and R: contacts in both requested MAINTAINERS entries:
DELAY, SLEEP, TIMEKEEPING, TIMERS [RUST] and RUST. It also retains the
discussion participants and mailing lists.

Georgios: I added the requested tag to the commit message:
Reported-by: Georgios Androutsopoulos <georgeandrout13@gmail.com>

Andreas: sorry about the HTML reply and sending it only to you. I will
use plain text and reply-all for this discussion.

Best regards,
chenhan

Re: [PATCH] rust: time: make Delta division and remainder fail consistently

[Miguel Ojeda]

On Thu, Oct 1, 2026 at 6:29 AM chenhan <chenhan0017.work@gmail.com> wrote:
>
> Yes. chenhan is the identity I consistently use, and
> chenhan0017.work@gmail.com is my email address.

I asked because I didn't see it in the kernel log, i.e. it seems to be
the first commit with it.

In other words, at least in my case I don't know that identity.
Maintainers picking the patch usually need to know (in this case, the
timekeeping ones -- it is up to them).

I hope that clarifies!

Cheers,
Miguel