[PATCH v1 0/2] rust: bug: add warn_once macro

[PATCH v1 0/2] rust: bug: add warn_once macro

[FUJITA Tomonori]

From: FUJITA Tomonori <fujita.tomonori@gmail.com>

This series adds `warn_once!`, the Rust version of C `WARN_ONCE()`.

Patch 1 documents when to use the WARN macros, based on the comment in
include/asm-generic/bug.h. Patch 2 adds `warn_once!`.

Miguel, patch 2 moves the UML `warn_slowpath_fmt()` call into
`warn_slowpath()` and updates its SAFETY comment, so [1] can be dropped
if this series is applied.

[1] https://lore.kernel.org/rust-for-linux/20261002104550.2877930-1-tomo@flapping.org/

FUJITA Tomonori (2):
  rust: bug: document when to use the WARN macros
  rust: bug: add warn_once macro

 rust/helpers/bug.c |  11 ++++
 rust/kernel/bug.rs | 134 +++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 135 insertions(+), 10 deletions(-)


base-commit: d52549881e6777fdcc79a349028413b182a641d2
-- 
2.43.0

[PATCH v1 1/2] rust: bug: document when to use the WARN macros

[FUJITA Tomonori]

From: FUJITA Tomonori <fujita.tomonori@gmail.com>

Add a note to the module documentation, based on the comment in
`include/asm-generic/bug.h`: use the WARN macros only for kernel bugs,
not for invalid inputs from user space, the network or devices.

Also mention `panic_on_warn`, since a warning can then crash the
machine.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/bug.rs | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/rust/kernel/bug.rs b/rust/kernel/bug.rs
index 6abed2c055b1..569dc05d75ad 100644
--- a/rust/kernel/bug.rs
+++ b/rust/kernel/bug.rs
@@ -4,6 +4,11 @@
 
 //! Support for BUG and WARN functionality.
 //!
+//! Use the WARN macros only for kernel bugs that the kernel can recover
+//! from. Do not use them for invalid inputs from user space, the network or
+//! devices, or for transient conditions like `ENOMEM` or `EAGAIN`. A warning
+//! can crash the machine when `panic_on_warn` is enabled.
+//!
 //! C header: [`include/asm-generic/bug.h`](srctree/include/asm-generic/bug.h)
 
 #[macro_export]
-- 
2.43.0

[PATCH v1 2/2] rust: bug: add warn_once macro

[FUJITA Tomonori]

From: FUJITA Tomonori <fujita.tomonori@gmail.com>

Add `warn_once!`, the Rust version of C `WARN_ONCE()`. It prints a
message and reports a warning the first time the condition is true,
and returns the condition every time.

Like the generic C `WARN_ONCE()`, use `do_once_lite!` for the once
state. Add `warn_printf!`, the Rust version of C `__WARN_printf()`:

- On architectures where Rust implements WARN with inline asm (x86_64,
  arm64, riscv and s390), print the message with `__warn_printk()`,
  then trap, as the generic C `__WARN_printf()` does. x86_64 and s390
  have an optimized C version, but Rust uses the generic way for now.

- On UML, pass the format to `warn_slowpath_fmt()`, as C does.
  `warn_flags!` now shares the same call.

- On ARM and LoongArch, add a `WARN()` helper, as `warn_flags!` uses
  the `WARN_ON()` helper there.

`WARN()` is a macro, so Rust calls it through the helper. A C function
cannot pass its variadic arguments on to `WARN()`, so the helper takes
the format and one argument. Rust passes "%pA" and a pointer to one
`fmt::Arguments`, as the `pr_*!` macros do.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/helpers/bug.c |  11 ++++
 rust/kernel/bug.rs | 129 +++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 130 insertions(+), 10 deletions(-)

diff --git a/rust/helpers/bug.c b/rust/helpers/bug.c
index b51e60772578..e5973e3c8aaa 100644
--- a/rust/helpers/bug.c
+++ b/rust/helpers/bug.c
@@ -11,3 +11,14 @@ __rust_helper bool rust_helper_WARN_ON(bool cond)
 {
 	return WARN_ON(cond);
 }
+
+/*
+ * Rust uses this only on ARM and LoongArch. On x86_64 and s390, WARN()
+ * needs a string literal format, so it cannot be built there.
+ */
+#if defined(CONFIG_ARM) || defined(CONFIG_LOONGARCH)
+__rust_helper bool rust_helper_WARN(bool cond, const char *fmt, const void *arg)
+{
+	return WARN(cond, fmt, arg);
+}
+#endif
diff --git a/rust/kernel/bug.rs b/rust/kernel/bug.rs
index 569dc05d75ad..f71ff3bc7b95 100644
--- a/rust/kernel/bug.rs
+++ b/rust/kernel/bug.rs
@@ -82,6 +82,62 @@ macro_rules! warn_flags {
     }
 }
 
