Message-ID: <CANiq72nP=u49vhj7+Z_digM+gKk0_=oAWUofbmyntyPsKy=+ew@mail.gmail.com>
Date: Fri, 11 Jul 2025 22:14:13 +0200
From: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To: Joel Fernandes <joelagnelf@...dia.com>
Cc: Danilo Krummrich <dakr@...nel.org>, abdiel.janulgue@...il.com, 
	daniel.almeida@...labora.com, robin.murphy@....com, a.hindborg@...nel.org, 
	ojeda@...nel.org, alex.gaynor@...il.com, boqun.feng@...il.com, 
	gary@...yguo.net, bjorn3_gh@...tonmail.com, lossin@...nel.org, 
	aliceryhl@...gle.com, tmgross@...ch.edu, bhelgaas@...gle.com, 
	kwilczynski@...nel.org, gregkh@...uxfoundation.org, rafael@...nel.org, 
	rust-for-linux@...r.kernel.org, linux-pci@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/5] rust: dma: add DMA addressing capabilities

On Fri, Jul 11, 2025 at 9:35 PM Joel Fernandes <joelagnelf@...dia.com> wrote:
>
> 2. Since the Rust code is wrapping around the C code, the data race is
> happening entirely on the C side right? So can we just rely on KCSAN to catch
> concurrency issues instead of marking the callers as unsafe? I feel the
> unsafe { } really might make the driver code ugly.
>
> 3. Maybe we could document this issue than enforce it via unsafe? My concern
> is wrapping unsafe { } makes the calling code ugly.

Yeah, this sort of dilemma comes up from time to time, yeah, e.g. see:

    https://lore.kernel.org/rust-for-linux/CANiq72k_NNFsQ=GGCsur34CTYhSFC0m=mHS83mTB8HQCDBcW=w@mail.gmail.com/
    https://lore.kernel.org/rust-for-linux/CANiq72m3WFj9Eb2iRUM3mLFibWW+cupAoNQt+cqtNa4O9=jq7Q@mail.gmail.com/

In short: that is the job of `unsafe {}` -- if we start to avoid it
just to make code prettier, then it loses its power.

There are few alternatives/notes, though:

  - If there is a way to somehow guarantee or check that something is
safe, perhaps with a tool like Klint, then that could allow us to
avoid `unsafe`.

  - If the blocks are very repetitive in a single user and don't add
any value, then one could consider having users write a single `unsafe
{}` where they promise to uphold X instead of requiring it in further
calls.

  - Worst case, we can promote something to the potential ASH list
idea ("Acknowledged Soundness Holes"): a list where we document things
that do not require `unsafe {}` that should require it but don't for
strong practical reasons.

> 5. In theory, all rust bindings wrappers are unsafe and we do mark it around
> the bindings call, right? But in this case, we're also making the calling
> code of the unsafe caller as unsafe. C code is 'unsafe' obviously from Rust
> PoV but I am not sure we worry about the internal implementation-unsafety of
> the C code because then maybe most bindings wrappers would need to be unsafe,
> not only these DMA ones.

It is orthogonal -- you may have a safe function that uses `unsafe {}`
inside (e.g. to call a C function), but also you will see unsafe
functions with just safe code inside. And, of course, safe functions
with only safe code inside and unsafe functions with `unsafe {}`
blocks inside.

In other words, a safe function does not mean unsafe code is used or
not inside. Similarly, an unsafe function does not mean unsafe code is
used (or not) inside either.

This is the usual "two meaning of `unsafe`" -- that of e.g. functions
(where it means there are safety preconditions for calling a function)
and that of e.g. `unsafe {}` blocks (where it means the caller must
play by the rules, e.g. satisfy the safety preconditions to call an
unsafe function).

So a Rust function that calls C functions (and thus uses `unsafe {}`)
may or may not need to be unsafe -- it all depends on the case. That
is, it depends on whether callers can cause UB or not. And similarly,
a Rust function that does not use unsafe (including not calling C
functions) can still be very much unsafe, because other code may rely
on that code for soundness (e.g. an unsafe method that assigns to an
internal pointer which then other safe methods rely on).

Cheers,
Miguel

Powered by blists - more mailing lists