linux-kernel - Re: [PATCH v5 2/2] rust: add tracepoint support

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: <a381ef22-96f4-4a72-8e3a-cc023dee111a@proton.me>
Date: Fri, 02 Aug 2024 17:23:59 +0000
From: Benno Lossin <benno.lossin@...ton.me>
To: Alice Ryhl <aliceryhl@...gle.com>, Steven Rostedt <rostedt@...dmis.org>, Masami Hiramatsu <mhiramat@...nel.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Peter Zijlstra <peterz@...radead.org>, Josh Poimboeuf <jpoimboe@...nel.org>, Jason Baron <jbaron@...mai.com>, Ard Biesheuvel <ardb@...nel.org>, Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, Wedson Almeida Filho <wedsonaf@...il.com>, Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, Björn Roy Baron <bjorn3_gh@...tonmail.com>, Andreas Hindborg <a.hindborg@...sung.com>
Cc: linux-trace-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org, Arnd Bergmann <arnd@...db.de>, linux-arch@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>, Sean Christopherson <seanjc@...gle.com>, Uros Bizjak <ubizjak@...il.com>, Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>, Marc Zyngier <maz@...nel.org>, Oliver Upton <oliver.upton@...ux.dev>, Mark Rutland <mark.rutland@....com>, Ryan Roberts <ryan.roberts@....com>, Fuad Tabba <tabba@...gle.com>, linux-arm-kernel@...ts.infradead.org, Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>, Anup Patel <apatel@...tanamicro.com>, Andrew Jones <ajones@...tanamicro.com>, Alexandre Ghiti <alexghiti@...osinc.com>, Conor Dooley <conor.dooley@...rochip.com>,
	Samuel Holland <samuel.holland@...ive.com>, linux-riscv@...ts.infradead.org, Huacai Chen <chenhuacai@...nel.org>, WANG Xuerui <kernel@...0n.name>, Bibo Mao <maobibo@...ngson.cn>, Tiezhu Yang <yangtiezhu@...ngson.cn>, Andrew Morton <akpm@...ux-foundation.org>, Tianrui Zhao <zhaotianrui@...ngson.cn>, loongarch@...ts.linux.dev, Carlos Llamas <cmllamas@...gle.com>
Subject: Re: [PATCH v5 2/2] rust: add tracepoint support

On 02.08.24 11:31, Alice Ryhl wrote:
> diff --git a/rust/kernel/tracepoint.rs b/rust/kernel/tracepoint.rs
> new file mode 100644
> index 000000000000..69dafdb8bfe8
> --- /dev/null
> +++ b/rust/kernel/tracepoint.rs
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +// Copyright (C) 2024 Google LLC.
> +
> +//! Logic for tracepoints.
> +
> +/// Declare the Rust entry point for a tracepoint.
> +///
> +/// This macro generates an unsafe function that calls into C, and its safety requirements will be
> +/// whatever the relevant C code requires. To document these safety requirements, you may add
> +/// doc-comments when invoking the macro.

I think we should mandate safety documentation for the function.

> +#[macro_export]
> +macro_rules! declare_trace {
> +    ($($(#[$attr:meta])* $pub:vis fn $name:ident($($argname:ident : $argtyp:ty),* $(,)?);)*) => {$(

Can you add an `unsafe` in front of `fn`, since this macro generates an
`unsafe` function? Otherwise I don't see how the SAFETY comment below is
correct.

---
Cheers,
Benno

> +        $( #[$attr] )*
> +        #[inline(always)]
> +        $pub unsafe fn $name($($argname : $argtyp),*) {
> +            #[cfg(CONFIG_TRACEPOINTS)]
> +            {
> +                // SAFETY: It's always okay to query the static key for a tracepoint.
> +                let should_trace = unsafe {
> +                    $crate::macros::paste! {
> +                        $crate::static_key::static_key_false!(
> +                            $crate::bindings::[< __tracepoint_ $name >],
> +                            $crate::bindings::tracepoint,
> +                            key
> +                        )
> +                    }
> +                };
> +
> +                if should_trace {
> +                    $crate::macros::paste! {
> +                        // SAFETY: The caller guarantees that it is okay to call this tracepoint.
> +                        unsafe { $crate::bindings::[< rust_do_trace_ $name >]($($argname),*) };
> +                    }
> +                }
> +            }
> +
> +            #[cfg(not(CONFIG_TRACEPOINTS))]
> +            {
> +                // If tracepoints are disabled, insert a trivial use of each argument
> +                // to avoid unused argument warnings.
> +                $( let _unused = $argname; )*
> +            }
> +        }
> +    )*}
> +}
> +
> +pub use declare_trace;
> 
> --
> 2.46.0.rc2.264.g509ed76dc8-goog
>