[PATCH v6 1/4] rust: io: forbid plain accesses racing with `SysMem`

[Andreas Hindborg <a.hindborg@kernel.org>]

`SysMem` accesses memory with volatile loads and stores and with the
kernel's `memcpy()`. Both are well-defined when racing with hardware or
user space, and when racing with atomic or volatile accesses from other
kernel code. However, if they race with plain, non-atomic accesses from
kernel code, they are considered a data race, and therefore undefined
behavior.

The invariants of `SysMem` only require the region to be valid and
aligned kernel accessible memory. Nothing stops a caller from creating
a `SysMem` over memory that other kernel code accesses with plain loads
and stores at the same time.

Make the requirement explicit. Require that no plain access to the
region races with an access performed through the view, define what a
plain access and a race is, and update the safety comments on the
backend implementation and on the existing callers of `SysMem::new` to
rely on the new invariant.

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/dma.rs           |  8 +++++++-
 rust/kernel/drm/gem/shmem.rs |  2 ++
 rust/kernel/io.rs            | 48 +++++++++++++++++++++++++++++---------------
 3 files changed, 41 insertions(+), 17 deletions(-)

diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index 580e1c3427bc..445b5859c60b 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -1237,7 +1237,13 @@ impl<'a, T: ?Sized + KnownSize> IoBase<'a> for &'a Coherent<'_, T> {
     #[inline]
     fn as_view(self) -> CoherentView<'a, Self::Target> {
         CoherentView {
-            // SAFETY: `cpu_addr` is valid and aligned kernel accessible memory.
+            // SAFETY:
+            // - Per type invariant, `cpu_addr` is valid and aligned kernel accessible memory.
+            // - Plain accesses to the region are only possible through `unsafe` APIs, that is
+            //   `Coherent::{as_ref,as_mut}`, `CoherentView::{as_ref,as_mut}` and dereferencing
+            //   the raw pointers returned by `Coherent::{as_ptr,as_mut_ptr}`, all of which require
+            //   the caller to avoid racing with other accesses to the region, including accesses
+            //   through this view.
             cpu_addr: unsafe { SysMem::new(self.cpu_addr.as_ptr()) },
             dma_addr: self.dma_addr,
         }
diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs
index 24cb5343e0db..e55c3d16729d 100644
--- a/rust/kernel/drm/gem/shmem.rs
+++ b/rust/kernel/drm/gem/shmem.rs
@@ -471,6 +471,8 @@ fn as_view(self) -> SysMem<'a, Region<SIZE>> {
         // - `addr` is page-aligned, which satisfies `Region`'s 4-byte alignment requirement.
         // - The memory remains valid until this `VMap` is dropped; since `self` is `&'a VMap`,
         //   the borrow prevents the `VMap` from being dropped for the lifetime `'a`.
+        // - `VMap` does not expose the mapping in any other way, so all accesses to the region
+        //   from Rust go through views created here, and those are never plain accesses.
         unsafe { SysMem::new(ptr) }
     }
 }
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index a877df11a03e..00bab24d613c 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -1452,7 +1452,8 @@ unsafe fn project_view<'a, T: ?Sized + KnownSize, U: ?Sized + KnownSize>(
         ptr: *mut U,
     ) -> Self::View<'a, U> {
         // INVARIANT: Per safety requirement, `ptr` is projection from `view`, so it is also a valid
-        // kernel accessible memory region.
+        // kernel accessible memory region. Accesses through the new view are accesses through
+        // `view`, which no plain access races with per the invariants of `view`.
         SysMem {
             ptr,
             phantom: PhantomData,
@@ -1470,8 +1471,9 @@ fn io_read(view: SysMem<'_, $ty>) -> $ty {
                 // SAFETY:
                 // - Per type invariant, `ptr` is valid and aligned.
                 // - Using read_volatile() here so that race with hardware is well-defined.
-                // - Using read_volatile() here is not sound if it races with other CPU per Rust
-                //   rules, but this is allowed per LKMM.
+                // - Per type invariant, no plain access races with this read. Racing with atomic
+                //   or volatile accesses from other CPUs is not sound per Rust rules, but this is
+                //   allowed per LKMM.
                 // - The macro is only used on primitives so all bit patterns are valid.
                 unsafe { view.ptr.read_volatile() }
             }
@@ -1481,8 +1483,9 @@ fn io_write(view: SysMem<'_, $ty>, value: $ty) {
                 // SAFETY:
                 // - Per type invariant, `ptr` is valid and aligned.
                 // - Using write_volatile() here so that race with hardware is well-defined.
-                // - Using write_volatile() here is not sound if it races with other CPU per Rust
-                //   rules, but this is allowed per LKMM.
+                // - Per type invariant, no plain access races with this write. Racing with atomic
+                //   or volatile accesses from other CPUs is not sound per Rust rules, but this is
+                //   allowed per LKMM.
                 unsafe { view.ptr.write_volatile(value) }
             }
         }
