mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] rust: sync: fix safety comment for `Arc::from_raw`
@ 2026-09-23 18:32 Andreas Hindborg
  2026-09-23 18:41 ` Gary Guo
  0 siblings, 1 reply; 4+ messages in thread
From: Andreas Hindborg @ 2026-09-23 18:32 UTC (permalink / raw)
  To: Boqun Feng, Gary Guo, Alice Ryhl, Lyude Paul, Daniel Almeida,
	Onur Özkan, Miguel Ojeda, Björn Roy Baron,
	Benno Lossin, Trevor Gross, Danilo Krummrich, Tamir Duberstein,
	Alexandre Courbot
  Cc: linux-kernel, rust-for-linux, Andreas Hindborg

`Arc::from_raw` does not require `T: Send`. But if `Arc::from_raw` was
executed on a different thread than `Arc::into_raw`, `T` must implement
`Send` for the API to be sound. If this is not the case, ownership of `T`
may be sent across a thread boundary even if `T: !Send`.

Augment safety comment for `Arc::from_raw` to close this gap.

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/sync/arc.rs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index 8ae0fe6f19ec..06838ecf633c 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -276,6 +276,9 @@ pub fn as_ptr(this: &Self) -> *const T {
     ///
     /// `ptr` must have been returned by a previous call to [`Arc::into_raw`]. Additionally, it
     /// must not be called more than once for each previous call to [`Arc::into_raw`].
+    ///
+    /// If [`Arc::into_raw`] was executed on a different thread than the one executing
+    /// [`Arc::from_raw`], `T` must implement [`Send`].
     pub unsafe fn from_raw(ptr: *const T) -> Self {
         // SAFETY: The caller promises that this pointer originates from a call to `into_raw` on an
         // `Arc` that is still valid.

---
base-commit: df2908090cda368b01ff43709f51890076c56157
change-id: 20260923-aref-from-raw-safety-78a7283627fa

Best regards,
--  
Andreas Hindborg <a.hindborg@kernel.org>



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

* Re: [PATCH] rust: sync: fix safety comment for `Arc::from_raw`
  2026-09-23 18:32 [PATCH] rust: sync: fix safety comment for `Arc::from_raw` Andreas Hindborg
@ 2026-09-23 18:41 ` Gary Guo
  2026-09-23 20:34   ` Andreas Hindborg
  0 siblings, 1 reply; 4+ messages in thread
From: Gary Guo @ 2026-09-23 18:41 UTC (permalink / raw)
  To: Andreas Hindborg, Boqun Feng, Gary Guo, Alice Ryhl, Lyude Paul,
	Daniel Almeida, Onur Özkan, Miguel Ojeda,
	Björn Roy Baron, Benno Lossin, Trevor Gross,
	Danilo Krummrich, Tamir Duberstein, Alexandre Courbot
  Cc: linux-kernel, rust-for-linux

On Wed Sep 23, 2026 at 7:32 PM BST, Andreas Hindborg wrote:
> `Arc::from_raw` does not require `T: Send`. But if `Arc::from_raw` was
> executed on a different thread than `Arc::into_raw`, `T` must implement
> `Send` for the API to be sound. If this is not the case, ownership of `T`
> may be sent across a thread boundary even if `T: !Send`.
>
> Augment safety comment for `Arc::from_raw` to close this gap.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
>  rust/kernel/sync/arc.rs | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 8ae0fe6f19ec..06838ecf633c 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -276,6 +276,9 @@ pub fn as_ptr(this: &Self) -> *const T {
>      ///
>      /// `ptr` must have been returned by a previous call to [`Arc::into_raw`]. Additionally, it
>      /// must not be called more than once for each previous call to [`Arc::into_raw`].
> +    ///
> +    /// If [`Arc::into_raw`] was executed on a different thread than the one executing
> +    /// [`Arc::from_raw`], `T` must implement [`Send`].

This needs to be `Send + Sync`?

That said, I am not sure if we want to enumerate all cases where things can go
wrong. For example, would `Box::from_raw` need to mention that the type is
`Send` too? `ARef::from_raw`? `ForeignOwnable::from_foreign/borrow/borrow_mut`
all have to mention about this, too?

Best,
Gary

>      pub unsafe fn from_raw(ptr: *const T) -> Self {
>          // SAFETY: The caller promises that this pointer originates from a call to `into_raw` on an
>          // `Arc` that is still valid.
>
> ---
> base-commit: df2908090cda368b01ff43709f51890076c56157
> change-id: 20260923-aref-from-raw-safety-78a7283627fa
>
> Best regards,
> --  
> Andreas Hindborg <a.hindborg@kernel.org>



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

* Re: [PATCH] rust: sync: fix safety comment for `Arc::from_raw`
  2026-09-23 18:41 ` Gary Guo
@ 2026-09-23 20:34   ` Andreas Hindborg
  2026-09-23 20:38     ` Andreas Hindborg
  0 siblings, 1 reply; 4+ messages in thread
