mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3] rust: cpufreq: reject NULL from cpufreq_cpu_get()
@ 2026-08-29 16:10 Mehmet Koseoglu
  2026-08-29 19:52 ` Onur Özkan
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Mehmet Koseoglu @ 2026-08-29 16:10 UTC (permalink / raw)
  To: rafael, viresh.kumar, ojeda
  Cc: boqun, gary, bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross,
	dakr, daniel.almeida, tamird, acourbot, work, linux-pm,
	rust-for-linux, linux-kernel

cpufreq_cpu_get() returns either a referenced policy or NULL.
PolicyCpu::from_cpu() passed its return value to from_err_ptr(), which
rejects ERR_PTR values but accepts NULL.

If the lookup fails, Policy::from_raw_mut() therefore constructs a mutable
reference from NULL. Dropping the resulting PolicyCpu then passes the
invalid pointer to cpufreq_cpu_put(), causing an oops in kobject_put().

Reject NULL with NonNull before constructing the Policy reference. Return
ENODEV instead.

A KUnit negative-control run reproduced the oops with the original
conversion. The same test passed with this change. The reproducer is
available on request.

Fixes: 6ebdd7c93177 ("rust: cpufreq: Extend abstractions for policy and driver ops")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Mehmet Koseoglu <mehmet.mkoseoglu@gmail.com>
---
Changes in v3:
- Add Cc: stable@vger.kernel.org.

Changes in v2:
- Format modified imports using the kernel's vertical import style.

v2: https://lore.kernel.org/r/20260828161834.29539-1-mehmet.mkoseoglu@gmail.com

 rust/kernel/cpufreq.rs | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index affa2b9490ef..4b992ea9a0f0 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -14,7 +14,13 @@
     cpumask,
     device::{Bound, Device},
     devres,
-    error::{code::*, from_err_ptr, from_result, to_result, Result, VTABLE_DEFAULT_ERROR},
+    error::{
+        code::*,
+        from_result,
+        to_result,
+        Result,
+        VTABLE_DEFAULT_ERROR, //
+    },
     ffi::{c_char, c_ulong},
     prelude::*,
     types::ForeignOwnable,
@@ -29,7 +35,10 @@
     marker::PhantomData,
     ops::{Deref, DerefMut},
     pin::Pin,
-    ptr,
+    ptr::{
+        self,
+        NonNull, //
+    },
 };
 
 use macros::vtable;
@@ -687,12 +696,13 @@ fn clear_data<T: ForeignOwnable>(&mut self) -> Option<T> {
 impl<'a> PolicyCpu<'a> {
     fn from_cpu(cpu: CpuId) -> Result<Self> {
         // SAFETY: It is safe to call `cpufreq_cpu_get` for any valid CPU.
-        let ptr = from_err_ptr(unsafe { bindings::cpufreq_cpu_get(u32::from(cpu)) })?;
+        let ptr =
+            NonNull::new(unsafe { bindings::cpufreq_cpu_get(u32::from(cpu)) }).ok_or(ENODEV)?;
 
         Ok(Self(
             // SAFETY: The `ptr` is guaranteed to be valid and remains valid for the lifetime of
             // the returned reference.
-            unsafe { Policy::from_raw_mut(ptr) },
+            unsafe { Policy::from_raw_mut(ptr.as_ptr()) },
         ))
     }
 }
-- 
2.55.0

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] rust: cpufreq: reject NULL from cpufreq_cpu_get()
  2026-08-29 16:10 [PATCH v3] rust: cpufreq: reject NULL from cpufreq_cpu_get() Mehmet Koseoglu
@ 2026-08-29 19:52 ` Onur Özkan
  2026-09-28 16:22 ` spidermana
  2026-10-01 10:19 ` [PATCH v4] " Mehmet Koseoglu
  2 siblings, 0 replies; 9+ messages in thread
From: Onur Özkan @ 2026-08-29 19:52 UTC (permalink / raw)
  To: Mehmet Koseoglu
  Cc: rafael, viresh.kumar, ojeda, boqun, gary, bjorn3_gh, lossin,
	a.hindborg, aliceryhl, tmgross, dakr, daniel.almeida, tamird,
	acourbot, linux-pm, rust-for-linux, linux-kernel,
	Onur Özkan

