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

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

[chenhan]

Delta's division and rem_nanos() use Rust operators on 64-bit and C
helpers on 32-bit. The report shows that rem_nanos(0) can return 10 for
a 10 ns dividend on ARMv7, while the same call panics on x86-64. The
i64::MIN / -1 case also differs, and neither API documents these inputs.

Use Rust's signed division and remainder semantics on both architectures,
as discussed in the report. Div should behave like i64 division, and
rem_nanos() supplies the corresponding remainder operation with a 32-bit
divisor. Both reject zero divisors and i64::MIN with -1, even when Rust
overflow checks are disabled.

Check these inputs before entering the architecture-specific code and
document the panic conditions. This preserves the API signatures and
valid-input results, but intentionally makes invalid inputs panic on
32-bit too. No in-tree callers of either API were found at the base
commit. Correct the rem_nanos() parameter name to divisor as well.

Tested on x86-64 and ARMv7 QEMU with a built-in Rust module that passes
runtime operands to both APIs. Separate boots verify each panic case;
valid-input tests cover mixed signs, signed extrema and full-width
division operands.

Fixes: 4521438fb076 ("rust: time: Implement basic arithmetic operations for Delta")
Reported-by: Georgios Androutsopoulos <georgeandrout13@gmail.com>
Closes: https://github.com/Rust-for-Linux/linux/issues/1254
Link: https://github.com/Rust-for-Linux/linux/issues/1254#issuecomment-5869573842
Link: https://github.com/Rust-for-Linux/linux/issues/1254#issuecomment-5869925737
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: chenhan <chenhan0017.work@gmail.com>
---
Changes in v2:
- Put the checks directly in div() and rem_nanos(), following Tomonori's
  suggestion. Each proposed helper had only one caller, and no other
  concrete users were found to justify a math module.
- Move # Panics onto div(), remove private-helper references, link the
  integer types, and describe each API's input domain separately.
- Explain the arithmetic and pointer conditions in the SAFETY comments.
- Add Georgios' Reported-by and all M:/R: contacts requested by Miguel.
- Cite the issue discussion establishing the intended panic semantics.
- Replace the local CONFIG_SAMPLE_RUST_REPRO name with a description of
  the test; the API calls and expected results are explained below.
- Base this revision on timekeeping-next at 2ea0119f72db, preserving the
  new to_jiffies_timeout() method, and rerun x86-64/ARMv7 validation.

Caller audit: no calls to either API outside their definitions were found
on the base above or rust-next at c82c75ae11fa. This covered tracked Rust
sources, including drivers, samples and lib, using named-call searches
and inspection of division expressions in Delta/Instant users, including
elapsed() results. Time-unit conversions use fixed positive divisors;
to_jiffies_timeout() uses an unsigned multiply/add/divide helper;
num/bounded.rs operates on generic integer types. None provides a concrete
extra user for the proposed signed helpers.

I kept the stable Cc as suggested. The 32-bit invalid-input behavior does
change, and I have not identified an affected in-tree caller or audited
all stable branches. Please let me know if this consistency fix should
not be backported.

On the MIN / -1 safety question: div_s64_rem() computes a quotient even
though the caller discards it. V1 already checked this input before the C
call; v2 keeps the guard and makes the representable-quotient condition
explicit in the SAFETY comment. Rust's signed remainder operator panics
for this input even though the mathematical remainder would be zero.

Testing: CONFIG_SAMPLE_RUST_REPRO in v1 was a local Kconfig option for
samples/rust/rust_repro.rs; neither was included in that submission.
The built-in module read dividend: i64, divisor: i32 and an operation
selector from module parameters during init, then evaluated one of:

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

For v2, an expanded module passes black_box operands to the actual Delta
APIs. Each architecture has a valid-input boot with 26 division and 21
remainder comparisons, and four separate boots for the panic cases.
The following core cases pass on both x86-64 and ARMv7:

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

Further valid cases include mixed signs, i32::MIN divisors,
i64::MIN/i64::MAX dividends and full-width i64 divisors for division.
Both configurations have CONFIG_RUST_OVERFLOW_CHECKS=y; I have not
boot-tested with it disabled. The added checks are unconditional.

