linux-kernel - Re: [PATCH v4 4/4] rust: Add warn

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <20250409.152645.2045309190127414357.fujita.tomonori@gmail.com>
Date: Wed, 09 Apr 2025 15:26:45 +0900 (JST)
From: FUJITA Tomonori <fujita.tomonori@...il.com>
To: aliceryhl@...gle.com
Cc: fujita.tomonori@...il.com, linux-kernel@...r.kernel.org,
 rust-for-linux@...r.kernel.org, x86@...nel.org,
 linux-riscv@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org,
 loongarch@...ts.linux.dev, tglx@...utronix.de, mingo@...hat.com,
 bp@...en8.de, dave.hansen@...ux.intel.com, peterz@...radead.org,
 hpa@...or.com, paul.walmsley@...ive.com, palmer@...belt.com,
 aou@...s.berkeley.edu, catalin.marinas@....com, will@...nel.org,
 chenhuacai@...nel.org, kernel@...0n.name, tangyouling@...ngson.cn,
 hejinyang@...ngson.cn, yangtiezhu@...ngson.cn, ojeda@...nel.org,
 alex.gaynor@...il.com, boqun.feng@...il.com, gary@...yguo.net,
 bjorn3_gh@...tonmail.com, benno.lossin@...ton.me, a.hindborg@...nel.org,
 tmgross@...ch.edu
Subject: Re: [PATCH v4 4/4] rust: Add warn_on macro

On Tue, 8 Apr 2025 15:55:38 +0200
Alice Ryhl <aliceryhl@...gle.com> wrote:

> On Wed, Mar 5, 2025 at 12:10 PM FUJITA Tomonori
> <fujita.tomonori@...il.com> 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 just uses 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@...il.com>
> 
> Overall looks reasonable. A few nits below:

Thanks!

>> +#[macro_export]
>> +#[doc(hidden)]
>> +#[cfg(all(CONFIG_BUG, not(CONFIG_UML), not(CONFIG_LOONGARCH)))]
>> +#[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>(),
> 
> The indentation here is odd. Shouldn't the arguments be indented one more?

Yeah, fixed.


>> +#[macro_export]
>> +#[doc(hidden)]
>> +#[cfg(all(CONFIG_BUG, not(CONFIG_UML), not(CONFIG_LOONGARCH)))]
>> +#[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>(),
> 
> Same thing here.

Fixed.

>> +            );
>> +        }
>> +    }
>> +}
>> +
>> +#[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 ::core::ffi::c_uchar,
>> +                line!() as i32,
>> +                $flags as u32,
>> +                ::core::ptr::null() as *const ::core::ffi::c_uchar,
> 
> Why unsigned char? I think this should be ::kernel::ffi::c_char to
> utilize the custom integer type definitions in kernel rather than the
> faulty ones in core.
> 
> (Maybe $crate rather than kernel)

Oops, replaced them with $crate::ffi::c_char