+#[macro_export]
+#[doc(hidden)]
+#[cfg(not(testlib))]
+#[cfg(all(CONFIG_BUG, not(CONFIG_UML), not(CONFIG_LOONGARCH), not(CONFIG_ARM)))]
+macro_rules! warn_printf {
+    ($args:expr) => {
+        const PRINT_FLAGS: u32 = $crate::bindings::BUGFLAG_NO_CUT_HERE
+            | $crate::bug::bugflag_taint($crate::bindings::TAINT_WARN);
+        let args: ::core::fmt::Arguments<'_> = $args;
+
+        // SAFETY: The format string is null-terminated and the `%pA`
+        // specifier matches the argument we are passing.
+        unsafe {
+            $crate::bindings::__warn_printk(
+                $crate::str::CStrExt::as_char_ptr(c"%pA"),
+                ::core::ptr::from_ref(&args).cast::<::core::ffi::c_void>(),
+            );
+        }
+        $crate::warn_flags!(::core::file!(), PRINT_FLAGS);
+    };
+}
+
+#[doc(hidden)]
+#[cfg(not(testlib))]
+#[cfg(all(CONFIG_BUG, CONFIG_UML))]
+pub fn warn_slowpath(
+    file: &'static crate::str::CStr,
+    line: u32,
+    args: Option<crate::fmt::Arguments<'_>>,
+) {
+    use crate::{
+        ffi::{c_int, c_void},
+        str::CStrExt,
+    };
+    use core::ptr;
+
+    let (fmt, arg) = match &args {
+        Some(args) => (c"%pA".as_char_ptr(), ptr::from_ref(args).cast::<c_void>()),
+        None => (ptr::null(), ptr::null()),
+    };
+
+    // SAFETY:
+    // - `file` is a static null-terminated string.
+    // - `TAINT_WARN` is a valid taint number.
+    // - `fmt` is either NULL, or "%pA" with `arg` pointing to a valid `fmt::Arguments`.
+    unsafe {
+        bindings::warn_slowpath_fmt(
+            file.as_char_ptr(),
+            line as c_int,
+            bindings::TAINT_WARN,
+            fmt,
+            arg,
+        )
+    };
+}
+
 #[macro_export]
 #[doc(hidden)]
 #[cfg(not(testlib))]
@@ -93,16 +149,17 @@ macro_rules! warn_flags {
             _ = $flags;
         }
 
-        // SAFETY: It is always safe to call `warn_slowpath_fmt()`
-        // with a valid null-terminated string.
-        unsafe {
-            $crate::bindings::warn_slowpath_fmt(
-                $crate::str::CStrExt::as_char_ptr($crate::c_str!(::core::file!())),
-                line!() as $crate::ffi::c_int,
-                $crate::bindings::TAINT_WARN,
-                ::core::ptr::null(),
-            );
-        }
+        $crate::bug::warn_slowpath($crate::c_str!(::core::file!()), line!(), None);
+    };
+}
+
+#[macro_export]
+#[doc(hidden)]
+#[cfg(not(testlib))]
+#[cfg(all(CONFIG_BUG, CONFIG_UML))]
+macro_rules! warn_printf {
+    ($args:expr) => {
+        $crate::bug::warn_slowpath($crate::c_str!(::core::file!()), line!(), Some($args));
     };
 }
 
@@ -122,6 +179,25 @@ macro_rules! warn_flags {
     };
 }
 
+#[macro_export]
+#[doc(hidden)]
+#[cfg(not(testlib))]
+#[cfg(all(CONFIG_BUG, any(CONFIG_LOONGARCH, CONFIG_ARM)))]
+macro_rules! warn_printf {
+    ($args:expr) => {
+        let args: ::core::fmt::Arguments<'_> = $args;
+        // SAFETY: The format string is null-terminated and the `%pA`
+        // specifier matches the argument we are passing.
+        unsafe {
+            $crate::bindings::WARN(
+                true,
+                $crate::str::CStrExt::as_char_ptr(c"%pA"),
+                ::core::ptr::from_ref(&args).cast::<::core::ffi::c_void>(),
+            )
+        }
+    };
+}
+
 #[macro_export]
 #[doc(hidden)]
 #[cfg(any(testlib, not(CONFIG_BUG)))]
@@ -134,6 +210,17 @@ macro_rules! warn_flags {
     };
 }
 
+#[macro_export]
+#[doc(hidden)]
+#[cfg(any(testlib, not(CONFIG_BUG)))]
+macro_rules! warn_printf {
+    ($args:expr) => {
+        if false {
+            _ = $args;
+        }
+    };
+}
+
 #[doc(hidden)]
 pub const fn bugflag_taint(value: u32) -> u32 {
     value << 8
@@ -159,6 +246,28 @@ macro_rules! warn_on {
     }};
 }
 
+/// Report a warning the first time `cond` is true, and return the condition's
+/// evaluation result every time.
+#[macro_export]
+macro_rules! warn_once {
+    ($cond:expr, $($arg:tt)*) => {{
+        let cond = $cond;
+
+        if cond {
+            let warn = || {
+                match $crate::prelude::fmt!($($arg)*) {
+                    args => {
+                        $crate::warn_printf!(args);
+                    }
+                }
+            };
+
+            $crate::do_once_lite!(warn());
+        }
+        cond
+    }};
+}
+
 #[cfg(CONFIG_RUST_BUG_KUNIT_TEST)]
 #[macros::kunit_tests(rust_kernel_bug)]
 mod tests {
-- 
2.43.0

Re: [PATCH v1 1/2] rust: bug: document when to use the WARN macros

[Andreas Hindborg]

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

> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
>
> Add a note to the module documentation, based on the comment in
> `include/asm-generic/bug.h`: use the WARN macros only for kernel bugs,
> not for invalid inputs from user space, the network or devices.
>
> Also mention `panic_on_warn`, since a warning can then crash the
> machine.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>


Best regards,
Andreas Hindborg