For the identity question: 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.

 rust/kernel/time.rs | 46 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 5ae377e70dd8..31a1c95bc786 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -412,8 +412,20 @@ fn mul_assign(&mut self, rhs: i64) {
 impl ops::Div for Delta {
     type Output = i64;
 
+    /// # Panics
+    ///
+    /// Panics if `rhs` is zero, or if `self` represents [`i64::MIN`] nanoseconds
+    /// and `rhs` represents `-1` nanosecond.
     #[inline]
     fn div(self, rhs: Self) -> Self::Output {
+        if rhs.value == 0 {
+            panic!("attempt to divide by zero");
+        }
+
+        if self.value == i64::MIN && rhs.value == -1 {
+            panic!("attempt to divide with overflow");
+        }
+
         #[cfg(CONFIG_64BIT)]
         {
             self.value / rhs.value
@@ -421,7 +433,7 @@ fn div(self, rhs: Self) -> Self::Output {
 
         #[cfg(not(CONFIG_64BIT))]
         {
-            // SAFETY: This function is always safe to call regardless of the input values
+            // SAFETY: The divisor is non-zero and the quotient is representable.
             unsafe { bindings::div64_s64(self.value, rhs.value) }
         }
     }
@@ -614,16 +626,33 @@ pub fn to_jiffies_timeout(self) -> Delta<Jiffy> {
         Delta::<Jiffy>::from_jiffies(jiffies)
     }
 
-    /// Return `self % dividend` where `dividend` is in nanoseconds.
+    /// Return `self % divisor`, where `divisor` is a number of nanoseconds.
+    ///
+    /// A non-zero result has the sign of `self`, and the magnitude of the result
+    /// 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.
+    /// The divisor is an [`i32`] because `div_s64_rem()`, used on 32-bit
+    /// platforms, takes a signed 32-bit divisor. The dividend remains an [`i64`].
+    ///
+    /// # Panics
+    ///
+    /// Panics if `divisor` is zero, or if `self` represents [`i64::MIN`]
+    /// nanoseconds and `divisor` is `-1`, matching Rust's signed remainder
+    /// operator even though the remainder would be zero.
     #[inline]
-    pub fn rem_nanos(self, dividend: i32) -> Self {
+    pub fn rem_nanos(self, divisor: i32) -> Self {
+        if divisor == 0 {
+            panic!("attempt to calculate the remainder with a divisor of zero");
+        }
+
+        if self.value == i64::MIN && divisor == -1 {
+            panic!("attempt to calculate the remainder with overflow");
+        }
+
         #[cfg(CONFIG_64BIT)]
         {
             Self {
-                value: self.as_nanos() % i64::from(dividend),
+                value: self.as_nanos() % i64::from(divisor),
             }
         }
 
@@ -631,8 +660,9 @@ pub fn rem_nanos(self, dividend: i32) -> Self {
         {
             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) };
+            // SAFETY: The divisor is non-zero and the quotient is representable.
+            // `&mut rem` provides a valid, aligned pointer to a writable `i32` for this call.
+            unsafe { bindings::div_s64_rem(self.as_nanos(), divisor, &mut rem) };
 
             Self {
                 value: i64::from(rem),

base-commit: 2ea0119f72dba597aa8a98cbdb72c564bfc5cb38
-- 
2.34.1

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

[Andreas Hindborg]

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

> Delta's division and rem_nanos() use Rust operators on 64-bit and C
> helpers on 32-bit. The report shows that rem_nanos(0) can return 10 for
> a 10 ns dividend on ARMv7, while the same call panics on x86-64. The
> i64::MIN / -1 case also differs, and neither API documents these inputs.
>
> Use Rust's signed division and remainder semantics on both architectures,
> as discussed in the report. Div should behave like i64 division, and
> rem_nanos() supplies the corresponding remainder operation with a 32-bit
> divisor. Both reject zero divisors and i64::MIN with -1, even when Rust
> overflow checks are disabled.
>
> Check these inputs before entering the architecture-specific code and
> document the panic conditions. This preserves the API signatures and
> valid-input results, but intentionally makes invalid inputs panic on
> 32-bit too. No in-tree callers of either API were found at the base
> commit. Correct the rem_nanos() parameter name to divisor as well.
>
> Tested on x86-64 and ARMv7 QEMU with a built-in Rust module that passes
> runtime operands to both APIs. Separate boots verify each panic case;
> valid-input tests cover mixed signs, signed extrema and full-width
> division operands.
>
> Fixes: 4521438fb076 ("rust: time: Implement basic arithmetic operations for Delta")
> Reported-by: Georgios Androutsopoulos <georgeandrout13@gmail.com>
> Closes: https://github.com/Rust-for-Linux/linux/issues/1254
> Link: https://github.com/Rust-for-Linux/linux/issues/1254#issuecomment-5869573842
> Link: https://github.com/Rust-for-Linux/linux/issues/1254#issuecomment-5869925737
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: chenhan <chenhan0017.work@gmail.com>
> ---
> Changes in v2:
> - Put the checks directly in div() and rem_nanos(), following Tomonori's
>   suggestion. Each proposed helper had only one caller, and no other
>   concrete users were found to justify a math module.

Even so, I think we should get the math module started and move the
division helpers there. This discussion is not the first on this
problem. Having the helpers available will help everyone, and create
precedence for further helpers down the line.

Best regards,
Andreas Hindborg

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

[FUJITA Tomonori]

On Thu, 01 Oct 2026 10:10:02 +0200
Andreas Hindborg <a.hindborg@kernel.org> wrote:

> "chenhan" <chenhan0017.work@gmail.com> writes:
> 
>> Delta's division and rem_nanos() use Rust operators on 64-bit and C
>> helpers on 32-bit. The report shows that rem_nanos(0) can return 10 for
>> a 10 ns dividend on ARMv7, while the same call panics on x86-64. The
>> i64::MIN / -1 case also differs, and neither API documents these inputs.
>>
>> Use Rust's signed division and remainder semantics on both architectures,
>> as discussed in the report. Div should behave like i64 division, and
>> rem_nanos() supplies the corresponding remainder operation with a 32-bit
>> divisor. Both reject zero divisors and i64::MIN with -1, even when Rust
>> overflow checks are disabled.
>>
>> Check these inputs before entering the architecture-specific code and
>> document the panic conditions. This preserves the API signatures and
>> valid-input results, but intentionally makes invalid inputs panic on
>> 32-bit too. No in-tree callers of either API were found at the base
>> commit. Correct the rem_nanos() parameter name to divisor as well.
>>
>> Tested on x86-64 and ARMv7 QEMU with a built-in Rust module that passes
>> runtime operands to both APIs. Separate boots verify each panic case;
>> valid-input tests cover mixed signs, signed extrema and full-width
>> division operands.
>>
>> Fixes: 4521438fb076 ("rust: time: Implement basic arithmetic operations for Delta")
>> Reported-by: Georgios Androutsopoulos <georgeandrout13@gmail.com>
>> Closes: https://github.com/Rust-for-Linux/linux/issues/1254
>> Link: https://github.com/Rust-for-Linux/linux/issues/1254#issuecomment-5869573842
>> Link: https://github.com/Rust-for-Linux/linux/issues/1254#issuecomment-5869925737
>> Cc: stable@vger.kernel.org
>> Assisted-by: LLM
>> Signed-off-by: chenhan <chenhan0017.work@gmail.com>
>> ---
>> Changes in v2:
>> - Put the checks directly in div() and rem_nanos(), following Tomonori's
>>   suggestion. Each proposed helper had only one caller, and no other
>>   concrete users were found to justify a math module.
> 
> Even so, I think we should get the math module started and move the
> division helpers there. This discussion is not the first on this
> problem. Having the helpers available will help everyone, and create
> precedence for further helpers down the line.

I'm not against starting the math module. Uwe asked for div helpers in
pwm_th1520 [1], and I plan to add the math module for them.

However, if this patch is backported to stable, I think it should be
as small as possible. Also, v2 does not add any new div helpers. It
only adds checks to the existing code.

How about merging this fix as is, and adding the math module in a
separate patch?

[1] https://lore.kernel.org/all/20261001.070215.521963012921142699.tomo@flapping.org/

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

[Andreas Hindborg]

"FUJITA Tomonori" <tomo@flapping.org> writes:

> On Thu, 01 Oct 2026 10:10:02 +0200
> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
>> "chenhan" <chenhan0017.work@gmail.com> writes:
>>
>>> Delta's division and rem_nanos() use Rust operators on 64-bit and C
>>> helpers on 32-bit. The report shows that rem_nanos(0) can return 10 for
>>> a 10 ns dividend on ARMv7, while the same call panics on x86-64. The
>>> i64::MIN / -1 case also differs, and neither API documents these inputs.
>>>
>>> Use Rust's signed division and remainder semantics on both architectures,
>>> as discussed in the report. Div should behave like i64 division, and
>>> rem_nanos() supplies the corresponding remainder operation with a 32-bit
>>> divisor. Both reject zero divisors and i64::MIN with -1, even when Rust
>>> overflow checks are disabled.
>>>
>>> Check these inputs before entering the architecture-specific code and
>>> document the panic conditions. This preserves the API signatures and
>>> valid-input results, but intentionally makes invalid inputs panic on
>>> 32-bit too. No in-tree callers of either API were found at the base
>>> commit. Correct the rem_nanos() parameter name to divisor as well.
>>>
>>> Tested on x86-64 and ARMv7 QEMU with a built-in Rust module that passes
>>> runtime operands to both APIs. Separate boots verify each panic case;
>>> valid-input tests cover mixed signs, signed extrema and full-width
>>> division operands.
>>>
>>> Fixes: 4521438fb076 ("rust: time: Implement basic arithmetic operations for Delta")
>>> Reported-by: Georgios Androutsopoulos <georgeandrout13@gmail.com>
>>> Closes: https://github.com/Rust-for-Linux/linux/issues/1254
>>> Link: https://github.com/Rust-for-Linux/linux/issues/1254#issuecomment-5869573842
>>> Link: https://github.com/Rust-for-Linux/linux/issues/1254#issuecomment-5869925737
>>> Cc: stable@vger.kernel.org
>>> Assisted-by: LLM
>>> Signed-off-by: chenhan <chenhan0017.work@gmail.com>
>>> ---
>>> Changes in v2:
>>> - Put the checks directly in div() and rem_nanos(), following Tomonori's
>>>   suggestion. Each proposed helper had only one caller, and no other
>>>   concrete users were found to justify a math module.
>>
>> Even so, I think we should get the math module started and move the
>> division helpers there. This discussion is not the first on this
>> problem. Having the helpers available will help everyone, and create
>> precedence for further helpers down the line.
>
> I'm not against starting the math module. Uwe asked for div helpers in
> pwm_th1520 [1], and I plan to add the math module for them.
>
> However, if this patch is backported to stable, I think it should be
> as small as possible. Also, v2 does not add any new div helpers. It
> only adds checks to the existing code.
>
> How about merging this fix as is, and adding the math module in a
> separate patch?

I'm not a stable expert, but I think adding the math module on stable
should be fine?

If you really want to, I guess we could have this patch be a 2-patch
series with this as the first patch with the stable Cc and then the 2nd
patch moving the code to the helpers from v1, but placed in the math
module.

Then we can pick patch 1 for fixes and have plenty time to discuss patch
2?

Best regards,
Andreas Hindborg

Re: [PATCH v2] 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:
>
> commit. Correct the rem_nanos() parameter name to divisor as well.

This seems like it should be a first patch.

> Link: https://github.com/Rust-for-Linux/linux/issues/1254#issuecomment-5869573842
> Link: https://github.com/Rust-for-Linux/linux/issues/1254#issuecomment-5869925737

The discussion is small enough, so I am not sure why we need the two
Link:s. They can be useful if you want to reference something in
particular, using e.g. the square brackets notation [1] to point to
them.

> On the MIN / -1 safety question: div_s64_rem() computes a quotient even
> though the caller discards it. V1 already checked this input before the C
> call; v2 keeps the guard and makes the representable-quotient condition
> explicit in the SAFETY comment.

Yes, that is fine -- what I meant is that even if we had the check,
the `// SAFETY` comment should still justify why it is not UB (please
see below).

> -            // SAFETY: This function is always safe to call regardless of the input values
> +            // SAFETY: The divisor is non-zero and the quotient is representable.

"representable" is a fine way to succinctly put it, but I wonder if it
would be better to just spell the condition.

In any case, safety comment need to explain *why* those conditions are
true, not just re-state them (otherwise, safety comments would be a
repetition of the `# Safety` documentation of the function called, and
thus not useful). In other words, the point is that they explain why
something is supposed to hold.

> -    /// Return `self % dividend` where `dividend` is in nanoseconds.
> +    /// Return `self % divisor`, where `divisor` is a number of nanoseconds.

"a number of" seems to be added spuriously -- please try not to do
unrelated changes, or you do them, please add as new commits (e.g.
this one wouldn't be a fix to backport).

> +    /// The divisor is an [`i32`] because `div_s64_rem()`, used on 32-bit
> +    /// platforms, takes a signed 32-bit divisor. The dividend remains an [`i64`].

Do we need this in the public documentation?

> +    /// Panics if `divisor` is zero, or if `self` represents [`i64::MIN`]
> +    /// nanoseconds and `divisor` is `-1`, matching Rust's signed remainder
> +    /// operator even though the remainder would be zero.

Not sure if we need the "even though ..." bit.

Thanks!

Cheers,
Miguel