[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?