linux-kernel - Re: [PATCH V8 00/14] Rust bindings for cpufreq and OPP core + sample driver

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] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANiq72kuebpOa4aPxmTXNMA0eo-SLL+Ht9u1SGHymXBF5_92eA@mail.gmail.com>
Date: Mon, 17 Feb 2025 09:39:08 +0100
From: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To: Viresh Kumar <viresh.kumar@...aro.org>
Cc: Danilo Krummrich <dakr@...nel.org>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	Danilo Krummrich <dakr@...hat.com>, Alex Gaynor <alex.gaynor@...il.com>, 
	Alice Ryhl <aliceryhl@...gle.com>, Andreas Hindborg <a.hindborg@...nel.org>, 
	Benno Lossin <benno.lossin@...ton.me>, Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Michael Turquette <mturquette@...libre.com>, Miguel Ojeda <ojeda@...nel.org>, Nishanth Menon <nm@...com>, 
	Peter Zijlstra <peterz@...radead.org>, Rasmus Villemoes <linux@...musvillemoes.dk>, 
	Stephen Boyd <sboyd@...nel.org>, Thomas Gleixner <tglx@...utronix.de>, Trevor Gross <tmgross@...ch.edu>, 
	Viresh Kumar <vireshk@...nel.org>, Yury Norov <yury.norov@...il.com>, linux-pm@...r.kernel.org, 
	Vincent Guittot <vincent.guittot@...aro.org>, rust-for-linux@...r.kernel.org, 
	Manos Pitsidianakis <manos.pitsidianakis@...aro.org>, Erik Schilling <erik.schilling@...aro.org>, 
	Alex Bennée <alex.bennee@...aro.org>, 
	Joakim Bech <joakim.bech@...aro.org>, Rob Herring <robh@...nel.org>, 
	Anisse Astier <anisse@...ier.eu>, linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V8 00/14] Rust bindings for cpufreq and OPP core + sample driver

On Mon, Feb 10, 2025 at 9:06 AM Viresh Kumar <viresh.kumar@...aro.org> wrote:
>
> error[E0133]: use of mutable static is unsafe and requires unsafe block
>    --> /mnt/ssd/all/work/repos/kernel/linux/rust/kernel/cpufreq.rs:632:26
>     |
> 632 |             addr_of_mut!(bindings::cpufreq_freq_attr_scaling_available_freqs) as *mut _;
>     |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ use of mutable static

Ah, I see now -- yeah, this is due to:

    https://blog.rust-lang.org/2024/10/17/Rust-1.82.0.html#safely-addressing-unsafe-statics

You could do (probably with a comment):

        pub fn new(name: &'static CStr, data: T::Data, flags: u16,
boost: bool) -> Result<Self> {
    +        #![allow(unused_unsafe)]
    +
            let mut drv = KBox::new(

Yeah, a bit annoying... :(

> I don't remember seeing a CLIPPY warning ever about removing the
> unsafe here, as reported in your case.

Please use a newer version to see them, e.g. the latest stable 1.84.1.

In general, please test patches with the minimum version and the
latest stable. The latest will give you more lints in general, and the
minimum will make sure it builds for older versions.

> It would require clippy::undocumented-unsafe-blocks to be disabled, in
> this case.

Hmm... why? We need the `allow` above because we need to keep the
`unsafe` for older Rust. But you can provide a comment nevertheless,
even if "dummy", so you should not need to disable anything else.

With your branch + the `allow` above + running `rustfmt`, it is Clippy
clean for me. Please see the diff below as an example (I also cleaned
the other Clippy warning -- and sorry for the wrapping).

Cheers,
Miguel

diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index d2e7913e170b..e7c62770fc3b 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -602,6 +602,8 @@ unsafe impl<T: Driver> Send for Registration<T> {}
 impl<T: Driver> Registration<T> {
     /// Registers a cpufreq driver with the rest of the kernel.
     pub fn new(name: &'static CStr, data: T::Data, flags: u16, boost:
bool) -> Result<Self> {
+        #![allow(unused_unsafe)]
+
         let mut drv = KBox::new(
             UnsafeCell::new(bindings::cpufreq_driver::default()),
             GFP_KERNEL,
@@ -626,19 +628,15 @@ pub fn new(name: &'static CStr, data: T::Data,
flags: u16, boost: bool) -> Resul
         let mut attr = KBox::new([ptr::null_mut(); 3], GFP_KERNEL)?;
         let mut next = 0;

-        // SAFETY: The C code returns a valid pointer here, which is
again passed to the C code in
-        // an array.
-        attr[next] = unsafe {
-            addr_of_mut!(bindings::cpufreq_freq_attr_scaling_available_freqs)
as *mut _
-        };
+        attr[next] =
+            // SAFETY: ...
+            unsafe {
addr_of_mut!(bindings::cpufreq_freq_attr_scaling_available_freqs) as
*mut _ };
         next += 1;

         if boost {
-            // SAFETY: The C code returns a valid pointer here, which
is again passed to the C code
-            // in an array.
-            attr[next] = unsafe {
-
addr_of_mut!(bindings::cpufreq_freq_attr_scaling_boost_freqs) as *mut
_
-            };
+            attr[next] =
+                // SAFETY: ...
+                unsafe {
addr_of_mut!(bindings::cpufreq_freq_attr_scaling_boost_freqs) as *mut
_ };
             next += 1;
         }
         attr[next] = ptr::null_mut();
diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
index b83bd97a4f37..aaf650f46ccb 100644
--- a/rust/kernel/opp.rs
+++ b/rust/kernel/opp.rs
@@ -781,7 +781,6 @@ fn drop(&mut self) {
 /// represents a pointer that owns a reference count on the OPP.
 ///
 /// A reference to the `OPP`, `&OPP` isn't refcounted by the Rust code.
-
 #[repr(transparent)]
 pub struct OPP(Opaque<bindings::dev_pm_opp>);