From: Andreas Hindborg @ 2026-09-23 20:34 UTC (permalink / raw)
  To: Gary Guo, Boqun Feng, Gary Guo, Alice Ryhl, Lyude Paul,
	Daniel Almeida, Onur Özkan, Miguel Ojeda,
	Björn Roy Baron, Benno Lossin, Trevor Gross,
	Danilo Krummrich, Tamir Duberstein, Alexandre Courbot
  Cc: linux-kernel, rust-for-linux

"Gary Guo" <gary@garyguo.net> writes:

> On Wed Sep 23, 2026 at 7:32 PM BST, Andreas Hindborg wrote:
>> `Arc::from_raw` does not require `T: Send`. But if `Arc::from_raw` was
>> executed on a different thread than `Arc::into_raw`, `T` must implement
>> `Send` for the API to be sound. If this is not the case, ownership of `T`
>> may be sent across a thread boundary even if `T: !Send`.
>>
>> Augment safety comment for `Arc::from_raw` to close this gap.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> ---
>>  rust/kernel/sync/arc.rs | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
>> index 8ae0fe6f19ec..06838ecf633c 100644
>> --- a/rust/kernel/sync/arc.rs
>> +++ b/rust/kernel/sync/arc.rs
>> @@ -276,6 +276,9 @@ pub fn as_ptr(this: &Self) -> *const T {
>>      ///
>>      /// `ptr` must have been returned by a previous call to [`Arc::into_raw`]. Additionally, it
>>      /// must not be called more than once for each previous call to [`Arc::into_raw`].
>> +    ///
>> +    /// If [`Arc::into_raw`] was executed on a different thread than the one executing
>> +    /// [`Arc::from_raw`], `T` must implement [`Send`].
>
> This needs to be `Send + Sync`?

Why does the `Arc` need to be `Sync` as well? We are transferring
ownership, not a reference.

> That said, I am not sure if we want to enumerate all cases where things can go
> wrong. For example, would `Box::from_raw` need to mention that the type is
> `Send` too? `ARef::from_raw`? `ForeignOwnable::from_foreign/borrow/borrow_mut`
> all have to mention about this, too?

I had a case where I was making unsoundness in the configfs API because
of this, but without breaking any safety requirements. I think if this
requirement had been present on `Arc`, I would have discovered the issue
while writing the safety comment for the unsafe call.

So I think it is worth it.

Best regards,
Andreas Hindborg


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

* Re: [PATCH] rust: sync: fix safety comment for `Arc::from_raw`
  2026-09-23 20:34   ` Andreas Hindborg
@ 2026-09-23 20:38     ` Andreas Hindborg
  0 siblings, 0 replies; 4+ messages in thread
From: Andreas Hindborg @ 2026-09-23 20:38 UTC (permalink / raw)
  To: Gary Guo, Boqun Feng, Gary Guo, Alice Ryhl, Lyude Paul,
	Daniel Almeida, Onur Özkan, Miguel Ojeda,
	Björn Roy Baron, Benno Lossin, Trevor Gross,
	Danilo Krummrich, Tamir Duberstein, Alexandre Courbot
  Cc: linux-kernel, rust-for-linux

Andreas Hindborg <a.hindborg@kernel.org> writes:

> "Gary Guo" <gary@garyguo.net> writes:
>
>> On Wed Sep 23, 2026 at 7:32 PM BST, Andreas Hindborg wrote:
>>> `Arc::from_raw` does not require `T: Send`. But if `Arc::from_raw` was
>>> executed on a different thread than `Arc::into_raw`, `T` must implement
>>> `Send` for the API to be sound. If this is not the case, ownership of `T`
>>> may be sent across a thread boundary even if `T: !Send`.
>>>
>>> Augment safety comment for `Arc::from_raw` to close this gap.
>>>
>>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>>> ---
>>>  rust/kernel/sync/arc.rs | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
>>> index 8ae0fe6f19ec..06838ecf633c 100644
>>> --- a/rust/kernel/sync/arc.rs
>>> +++ b/rust/kernel/sync/arc.rs
>>> @@ -276,6 +276,9 @@ pub fn as_ptr(this: &Self) -> *const T {
>>>      ///
>>>      /// `ptr` must have been returned by a previous call to [`Arc::into_raw`]. Additionally, it
>>>      /// must not be called more than once for each previous call to [`Arc::into_raw`].
>>> +    ///
>>> +    /// If [`Arc::into_raw`] was executed on a different thread than the one executing
>>> +    /// [`Arc::from_raw`], `T` must implement [`Send`].
>>
>> This needs to be `Send + Sync`?
>
> Why does the `Arc` need to be `Sync` as well? We are transferring
> ownership, not a reference.

Ah, I get your point. The comment is wrong, I meant to say "`Arc<T>`
must implement `Send`". I did not mean to address `T`.

Best regards,
Andreas Hindborg



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

end of thread, other threads:[~2026-09-23 20:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-23 18:32 [PATCH] rust: sync: fix safety comment for `Arc::from_raw` Andreas Hindborg
2026-09-23 18:41 ` Gary Guo
2026-09-23 20:34   ` Andreas Hindborg
2026-09-23 20:38     ` Andreas Hindborg

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®