From: Robin Murphy <robin.murphy@arm.com>
To: Ralf Jung <post@ralfj.de>, Alice Ryhl <aliceryhl@google.com>
Cc: "Boqun Feng" <boqun.feng@gmail.com>, comex <comexk@gmail.com>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Daniel Almeida" <daniel.almeida@collabora.com>,
"Benno Lossin" <benno.lossin@proton.me>,
"Abdiel Janulgue" <abdiel.janulgue@gmail.com>,
dakr@kernel.org, rust-for-linux@vger.kernel.org,
"Miguel Ojeda" <ojeda@kernel.org>,
"Alex Gaynor" <alex.gaynor@gmail.com>,
"Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Trevor Gross" <tmgross@umich.edu>,
"Valentin Obst" <kernel@valentinobst.de>,
linux-kernel@vger.kernel.org, "Christoph Hellwig" <hch@lst.de>,
"Marek Szyprowski" <m.szyprowski@samsung.com>,
airlied@redhat.com, iommu@lists.linux.dev, lkmm@lists.linux.dev
Subject: Re: Allow data races on some read/write operations
Date: Wed, 5 Mar 2025 14:40:51 +0000 [thread overview]
Message-ID: <39aa741e-8522-497d-a8f2-d43bc93fb29f@arm.com> (raw)
In-Reply-To: <3aabca39-4658-454a-b0e3-e946e72977e1@ralfj.de>
On 05/03/2025 1:27 pm, Ralf Jung wrote:
> Hi,
>
> On 05.03.25 14:23, Alice Ryhl wrote:
>> On Wed, Mar 5, 2025 at 2:10 PM Ralf Jung <post@ralfj.de> wrote:
>>>
>>> Hi,
>>>
>>> On 05.03.25 04:24, Boqun Feng wrote:
>>>> On Tue, Mar 04, 2025 at 12:18:28PM -0800, comex wrote:
>>>>>
>>>>>> On Mar 4, 2025, at 11:03 AM, Ralf Jung <post@ralfj.de> wrote:
>>>>> However, these optimizations should rarely trigger misbehavior in
>>>>> practice, so I wouldn’t be surprised if Linux had some code that
>>>>> expected memcpy to act volatile…
>>>>>
>>>>
>>>> Also in this particular case we are discussing [1], it's a memcpy (from
>>>> or to) a DMA buffer, which means the device can also read or write the
>>>> memory, therefore the content of the memory may be altered outside the
>>>> program (the kernel), so we cannot use copy_nonoverlapping() I believe.
>>>>
>>>> [1]: https://lore.kernel.org/rust-for-linux/87bjuil15w.fsf@kernel.org/
>>>
>>> Is there actually a potential for races (with reads by hardware, not
>>> other
>>> threads) on the memcpy'd memory? Or is this the pattern where you
>>> copy some data
>>> somewhere and then set a flag in an MMIO register to indicate that
>>> the data is
>>> ready and the device can start reading it? In the latter case, the
>>> actual data
>>> copy does not race with anything, so it can be a regular non-atomic
>>> non-volatile
>>> memcpy. The flag write *should* be a release write, and release
>>> volatile writes
>>> do not exist, so that is a problem, but it's a separate problem from
>>> volatile
>>> memcpy. One can use a release fence followed by a relaxed write instead.
>>> Volatile writes do not currently act like relaxed writes, but you
>>> need that
>>> anyway for WRITE_ONCE to make sense so it seems fine to rely on that
>>> here as well.
>>>
>>> Rust should have atomic volatile accesses, and various ideas have
>>> been proposed
>>> over the years, but sadly nobody has shown up to try and push this
>>> through.
>>>
>>> If the memcpy itself can indeed race, you need an atomic volatile
>>> memcpy --
>>> which neither C nor Rust have, though there are proposals for atomic
>>> memcpy (and
>>> arguably, there should be a way to interact with a device using
>>> non-volatile
>>> atomics... but anyway in the LKMM, atomics are modeled with volatile,
>>> so things
>>> are even more entangled than usual ;).
>>
>> For some kinds of hardware, we might not want to trust the hardware.
>> I.e., there is no race under normal operation, but the hardware could
>> have a bug or be malicious and we might not want that to result in UB.
>> This is pretty similar to syscalls that take a pointer into userspace
>> memory and read it - userspace shouldn't modify that memory during the
>> syscall, but it can and if it does, that should be well-defined.
>> (Though in the case of userspace, the copy happens in asm since it
>> also needs to deal with virtual memory and so on.)
>
> Wow you are really doing your best to combine all the hard problems at
> the same time. ;)
> Sharing memory with untrusted parties is another tricky issue, and even
> leaving aside all the theoretical trouble, practically speaking you'll
> want to exclusively use atomic accesses to interact with such memory. So
> doing this properly requires atomic memcpy. I don't know what that is
> blocked on, but it is good to know that it would help the kernel.
If you don't trust the device then I wouldn't think it actually matters
what happens at this level - the higher-level driver is already going to
have to carefully check and sanitise whatever data it reads back from
the buffer before consuming it, at which point reading a torn value due
to a race would be essentially indistinguishable from if the device had
gone wrong and simply written that nonsense value itself.
I think the more significant case is when polling for the device to
write back some kind of status word, where in C code the driver would
use READ_ONCE() to ensure a single-copy-atomic read of the same size the
device is going to write - sticking a regular memcpy() into the middle
of that can't necessarily be trusted to work correctly (even if it may
appear to 99% of the time).
Thanks,
Robin.
next prev parent reply other threads:[~2025-03-05 14:40 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <6dea7b6a-1534-47e7-94d2-d67417c3d4c1@proton.me>
[not found] ` <tnwDK3QN_Xr0Yoa3U8HVxS5OqjvxIhgmmO_ifTGJR_EtIzjoxawOHtnbOJ9yChsUWXyFPcU9beIdrgbpfGZI8w==@protonmail.internalid>
[not found] ` <3202F69F-397E-4BC4-8DD8-E2D4B0AB056F@collabora.com>
[not found] ` <87bjuil15w.fsf@kernel.org>
[not found] ` <t4HxdvR7WBX_861hiTXo72jqC9F9oRpIzgA_dD2yhcSuLISEkC-shMfSgllrFPpnkSZXGfRcc47keudMooNiIQ==@protonmail.internalid>
[not found] ` <CAH5fLgg5MuUu=TX8mMsPf5RcLhMLHSU4Vct=h8rFX6Z7HjPxeA@mail.gmail.com>
[not found] ` <87ikoqjg1n.fsf@kernel.org>
[not found] ` <KpWTCfIlcLYFBpSvWPfALJ9VQn5a99_RAvxgMBc1aCrSalPB-qaW9IhXyaDG7HM1AcFPX5chj_Yr7IQp3F7UqA==@protonmail.internalid>
[not found] ` <CAH5fLgh6ubawHh76wq7JPbcuBCWhm91m7Rc01MVsX-a3C6qaVA@mail.gmail.com>
[not found] ` <87mse2hrd8.fsf@kernel.org>
2025-03-03 20:08 ` Allow data races on some read/write operations Boqun Feng
2025-03-04 19:03 ` Ralf Jung
2025-03-04 20:18 ` comex
2025-03-05 3:24 ` Boqun Feng
2025-03-05 13:10 ` Ralf Jung
2025-03-05 13:23 ` Alice Ryhl
2025-03-05 13:27 ` Ralf Jung
2025-03-05 14:40 ` Robin Murphy [this message]
2025-03-05 18:43 ` Andreas Hindborg
2025-03-05 19:30 ` Alan Stern
2025-03-05 19:42 ` Ralf Jung
2025-03-05 21:26 ` Andreas Hindborg
2025-03-05 21:53 ` Ralf Jung
2025-03-07 8:43 ` Andreas Hindborg
2025-03-18 14:44 ` Ralf Jung
2025-03-05 18:41 ` Andreas Hindborg
2025-03-05 14:25 ` Daniel Almeida
2025-03-05 18:38 ` Andreas Hindborg
2025-03-05 22:01 ` Ralf Jung
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=39aa741e-8522-497d-a8f2-d43bc93fb29f@arm.com \
--to=robin.murphy@arm.com \
--cc=a.hindborg@kernel.org \
--cc=abdiel.janulgue@gmail.com \
--cc=airlied@redhat.com \
--cc=alex.gaynor@gmail.com \
--cc=aliceryhl@google.com \
--cc=benno.lossin@proton.me \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=comexk@gmail.com \
--cc=dakr@kernel.org \
--cc=daniel.almeida@collabora.com \
--cc=gary@garyguo.net \
--cc=hch@lst.de \
--cc=iommu@lists.linux.dev \
--cc=kernel@valentinobst.de \
--cc=linux-kernel@vger.kernel.org \
--cc=lkmm@lists.linux.dev \
--cc=m.szyprowski@samsung.com \
--cc=ojeda@kernel.org \
--cc=post@ralfj.de \
--cc=rust-for-linux@vger.kernel.org \
--cc=tmgross@umich.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).