[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <98177196-09ac-4678-950d-81f3f1d487b8@nvidia.com>
Date: Tue, 28 Oct 2025 15:02:46 -0700
From: John Hubbard <jhubbard@...dia.com>
To: Alice Ryhl <aliceryhl@...gle.com>, Lyude Paul <lyude@...hat.com>
Cc: dri-devel@...ts.freedesktop.org, rust-for-linux@...r.kernel.org,
linux-kernel@...r.kernel.org, Danilo Krummrich <dakr@...nel.org>,
Abdiel Janulgue <abdiel.janulgue@...il.com>,
Daniel Almeida <daniel.almeida@...labora.com>,
Robin Murphy <robin.murphy@....com>, Andreas Hindborg
<a.hindborg@...nel.org>, Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>,
Gary Guo <gary@...yguo.net>, Björn Roy Baron
<bjorn3_gh@...tonmail.com>, Benno Lossin <lossin@...nel.org>,
Trevor Gross <tmgross@...ch.edu>
Subject: Re: [PATCH] rust/dma: Take &mut self in
CoherentAllocation::field_write()
On 10/28/25 2:52 PM, Alice Ryhl wrote:
> On Tue, Oct 28, 2025 at 05:18:01PM -0400, Lyude Paul wrote:
>> At the moment - CoherentAllocation::field_write() only takes an immutable
>> reference to self. This means it's possible for a user to mistakenly call
>> field_write() while Rust still has a slice taken out for the coherent
>> allocation:
>>
>> let alloc: CoherentAllocation<CoolStruct> = /* … */;
>>
>> let evil_slice = unsafe { alloc.as_slice(/* … */)? };
>> dma_write!(alloc[1].cool_field = 42); /* UB! */
>>
>> Keep in mind: the above example is technically a violation of the safety
>> contract of as_slice(), so luckily this detail shouldn't currently be
>> causing any UB in the kernel. But, there's no reason we should be solely
>> relying on the safety contract for enforcing this when we can just use a
>> mutable reference and already do so in other parts of the API.
>>
>> Signed-off-by: Lyude Paul <lyude@...hat.com>
>> Cc: Danilo Krummrich <dakr@...nel.org>
>
> Didn't we do this intentionally so that it's possible to write to
> different parts of the allocation without protecting the entire region
> with a lock?
>
If so, that seems like it was a good decision, IMHO. Because with
DMA, you can't use CPU-side Rust code to provide full safety (the
device is blithely unaware of any of this, and can scribble all over
the area at any time). And while you could prevent the CPU-side code
from interfering with itself in a dma region, the downside is that
we'll take a performance hit and even a deadlock risk, and run slower
than the non-Rust DMA code in the kernel.
thanks,
--
John Hubbard
Powered by blists - more mailing lists