| Message ID | 20250409065802.136971-5-fujita.tomonori@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | rust: Add bug/warn abstractions | expand |
On Wed, Apr 09, 2025 at 03:58:01PM +0900, FUJITA Tomonori wrote: > Add warn_on macro, uses the BUG/WARN feature (lib/bug.c) via assembly > for x86_64/arm64/riscv. > > The current Rust code simply wraps BUG() macro but doesn't provide the > proper debug information. The BUG/WARN feature can only be used from > assembly. > > This uses the assembly code exported by the C side via ARCH_WARN_ASM > macro. To avoid duplicating the assembly code, this approach follows > the same strategy as the static branch code: it generates the assembly > code for Rust using the C preprocessor at compile time. > > Similarly, ARCH_WARN_REACHABLE is also used at compile time to > generate the assembly code; objtool's reachable anotation code. It's > used for only architectures that use objtool. > > For now, Loongarch and arm32 just use a wrapper for WARN macro. > > UML doesn't use the assembly BUG/WARN feature; just wrapping generic > BUG/WARN functions implemented in C works. > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> I don't object to this change, but I would STRONGLY recommend never using this in any driver if at all possible. Only use this if the system is in such a state that the only way out is to reboot the kernel, which is what both WARN() and BUG() will do. Note, any way that a user can trigger either of these code paths will result in a CVE, so don't do that either. Almost always just properly handle the issue and propagate up the error to the caller. thanks, gre gk-h
On Wed, Apr 09, 2025 at 03:58:01PM +0900, FUJITA Tomonori wrote: > Add warn_on macro, uses the BUG/WARN feature (lib/bug.c) via assembly > for x86_64/arm64/riscv. > > The current Rust code simply wraps BUG() macro but doesn't provide the > proper debug information. The BUG/WARN feature can only be used from > assembly. > > This uses the assembly code exported by the C side via ARCH_WARN_ASM > macro. To avoid duplicating the assembly code, this approach follows > the same strategy as the static branch code: it generates the assembly > code for Rust using the C preprocessor at compile time. > > Similarly, ARCH_WARN_REACHABLE is also used at compile time to > generate the assembly code; objtool's reachable anotation code. It's > used for only architectures that use objtool. > > For now, Loongarch and arm32 just use a wrapper for WARN macro. > > UML doesn't use the assembly BUG/WARN feature; just wrapping generic > BUG/WARN functions implemented in C works. > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> > --- > rust/Makefile | 8 ++ > rust/helpers/bug.c | 5 + > rust/kernel/.gitignore | 2 + > rust/kernel/bug.rs | 114 ++++++++++++++++++ > rust/kernel/generated_arch_reachable_asm.rs.S | 7 ++ > rust/kernel/generated_arch_warn_asm.rs.S | 7 ++ > rust/kernel/lib.rs | 1 + > 7 files changed, 144 insertions(+) > create mode 100644 rust/kernel/bug.rs > create mode 100644 rust/kernel/generated_arch_reachable_asm.rs.S > create mode 100644 rust/kernel/generated_arch_warn_asm.rs.S > > diff --git a/rust/Makefile b/rust/Makefile > index fa0eea8a9eca..25f498607d1b 100644 > --- a/rust/Makefile > +++ b/rust/Makefile > @@ -34,6 +34,9 @@ obj-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated.o > obj-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated_kunit.o > > always-$(subst y,$(CONFIG_RUST),$(CONFIG_JUMP_LABEL)) += kernel/generated_arch_static_branch_asm.rs > +ifndef CONFIG_UML > +always-$(subst y,$(CONFIG_RUST),$(CONFIG_BUG)) += kernel/generated_arch_warn_asm.rs kernel/generated_arch_reachable_asm.rs > +endif > > # Avoids running `$(RUSTC)` when it may not be available. > ifdef CONFIG_RUST > @@ -536,5 +539,10 @@ $(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/build_error.o $(obj)/pin_init.o \ > ifdef CONFIG_JUMP_LABEL > $(obj)/kernel.o: $(obj)/kernel/generated_arch_static_branch_asm.rs > endif > +ifndef CONFIG_UML > +ifdef CONFIG_BUG > +$(obj)/kernel.o: $(obj)/kernel/generated_arch_warn_asm.rs $(obj)/kernel/generated_arch_reachable_asm.rs > +endif > +endif > > endif # CONFIG_RUST > diff --git a/rust/helpers/bug.c b/rust/helpers/bug.c > index e2d13babc737..a62c96f507d1 100644 > --- a/rust/helpers/bug.c > +++ b/rust/helpers/bug.c > @@ -6,3 +6,8 @@ __noreturn void rust_helper_BUG(void) > { > BUG(); > } > + > +bool rust_helper_WARN_ON(bool cond) > +{ > + return WARN_ON(cond); > +} > diff --git a/rust/kernel/.gitignore b/rust/kernel/.gitignore > index 6ba39a178f30..f636ad95aaf3 100644 > --- a/rust/kernel/.gitignore > +++ b/rust/kernel/.gitignore > @@ -1,3 +1,5 @@ > # SPDX-License-Identifier: GPL-2.0 > > /generated_arch_static_branch_asm.rs > +/generated_arch_warn_asm.rs > +/generated_arch_reachable_asm.rs > diff --git a/rust/kernel/bug.rs b/rust/kernel/bug.rs > new file mode 100644 > index 000000000000..761f0c49ae04 > --- /dev/null > +++ b/rust/kernel/bug.rs > @@ -0,0 +1,114 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +// Copyright (C) 2024, 2025 FUJITA Tomonori <fujita.tomonori@gmail.com> > + > +//! Support for BUG and WARN functionality. > +//! > +//! C header: [`include/asm-generic/bug.h`](srctree/include/asm-generic/bug.h) > + > +#[macro_export] > +#[doc(hidden)] > +#[cfg(all(CONFIG_BUG, not(CONFIG_UML), not(CONFIG_LOONGARCH), not(CONFIG_ARM)))] > +#[cfg(CONFIG_DEBUG_BUGVERBOSE)] > +macro_rules! warn_flags { > + ($flags:expr) => { > + const FLAGS: u32 = $crate::bindings::BUGFLAG_WARNING | $flags; > + const _FILE: &[u8] = file!().as_bytes(); > + // Plus one for null-terminator. > + static FILE: [u8; _FILE.len() + 1] = { > + let mut bytes = [0; _FILE.len() + 1]; > + let mut i = 0; > + while i < _FILE.len() { > + bytes[i] = _FILE[i]; > + i += 1; > + } > + bytes > + }; > + // SAFETY: Just an FFI call. Safety comments could be improved. This being an FFI call is not the reason why it's okay. Furthermore, it's not an FFI call. Otherwise, this series LGTM. Alice
On Thu, 10 Apr 2025 07:39:48 +0000 Alice Ryhl <aliceryhl@google.com> wrote: >> diff --git a/rust/kernel/bug.rs b/rust/kernel/bug.rs >> new file mode 100644 >> index 000000000000..761f0c49ae04 >> --- /dev/null >> +++ b/rust/kernel/bug.rs >> @@ -0,0 +1,114 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> + >> +// Copyright (C) 2024, 2025 FUJITA Tomonori <fujita.tomonori@gmail.com> >> + >> +//! Support for BUG and WARN functionality. >> +//! >> +//! C header: [`include/asm-generic/bug.h`](srctree/include/asm-generic/bug.h) >> + >> +#[macro_export] >> +#[doc(hidden)] >> +#[cfg(all(CONFIG_BUG, not(CONFIG_UML), not(CONFIG_LOONGARCH), not(CONFIG_ARM)))] >> +#[cfg(CONFIG_DEBUG_BUGVERBOSE)] >> +macro_rules! warn_flags { >> + ($flags:expr) => { >> + const FLAGS: u32 = $crate::bindings::BUGFLAG_WARNING | $flags; >> + const _FILE: &[u8] = file!().as_bytes(); >> + // Plus one for null-terminator. >> + static FILE: [u8; _FILE.len() + 1] = { >> + let mut bytes = [0; _FILE.len() + 1]; >> + let mut i = 0; >> + while i < _FILE.len() { >> + bytes[i] = _FILE[i]; >> + i += 1; >> + } >> + bytes >> + }; >> + // SAFETY: Just an FFI call. > > Safety comments could be improved. This being an FFI call is not the > reason why it's okay. Furthermore, it's not an FFI call. > > Otherwise, this series LGTM. SAFETY: It's always safe to embed metadata such as the source file name, line number, and flags into the .bug_table ELF section. Looks good?
On Wed, 9 Apr 2025 09:38:53 +0200 Greg KH <gregkh@linuxfoundation.org> wrote: > On Wed, Apr 09, 2025 at 03:58:01PM +0900, FUJITA Tomonori wrote: >> Add warn_on macro, uses the BUG/WARN feature (lib/bug.c) via assembly >> for x86_64/arm64/riscv. >> >> The current Rust code simply wraps BUG() macro but doesn't provide the >> proper debug information. The BUG/WARN feature can only be used from >> assembly. >> >> This uses the assembly code exported by the C side via ARCH_WARN_ASM >> macro. To avoid duplicating the assembly code, this approach follows >> the same strategy as the static branch code: it generates the assembly >> code for Rust using the C preprocessor at compile time. >> >> Similarly, ARCH_WARN_REACHABLE is also used at compile time to >> generate the assembly code; objtool's reachable anotation code. It's >> used for only architectures that use objtool. >> >> For now, Loongarch and arm32 just use a wrapper for WARN macro. >> >> UML doesn't use the assembly BUG/WARN feature; just wrapping generic >> BUG/WARN functions implemented in C works. >> >> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> > > I don't object to this change, but I would STRONGLY recommend never > using this in any driver if at all possible. Only use this if the > system is in such a state that the only way out is to reboot the kernel, > which is what both WARN() and BUG() will do. Totally agree. As stated in the commit log, BUG() is already used in multiple locations within the Rust abstractions. I'm not trying to introduce BUG() or WARN() as new features. This patchset aims to improve the information provided by BUG() and WARN().
diff --git a/rust/Makefile b/rust/Makefile index fa0eea8a9eca..25f498607d1b 100644 --- a/rust/Makefile +++ b/rust/Makefile @@ -34,6 +34,9 @@ obj-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated.o obj-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated_kunit.o always-$(subst y,$(CONFIG_RUST),$(CONFIG_JUMP_LABEL)) += kernel/generated_arch_static_branch_asm.rs +ifndef CONFIG_UML +always-$(subst y,$(CONFIG_RUST),$(CONFIG_BUG)) += kernel/generated_arch_warn_asm.rs kernel/generated_arch_reachable_asm.rs +endif # Avoids running `$(RUSTC)` when it may not be available. ifdef CONFIG_RUST @@ -536,5 +539,10 @@ $(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/build_error.o $(obj)/pin_init.o \ ifdef CONFIG_JUMP_LABEL $(obj)/kernel.o: $(obj)/kernel/generated_arch_static_branch_asm.rs endif +ifndef CONFIG_UML +ifdef CONFIG_BUG +$(obj)/kernel.o: $(obj)/kernel/generated_arch_warn_asm.rs $(obj)/kernel/generated_arch_reachable_asm.rs +endif +endif endif # CONFIG_RUST diff --git a/rust/helpers/bug.c b/rust/helpers/bug.c index e2d13babc737..a62c96f507d1 100644 --- a/rust/helpers/bug.c +++ b/rust/helpers/bug.c @@ -6,3 +6,8 @@ __noreturn void rust_helper_BUG(void) { BUG(); } + +bool rust_helper_WARN_ON(bool cond) +{ + return WARN_ON(cond); +} diff --git a/rust/kernel/.gitignore b/rust/kernel/.gitignore index 6ba39a178f30..f636ad95aaf3 100644 --- a/rust/kernel/.gitignore +++ b/rust/kernel/.gitignore @@ -1,3 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 /generated_arch_static_branch_asm.rs +/generated_arch_warn_asm.rs +/generated_arch_reachable_asm.rs diff --git a/rust/kernel/bug.rs b/rust/kernel/bug.rs new file mode 100644 index 000000000000..761f0c49ae04 --- /dev/null +++ b/rust/kernel/bug.rs @@ -0,0 +1,114 @@ +// SPDX-License-Identifier: GPL-2.0 + +// Copyright (C) 2024, 2025 FUJITA Tomonori <fujita.tomonori@gmail.com> + +//! Support for BUG and WARN functionality. +//! +//! C header: [`include/asm-generic/bug.h`](srctree/include/asm-generic/bug.h) + +#[macro_export] +#[doc(hidden)] +#[cfg(all(CONFIG_BUG, not(CONFIG_UML), not(CONFIG_LOONGARCH), not(CONFIG_ARM)))] +#[cfg(CONFIG_DEBUG_BUGVERBOSE)] +macro_rules! warn_flags { + ($flags:expr) => { + const FLAGS: u32 = $crate::bindings::BUGFLAG_WARNING | $flags; + const _FILE: &[u8] = file!().as_bytes(); + // Plus one for null-terminator. + static FILE: [u8; _FILE.len() + 1] = { + let mut bytes = [0; _FILE.len() + 1]; + let mut i = 0; + while i < _FILE.len() { + bytes[i] = _FILE[i]; + i += 1; + } + bytes + }; + // SAFETY: Just an FFI call. + unsafe { + $crate::asm!( + concat!( + "/* {size} */", + include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_warn_asm.rs")), + include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_reachable_asm.rs"))); + file = sym FILE, + line = const line!(), + flags = const FLAGS, + size = const ::core::mem::size_of::<$crate::bindings::bug_entry>(), + ); + } + } +} + +#[macro_export] +#[doc(hidden)] +#[cfg(all(CONFIG_BUG, not(CONFIG_UML), not(CONFIG_LOONGARCH), not(CONFIG_ARM)))] +#[cfg(not(CONFIG_DEBUG_BUGVERBOSE))] +macro_rules! warn_flags { + ($flags:expr) => { + const FLAGS: u32 = $crate::bindings::BUGFLAG_WARNING | $flags; + // SAFETY: Just an FFI call. + unsafe { + $crate::asm!( + concat!( + "/* {size} */", + include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_warn_asm.rs")), + include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_reachable_asm.rs"))); + flags = const FLAGS, + size = const ::core::mem::size_of::<$crate::bindings::bug_entry>(), + ); + } + } +} + +#[macro_export] +#[doc(hidden)] +#[cfg(all(CONFIG_BUG, CONFIG_UML))] +macro_rules! warn_flags { + ($flags:expr) => { + // SAFETY: Just an FFI call. + unsafe { + $crate::bindings::warn_slowpath_fmt( + $crate::c_str!(::core::file!()).as_ptr() as *const $crate::ffi::c_char, + line!() as i32, + $flags as u32, + ::core::ptr::null() as *const $crate::ffi::c_char, + ); + } + }; +} + +#[macro_export] +#[doc(hidden)] +#[cfg(all(CONFIG_BUG, any(CONFIG_LOONGARCH, CONFIG_ARM)))] +macro_rules! warn_flags { + ($flags:expr) => { + // SAFETY: Just an FFI call. + unsafe { $crate::bindings::WARN_ON(true) } + }; +} + +#[macro_export] +#[doc(hidden)] +#[cfg(not(CONFIG_BUG))] +macro_rules! warn_flags { + ($flags:expr) => {}; +} + +#[doc(hidden)] +pub const fn bugflag_taint(value: u32) -> u32 { + value << 8 +} + +/// Report a warning if `cond` is true and return the condition's evaluation result. +#[macro_export] +macro_rules! warn_on { + ($cond:expr) => {{ + if $cond { + const WARN_ON_FLAGS: u32 = $crate::bug::bugflag_taint($crate::bindings::TAINT_WARN); + + $crate::warn_flags!(WARN_ON_FLAGS); + } + $cond + }}; +} diff --git a/rust/kernel/generated_arch_reachable_asm.rs.S b/rust/kernel/generated_arch_reachable_asm.rs.S new file mode 100644 index 000000000000..3886a9ad3a99 --- /dev/null +++ b/rust/kernel/generated_arch_reachable_asm.rs.S @@ -0,0 +1,7 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#include <linux/bug.h> + +// Cut here. + +::kernel::concat_literals!(ARCH_WARN_REACHABLE) diff --git a/rust/kernel/generated_arch_warn_asm.rs.S b/rust/kernel/generated_arch_warn_asm.rs.S new file mode 100644 index 000000000000..409eb4c2d3a1 --- /dev/null +++ b/rust/kernel/generated_arch_warn_asm.rs.S @@ -0,0 +1,7 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#include <linux/bug.h> + +// Cut here. + +::kernel::concat_literals!(ARCH_WARN_ASM("{file}", "{line}", "{flags}", "{size}")) diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index de07aadd1ff5..bc13e92bdb1e 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -40,6 +40,7 @@ pub mod alloc; #[cfg(CONFIG_BLOCK)] pub mod block; +pub mod bug; #[doc(hidden)] pub mod build_assert; pub mod cred;
Add warn_on macro, uses the BUG/WARN feature (lib/bug.c) via assembly for x86_64/arm64/riscv. The current Rust code simply wraps BUG() macro but doesn't provide the proper debug information. The BUG/WARN feature can only be used from assembly. This uses the assembly code exported by the C side via ARCH_WARN_ASM macro. To avoid duplicating the assembly code, this approach follows the same strategy as the static branch code: it generates the assembly code for Rust using the C preprocessor at compile time. Similarly, ARCH_WARN_REACHABLE is also used at compile time to generate the assembly code; objtool's reachable anotation code. It's used for only architectures that use objtool. For now, Loongarch and arm32 just use a wrapper for WARN macro. UML doesn't use the assembly BUG/WARN feature; just wrapping generic BUG/WARN functions implemented in C works. Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> --- rust/Makefile | 8 ++ rust/helpers/bug.c | 5 + rust/kernel/.gitignore | 2 + rust/kernel/bug.rs | 114 ++++++++++++++++++ rust/kernel/generated_arch_reachable_asm.rs.S | 7 ++ rust/kernel/generated_arch_warn_asm.rs.S | 7 ++ rust/kernel/lib.rs | 1 + 7 files changed, 144 insertions(+) create mode 100644 rust/kernel/bug.rs create mode 100644 rust/kernel/generated_arch_reachable_asm.rs.S create mode 100644 rust/kernel/generated_arch_warn_asm.rs.S