On Sat, 29 Aug 2026 19:10:13 +0300
Mehmet Koseoglu <mehmet.mkoseoglu@gmail.com> wrote:

> cpufreq_cpu_get() returns either a referenced policy or NULL.
> PolicyCpu::from_cpu() passed its return value to from_err_ptr(), which
> rejects ERR_PTR values but accepts NULL.
> 
> If the lookup fails, Policy::from_raw_mut() therefore constructs a mutable
> reference from NULL. Dropping the resulting PolicyCpu then passes the
> invalid pointer to cpufreq_cpu_put(), causing an oops in kobject_put().
> 
> Reject NULL with NonNull before constructing the Policy reference. Return
> ENODEV instead.
> 
> A KUnit negative-control run reproduced the oops with the original
> conversion. The same test passed with this change. The reproducer is
> available on request.
> 
> Fixes: 6ebdd7c93177 ("rust: cpufreq: Extend abstractions for policy and driver ops")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Mehmet Koseoglu <mehmet.mkoseoglu@gmail.com>

Reviewed-by: Onur Özkan <work@onurozkan.dev>

> ---
> Changes in v3:
> - Add Cc: stable@vger.kernel.org.
> 
> Changes in v2:
> - Format modified imports using the kernel's vertical import style.
> 
> v2: https://lore.kernel.org/r/20260828161834.29539-1-mehmet.mkoseoglu@gmail.com
> 
>  rust/kernel/cpufreq.rs | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
> index affa2b9490ef..4b992ea9a0f0 100644
> --- a/rust/kernel/cpufreq.rs
> +++ b/rust/kernel/cpufreq.rs
> @@ -14,7 +14,13 @@
>      cpumask,
>      device::{Bound, Device},
>      devres,
> -    error::{code::*, from_err_ptr, from_result, to_result, Result, VTABLE_DEFAULT_ERROR},
> +    error::{
> +        code::*,
> +        from_result,
> +        to_result,
> +        Result,
> +        VTABLE_DEFAULT_ERROR, //
> +    },
>      ffi::{c_char, c_ulong},
>      prelude::*,
>      types::ForeignOwnable,
> @@ -29,7 +35,10 @@
>      marker::PhantomData,
>      ops::{Deref, DerefMut},
>      pin::Pin,
> -    ptr,
> +    ptr::{
> +        self,
> +        NonNull, //
> +    },
>  };
>  
>  use macros::vtable;
> @@ -687,12 +696,13 @@ fn clear_data<T: ForeignOwnable>(&mut self) -> Option<T> {
>  impl<'a> PolicyCpu<'a> {
>      fn from_cpu(cpu: CpuId) -> Result<Self> {
>          // SAFETY: It is safe to call `cpufreq_cpu_get` for any valid CPU.
> -        let ptr = from_err_ptr(unsafe { bindings::cpufreq_cpu_get(u32::from(cpu)) })?;
> +        let ptr =
> +            NonNull::new(unsafe { bindings::cpufreq_cpu_get(u32::from(cpu)) }).ok_or(ENODEV)?;
>  
>          Ok(Self(
>              // SAFETY: The `ptr` is guaranteed to be valid and remains valid for the lifetime of
>              // the returned reference.
> -            unsafe { Policy::from_raw_mut(ptr) },
> +            unsafe { Policy::from_raw_mut(ptr.as_ptr()) },
>          ))
>      }
>  }
> -- 
> 2.55.0

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] rust: cpufreq: reject NULL from cpufreq_cpu_get()
  2026-08-29 16:10 [PATCH v3] rust: cpufreq: reject NULL from cpufreq_cpu_get() Mehmet Koseoglu
  2026-08-29 19:52 ` Onur Özkan
@ 2026-09-28 16:22 ` spidermana
  2026-09-28 17:17   ` Miguel Ojeda
  2026-10-01 10:19 ` [PATCH v4] " Mehmet Koseoglu
  2 siblings, 1 reply; 9+ messages in thread
From: spidermana @ 2026-09-28 16:22 UTC (permalink / raw)
  To: mehmet.mkoseoglu
  Cc: a.hindborg, acourbot, aliceryhl, bjorn3_gh, boqun, dakr,
	daniel.almeida, gary, linux-kernel, linux-pm, lossin, ojeda,
	rafael, rust-for-linux, tamird, tmgross, viresh.kumar, work

