Re: [PATCH v5 02/15] rust: sync: completion: add wait_for_completion_timeout()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] rust: sync: wait_for_completion_timeout stack dump on large timeouts
--

commit f89eb4d12071b7e3c61766cd1bbbd5c2cfee1ea5
Author: Joel Fernandes <joelagnelf@nvidia.com>

rust: sync: completion: add wait_for_completion_timeout()

This commit adds wait_for_completion_timeout() to the Rust sync completion
primitives. It provides a way to wait for a completion with a specified
timeout in jiffies, preventing indefinite stalls in drivers.

> diff --git a/rust/kernel/sync/completion.rs b/rust/kernel/sync/completion.rs
> --- a/rust/kernel/sync/completion.rs
> +++ b/rust/kernel/sync/completion.rs
[ ... ]
> @@ -111,4 +116,20 @@ pub fn wait_for_completion(&self) {
>          // SAFETY: `self.as_raw()` is a pointer to a valid `struct completion`.
>          unsafe { bindings::wait_for_completion(self.as_raw()) };
>      }
> +
> +    /// Wait for completion of a task, with a timeout.
[ ... ]
> +    #[inline]
> +    pub fn wait_for_completion_timeout(&self, timeout: Jiffies) -> Option<Jiffies> {
> +        // SAFETY: `self.as_raw()` is a pointer to a valid `struct completion`.
> +        match unsafe { bindings::wait_for_completion_timeout(self.as_raw(), timeout) } {

[Severity: Medium]
Does this code overflow the timeout parameter if large values are provided?

The timeout parameter is passed directly to the C API without bounds checking.
Because C implicitly casts it to a signed long downstream, values greater than
LONG_MAX can lead to an immediate unhandled timeout and a kernel stack dump.

For example, passing Jiffies::MAX causes the unsigned timeout to overflow into
a negative long inside the C wait_for_common() and schedule_timeout()
functions. This triggers a kernel stack dump (printk(KERN_ERR ...);
dump_stack();) and causes the function to return immediately without waiting,
violating the safe function's contract.

Could this be clamped to MAX_SCHEDULE_TIMEOUT to prevent unexpected behavior
from excessively large timeouts?

> +            0 => None,
> +            remaining => Some(remaining),
> +        }
> +    }
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930034148.590687-1-jhubbard@nvidia.com?part=2