linux-kernel - Re: [PATCH v2 03/14] rust: sync: add `Arc::as

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: <ad3e4f4c-883b-49a5-ba4b-562c13eee08e@de.bosch.com>
Date: Tue, 1 Oct 2024 06:56:13 +0200
From: Dirk Behme <dirk.behme@...bosch.com>
To: Benno Lossin <benno.lossin@...ton.me>, Andreas Hindborg
	<a.hindborg@...nel.org>, Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor
	<alex.gaynor@...il.com>, Anna-Maria Behnsen <anna-maria@...utronix.de>,
	Frederic Weisbecker <frederic@...nel.org>, Thomas Gleixner
	<tglx@...utronix.de>
CC: Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, Alice Ryhl
	<aliceryhl@...gle.com>, <rust-for-linux@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 03/14] rust: sync: add `Arc::as_ptr`

On 19.09.2024 16:03, Benno Lossin wrote:
> On 18.09.24 00:27, Andreas Hindborg wrote:
>> Add a method to get a pointer to the data contained in an `Arc`.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@...nel.org>
>> ---
>>   rust/kernel/sync/arc.rs | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
>> index 3673496c2363..a57ea3e2b44c 100644
>> --- a/rust/kernel/sync/arc.rs
>> +++ b/rust/kernel/sync/arc.rs
>> @@ -258,6 +258,14 @@ pub fn into_raw(self) -> *const T {
>>           unsafe { core::ptr::addr_of!((*ptr).data) }
>>       }
>>
>> +    /// Return a raw pointer to the data in this arc.
>> +    pub fn as_ptr(&self) -> *const T {
> 
> I don't know if we have a convention for this, but shouldn't this be an
> associated function? Because if `T` also has an `as_ptr` function, it
> will be shadowed by this one.

Yes. In Fabien's out of tree regmap we have an as_ptr() for Regmap [1] 
which operates on &Arc<Regmap> [2]. Once this patch is applied to arc.rs 
the compilation fails as then Arc.as_ptr() is used, not the 
Regmap.as_ptr() any more [3]. Switching this to something like [4] makes 
the compiler happy.

Thanks,

Dirk

P.S.: Just to learn something: For the unmodified, failing case: Is 
there a rule when which as_ptr() will be used? Is there an order rule 
for the shadowing? Any documentation link?


[1] 
https://github.com/Fabo/linux/blob/fparent/rust-ncv6336/rust/kernel/regmap.rs#L71

[2] 
https://github.com/Fabo/linux/blob/fparent/rust-ncv6336/rust/kernel/regulator/driver.rs#L418

[3]

error[E0308]: mismatched types
    --> rust/kernel/regulator/driver.rs:420:33
     |
420 |             config.cfg.regmap = regmap.as_ptr();
     |                                 ^^^^^^^^^^^^^^^ types differ in 
mutability
     |
     = note: expected raw pointer `*mut bindings::regmap`
                found raw pointer `*const Regmap`

error: aborting due to 1 previous error

[4]

diff --git a/rust/kernel/hrtimer/arc.rs b/rust/kernel/hrtimer/arc.rs
index ff04b0b75bb39..7c39ab440e1c6 100644
--- a/rust/kernel/hrtimer/arc.rs
+++ b/rust/kernel/hrtimer/arc.rs
@@ -25,7 +25,7 @@ unsafe impl<U> TimerHandle for ArcTimerHandle<U>
      U: HasTimer<U>,
  {
      fn cancel(&mut self) -> bool {
-        let self_ptr = self.inner.as_ptr();
+        let self_ptr = Arc::as_ptr(&self.inner);

          // SAFETY: As we obtained `self_ptr` from a valid reference 
above, it
          // must point to a valid `U`.
@@ -57,7 +57,7 @@ impl<U> TimerPointer for Arc<U>
      fn schedule(self, expires: Ktime) -> ArcTimerHandle<U> {
          // SAFETY: Since we generate the pointer passed to `schedule` 
from a
          // valid reference, it is a valid pointer.
-        unsafe { U::schedule(self.as_ptr(), expires) };
+        unsafe { U::schedule(Arc::as_ptr(&self), expires) };

          ArcTimerHandle { inner: self }
      }
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index 1466d9cd41652..0a314c2f4c5ea 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -259,8 +259,8 @@ pub fn into_raw(self) -> *const T {
      }

      /// Return a raw pointer to the data in this arc.
-    pub fn as_ptr(&self) -> *const T {
-        let ptr = self.ptr.as_ptr();
+    pub fn as_ptr(arc: &Self) -> *const T {
+        let ptr = arc.ptr.as_ptr();
          // SAFETY: As we derive the pointer from a reference above, 
the pointer
          // must be valid.
          unsafe { core::ptr::addr_of!((*ptr).data) }