On Sat, 29 Aug 2026 19:10:13 +0300, Mehmet Koseoglu wrote:
> cpufreq_cpu_get() returns either a referenced policy or NULL.
> PolicyCpu::from_cpu() passed its return value to from_err_ptr(), which
> rejects ERR_PTR values but accepts NULL.
[...]
>          Ok(Self(
>              // SAFETY: The `ptr` is guaranteed to be valid and remains valid for the lifetime of
>              // the returned reference.
> -            unsafe { Policy::from_raw_mut(ptr) },
> +            unsafe { Policy::from_raw_mut(ptr.as_ptr()) },

Just a quick nit here, maybe rephrase this SAFETY comment, e.g.:

            // SAFETY: `ptr` is non-NULL and `cpufreq_cpu_get()` took a reference on it, so it is
            // valid for writing and remains valid for the lifetime of the returned reference.

Either way, looks good to me.
FWIW, I hit this too and have a reproducer [1].
[1] https://github.com/Rust-for-Linux/linux/issues/1258

Reviewed-by: Elowen Xu <malanalyzing@gmail.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] rust: cpufreq: reject NULL from cpufreq_cpu_get()
  2026-09-28 16:22 ` spidermana
@ 2026-09-28 17:17   ` Miguel Ojeda
  2026-09-29  9:55     ` spidermana
  0 siblings, 1 reply; 9+ messages in thread
From: Miguel Ojeda @ 2026-09-28 17:17 UTC (permalink / raw)
  To: spidermana
  Cc: mehmet.mkoseoglu, a.hindborg, acourbot, aliceryhl, bjorn3_gh,
	boqun, dakr, daniel.almeida, gary, linux-kernel, linux-pm,
	lossin, ojeda, rafael, rust-for-linux, tamird, tmgross,
	viresh.kumar, work

On Mon, Sep 28, 2026 at 6:22 PM spidermana <xuyiwen14@gmail.com> wrote:
>
> Either way, looks good to me.
> FWIW, I hit this too and have a reproducer [1].
> [1] https://github.com/Rust-for-Linux/linux/issues/1258
>
> Reviewed-by: Elowen Xu <malanalyzing@gmail.com>

Thanks for taking the time to send the tag! :)

The From: doesn't seem to match the email of the tag, is that intended?

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] rust: cpufreq: reject NULL from cpufreq_cpu_get()
  2026-09-28 17:17   ` Miguel Ojeda
@ 2026-09-29  9:55     ` spidermana
  2026-10-01  6:25       ` Viresh Kumar
  0 siblings, 1 reply; 9+ messages in thread
From: spidermana @ 2026-09-29  9:55 UTC (permalink / raw)
  To: miguel.ojeda.sandonis
  Cc: a.hindborg, acourbot, aliceryhl, bjorn3_gh, boqun, dakr,
	daniel.almeida, gary, linux-kernel, linux-pm, lossin,
	mehmet.mkoseoglu, ojeda, rafael, rust-for-linux, tamird, tmgross,
	viresh.kumar, work, xuyiwen14

On Mon, 28 Sep 2026 19:17:08 +0200, Miguel Ojeda wrote:
> The From: doesn't seem to match the email of the tag, is that intended?

Oops, no, thanks for noticing! Please disregard the tag in my
previous reply. the correct one is:

Reviewed-by: spidermana <xuyiwen14@gmail.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] rust: cpufreq: reject NULL from cpufreq_cpu_get()
  2026-09-29  9:55     ` spidermana
@ 2026-10-01  6:25       ` Viresh Kumar
  2026-10-01  9:49         ` Mehmet Koseoglu
  0 siblings, 1 reply; 9+ messages in thread
From: Viresh Kumar @ 2026-10-01  6:25 UTC (permalink / raw)
  To: spidermana
  Cc: miguel.ojeda.sandonis, a.hindborg, acourbot, aliceryhl,
	bjorn3_gh, boqun, dakr, daniel.almeida, gary, linux-kernel,
	linux-pm, lossin, mehmet.mkoseoglu, ojeda, rafael,
	rust-for-linux, tamird, tmgross, work

On 29-09-26, 11:55, spidermana wrote:
> On Mon, 28 Sep 2026 19:17:08 +0200, Miguel Ojeda wrote:
> > The From: doesn't seem to match the email of the tag, is that intended?
> 
> Oops, no, thanks for noticing! Please disregard the tag in my
> previous reply. the correct one is:
> 
> Reviewed-by: spidermana <xuyiwen14@gmail.com>

Please resend the patch with all the fixes / tags / improvements. Thanks.

-- 
viresh

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] rust: cpufreq: reject NULL from cpufreq_cpu_get()
  2026-10-01  6:25       ` Viresh Kumar