@@ -1519,8 +1522,9 @@ fn copy_read<T: FromBytes>(view: Self::View<'_, T>) -> T {
         // SAFETY:
         // - Per type invariant, `ptr` is valid and aligned.
         // - Using read_volatile() here so that race with hardware is well-defined.
-        // - Using read_volatile() here is not sound if it races with other CPU per Rust
-        //   rules, but this is allowed per LKMM.
+        // - Per type invariant, no plain access races with this read. Racing with atomic or
+        //   volatile accesses from other CPUs is not sound per Rust rules, but this is allowed
+        //   per LKMM.
         // - `T: FromBytes` so all bit patterns are valid.
         unsafe { view.ptr.read_volatile() }
     }
@@ -1530,22 +1534,32 @@ fn copy_write<T: IntoBytes>(view: Self::View<'_, T>, value: T) {
         // SAFETY:
         // - Per type invariant, `ptr` is valid and aligned.
         // - Using write_volatile() here so that race with hardware is well-defined.
-        // - Using write_volatile() here is not sound if it races with other CPU per Rust
-        //   rules, but this is allowed per LKMM.
+        // - Per type invariant, no plain access races with this write. Racing with atomic or
+        //   volatile accesses from other CPUs is not sound per Rust rules, but this is allowed
+        //   per LKMM.
         unsafe { view.ptr.write_volatile(value) }
     }
 }
 
 /// A view of a system memory region.
 ///
-/// Provides `Io` trait implementation for kernel virtual address ranges,
-/// using volatile read/write to safely access shared memory that may be
-/// concurrently accessed by external hardware.
+/// Provides `Io` trait implementation for kernel virtual address ranges, using volatile or
+/// byte-wise atomic accesses to safely access shared memory that may be concurrently accessed by
+/// external actors, such as hardware or user space threads, or by other kernel code using atomic or
+/// volatile accesses.
+///
+/// Accesses through a `SysMem` are never plain accesses. A plain access is a non-atomic,
+/// non-volatile access performed by kernel code, whether in C or in Rust. Two accesses to the same
+/// memory race if at least one of them is a write and they are not ordered by synchronization. A
+/// plain access that races with another access is a data race, and therefore undefined behavior.
+/// Accesses performed by hardware or by user space are not plain accesses in this sense. Racing
+/// with them is well-defined, but the data observed may be arbitrary.
 ///
 /// # Invariants
 ///
-/// `self.ptr.addr() .. self.ptr.addr() + KnownSize::size(self.ptr)` is valid and aligned kernel
-/// accessible memory region for the lifetime `'a`.
+/// - `self.ptr.addr() .. self.ptr.addr() + KnownSize::size(self.ptr)` is valid and aligned kernel
+///   accessible memory region for the lifetime `'a`.
+/// - No plain access to that memory region races with an access performed through this view.
 pub struct SysMem<'a, T: ?Sized> {
     ptr: *mut T,
     phantom: PhantomData<&'a ()>,
@@ -1570,8 +1584,10 @@ impl<'a, T: ?Sized> SysMem<'a, T> {
     ///
     /// # Safety
     ///
-    /// `ptr.addr() .. ptr.addr() + KnownSize::size(ptr)` must be valid and aligned kernel
-    /// accessible memory region for the lifetime `'a`.
+    /// - `ptr.addr() .. ptr.addr() + KnownSize::size(ptr)` must be valid and aligned kernel
+    ///   accessible memory region for the lifetime `'a`.
+    /// - No plain access (see [`SysMem`]) to that memory region may race with an access performed
+    ///   through the returned view.
     #[inline]
     pub unsafe fn new(ptr: *mut T) -> Self {
         // INVARIANT: Per safety requirement.

-- 
2.54.0