mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: chenhan <chenhan0017.work@gmail.com>
To: rust-for-linux@vger.kernel.org
Cc: a.hindborg@kernel.org, boqun@kernel.org,
	fujita.tomonori@gmail.com, frederic@kernel.org, lyude@redhat.com,
	tglx@kernel.org, anna-maria@linutronix.de, jstultz@google.com,
	sboyd@kernel.org, ojeda@kernel.org, gary@garyguo.net,
	bjorn3_gh@protonmail.com, lossin@kernel.org,
	aliceryhl@google.com, tmgross@umich.edu, dakr@kernel.org,
	daniel.almeida@collabora.com, tamird@kernel.org,
	acourbot@nvidia.com, work@onurozkan.dev,
	miguel.ojeda.sandonis@gmail.com, tomo@flapping.org,
	georgeandrout13@gmail.com, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: [PATCH v2] rust: time: make Delta division and remainder fail consistently
Date: Thu,  1 Oct 2026 12:29:42 +0800	[thread overview]
Message-ID: <20261001042942.109012-1-chenhan0017.work@gmail.com> (raw)

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

             reply	other threads:[~2026-10-01  4:29 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <6z5gg2l6ctq6aQIEoJ5hBjZr03ffUqGeECNSGQcVv5eyiZz35kWClxv9o5N7Y-PPk2R2dgpdDxEDLfG7GpkGog==@protonmail.internalid>
2026-10-01  4:29 ` chenhan [this message]
2026-10-01  8:10   ` Andreas Hindborg
2026-10-01  9:31     ` FUJITA Tomonori
2026-10-01 10:10       ` Andreas Hindborg
2026-10-01 10:46   ` Miguel Ojeda

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261001042942.109012-1-chenhan0017.work@gmail.com \
    --to=chenhan0017.work@gmail.com \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=aliceryhl@google.com \
    --cc=anna-maria@linutronix.de \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=frederic@kernel.org \
    --cc=fujita.tomonori@gmail.com \
    --cc=gary@garyguo.net \
    --cc=georgeandrout13@gmail.com \
    --cc=jstultz@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=lyude@redhat.com \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=sboyd@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=tamird@kernel.org \
    --cc=tglx@kernel.org \
    --cc=tmgross@umich.edu \
    --cc=tomo@flapping.org \
    --cc=work@onurozkan.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®