@ 2026-10-01  9:49         ` Mehmet Koseoglu
  2026-10-01  9:58           ` Viresh Kumar
  0 siblings, 1 reply; 9+ messages in thread
From: Mehmet Koseoglu @ 2026-10-01  9:49 UTC (permalink / raw)
  To: Viresh Kumar, spidermana
  Cc: miguel.ojeda.sandonis, a.hindborg, acourbot, aliceryhl,
	bjorn3_gh, boqun, dakr, daniel.almeida, gary, linux-kernel,
	linux-pm, lossin, mehmet.mkoseoglu, ojeda, rafael,
	rust-for-linux, tamird, tmgross, work

On Thu Oct 1, 2026 at 9:25 AM +03, Viresh Kumar wrote:
> On 29-09-26, 11:55, spidermana wrote:
>> On Mon, 28 Sep 2026 19:17:08 +0200, Miguel Ojeda wrote:
>> > The From: doesn't seem to match the email of the tag, is that intended?
>> 
>> Oops, no, thanks for noticing! Please disregard the tag in my
>> previous reply. the correct one is:
>> 
>> Reviewed-by: spidermana <xuyiwen14@gmail.com>
>
> Please resend the patch with all the fixes / tags / improvements. Thanks.

Hi Viresh,

I see that v3 was already applied as 0ba1ee052b3f. Would you prefer
a follow-up patch updating the SAFETY comment, or a v4 of the original
patch with the comment update and collected review tags?

Thanks,
Mehmet


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] rust: cpufreq: reject NULL from cpufreq_cpu_get()
  2026-10-01  9:49         ` Mehmet Koseoglu
@ 2026-10-01  9:58           ` Viresh Kumar
  0 siblings, 0 replies; 9+ messages in thread
From: Viresh Kumar @ 2026-10-01  9:58 UTC (permalink / raw)
  To: Mehmet Koseoglu
  Cc: spidermana, miguel.ojeda.sandonis, a.hindborg, acourbot,
	aliceryhl, bjorn3_gh, boqun, dakr, daniel.almeida, gary,
	linux-kernel, linux-pm, lossin, ojeda, rafael, rust-for-linux,
	tamird, tmgross, work

On 01-10-26, 12:49, Mehmet Koseoglu wrote:
> I see that v3 was already applied as 0ba1ee052b3f. Would you prefer
> a follow-up patch updating the SAFETY comment, or a v4 of the original
> patch with the comment update and collected review tags?

Oops, I thought I haven't applied it yet. I did that couple of week ago before I
went on vacation :(

You can resend the patch and I will apply the new one and drop the old one.

-- 
viresh

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v4] rust: cpufreq: reject NULL from cpufreq_cpu_get()
  2026-08-29 16:10 [PATCH v3] rust: cpufreq: reject NULL from cpufreq_cpu_get() Mehmet Koseoglu
  2026-08-29 19:52 ` Onur Özkan
  2026-09-28 16:22 ` spidermana
@ 2026-10-01 10:19 ` Mehmet Koseoglu
  2 siblings, 0 replies; 9+ messages in thread
From: Mehmet Koseoglu @ 2026-10-01 10:19 UTC (permalink / raw)
  To: rafael, viresh.kumar, ojeda
  Cc: Mehmet Koseoglu, boqun, gary, bjorn3_gh, lossin, a.hindborg,
	aliceryhl, tmgross, dakr, daniel.almeida, tamird, acourbot, work,
	linux-pm, rust-for-linux, linux-kernel, xuyiwen14,
	miguel.ojeda.sandonis, stable

cpufreq_cpu_get() returns either a referenced policy or NULL.
PolicyCpu::from_cpu() passed its return value to from_err_ptr(), which
rejects ERR_PTR values but accepts NULL.

If the lookup fails, Policy::from_raw_mut() therefore constructs a mutable
reference from NULL. Dropping the resulting PolicyCpu then passes the
invalid pointer to cpufreq_cpu_put(), causing an oops in kobject_put().

Reject NULL with NonNull before constructing the Policy reference. Return
ENODEV instead.

A KUnit negative-control run reproduced the oops with the original
conversion. The same test passed with this change. The reproducer is
available on request.

Fixes: 6ebdd7c93177 ("rust: cpufreq: Extend abstractions for policy and driver ops")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Mehmet Koseoglu <mehmet.mkoseoglu@gmail.com>
Reviewed-by: Onur Özkan <work@onurozkan.dev>
Reviewed-by: spidermana <xuyiwen14@gmail.com>
---
No worries at all, here is a v4 update with the comment update and
review tags.

Changes in v4:
- Clarify that the policy pointer is non-NULL and cpufreq_cpu_get() holds
  a reference for the lifetime of the returned reference.
- Add Reviewed-by tags from Onur Özkan and spidermana (using the corrected
  tag from spidermana's follow-up).
- Resend at Viresh's request to replace the previously applied v3.

Changes in v3:
- Add Cc: stable@vger.kernel.org.

Changes in v2:
- Format modified imports using the kernel's vertical import style.

v3: https://lore.kernel.org/r/20260829161013.102967-1-mehmet.mkoseoglu@gmail.com
v2: https://lore.kernel.org/r/20260828161834.29539-1-mehmet.mkoseoglu@gmail.com

 rust/kernel/cpufreq.rs | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index affa2b9490ef..b75d692846a2 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -14,7 +14,13 @@
     cpumask,
     device::{Bound, Device},
     devres,
-    error::{code::*, from_err_ptr, from_result, to_result, Result, VTABLE_DEFAULT_ERROR},
+    error::{
+        code::*,
+        from_result,
+        to_result,
+        Result,
+        VTABLE_DEFAULT_ERROR, //
+    },
     ffi::{c_char, c_ulong},
     prelude::*,
     types::ForeignOwnable,
@@ -29,7 +35,10 @@
     marker::PhantomData,
     ops::{Deref, DerefMut},
     pin::Pin,
-    ptr,
+    ptr::{
+        self,
+        NonNull, //
+    },
 };
 
 use macros::vtable;
@@ -687,12 +696,13 @@ fn clear_data<T: ForeignOwnable>(&mut self) -> Option<T> {
 impl<'a> PolicyCpu<'a> {
     fn from_cpu(cpu: CpuId) -> Result<Self> {
         // SAFETY: It is safe to call `cpufreq_cpu_get` for any valid CPU.
-        let ptr = from_err_ptr(unsafe { bindings::cpufreq_cpu_get(u32::from(cpu)) })?;
+        let ptr =
+            NonNull::new(unsafe { bindings::cpufreq_cpu_get(u32::from(cpu)) }).ok_or(ENODEV)?;
 
         Ok(Self(
-            // SAFETY: The `ptr` is guaranteed to be valid and remains valid for the lifetime of
-            // the returned reference.
+            // SAFETY: `ptr` is non-NULL and `cpufreq_cpu_get()` took a reference on it, so it is
+            // valid for writing and remains valid for the lifetime of the returned reference.
-            unsafe { Policy::from_raw_mut(ptr) },
+            unsafe { Policy::from_raw_mut(ptr.as_ptr()) },
         ))
     }
 }
-- 
2.55.0

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-10-01 10:22 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-29 16:10 [PATCH v3] rust: cpufreq: reject NULL from cpufreq_cpu_get() Mehmet Koseoglu
2026-08-29 19:52 ` Onur Özkan
2026-09-28 16:22 ` spidermana
2026-09-28 17:17   ` Miguel Ojeda
2026-09-29  9:55     ` spidermana
2026-10-01  6:25       ` Viresh Kumar
2026-10-01  9:49         ` Mehmet Koseoglu
2026-10-01  9:58           ` Viresh Kumar
2026-10-01 10:19 ` [PATCH v4] " Mehmet Koseoglu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®