Re: [PATCH v8 2/7] rust: uaccess: add write_dma() for copying from DMA buffers to userspace

["Alexandre Courbot" <acourbot@nvidia.com>]

On Wed Mar 11, 2026 at 6:59 AM JST, Timur Tabi wrote:
> Add UserSliceWriter::write_dma() to copy data from a CoherentAllocation<u8>
> to userspace. This provides a safe interface for copying DMA buffer
> contents to userspace without requiring callers to work with raw pointers.
>
> Because write_dma() and write_slice() have common code, factor that code
> out into a helper function, write_raw().
>
> The method handles bounds checking and offset calculation internally,
> wrapping the unsafe copy_to_user() call.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  rust/kernel/uaccess.rs | 84 +++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 74 insertions(+), 10 deletions(-)
>
> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
> index f989539a31b4..3f569acc3718 100644
> --- a/rust/kernel/uaccess.rs
> +++ b/rust/kernel/uaccess.rs
> @@ -7,6 +7,7 @@
>  use crate::{
>      alloc::{Allocator, Flags},
>      bindings,
> +    dma::CoherentAllocation,
>      error::Result,
>      ffi::{c_char, c_void},
>      fs::file,
> @@ -459,20 +460,25 @@ pub fn is_empty(&self) -> bool {
>          self.length == 0
>      }
>  
> -    /// Writes raw data to this user pointer from a kernel buffer.
> +    /// Low-level write from a raw pointer.
>      ///
> -    /// Fails with [`EFAULT`] if the write happens on a bad address, or if the write goes out of
> -    /// bounds of this [`UserSliceWriter`]. This call may modify the associated userspace slice even
> -    /// if it returns an error.
> -    pub fn write_slice(&mut self, data: &[u8]) -> Result {
> -        let len = data.len();
> -        let data_ptr = data.as_ptr().cast::<c_void>();
> +    /// # Safety
> +    ///
> +    /// The caller must ensure that `ptr` points to a valid slice of `len` bytes (i.e., it is
> +    /// valid for reads of `len` bytes and is properly aligned).

Bytes arrays are supposed to be byte-aligned, so I am not sure the
"properly aligned" adds something (it's also not technically incorrect
so fine to keep it).

> +    unsafe fn write_raw(&mut self, ptr: *const u8, len: usize) -> Result {
>          if len > self.length {
>              return Err(EFAULT);
>          }
> -        // SAFETY: `data_ptr` points into an immutable slice of length `len`, so we may read
> -        // that many bytes from it.
> -        let res = unsafe { bindings::copy_to_user(self.ptr.as_mut_ptr(), data_ptr, len) };
> +        // SAFETY:
> +        // - `self.ptr` is a userspace pointer, and `len <= self.length` is checked above to
> +        //   ensure we don't exceed the caller-specified bounds.
> +        // - `ptr` is valid for reading `len` bytes as required by this function's safety contract.
> +        // - `copy_to_user` validates the userspace address at runtime and returns non-zero on
> +        //   failure (e.g., bad address or unmapped memory).
> +        let res = unsafe {
> +            bindings::copy_to_user(self.ptr.as_mut_ptr(), ptr.cast::<c_void>(), len)
> +        };
>          if res != 0 {
>              return Err(EFAULT);
>          }
> @@ -481,6 +487,64 @@ pub fn write_slice(&mut self, data: &[u8]) -> Result {
>          Ok(())
>      }
>  
> +    /// Writes raw data to this user pointer from a kernel buffer.
> +    ///
> +    /// Fails with [`EFAULT`] if the write happens on a bad address, or if the write goes out of
> +    /// bounds of this [`UserSliceWriter`]. This call may modify the associated userspace slice even
> +    /// if it returns an error.
> +    pub fn write_slice(&mut self, data: &[u8]) -> Result {
> +        // SAFETY: `data` is a valid slice, so `data.as_ptr()` is valid for
> +        // reading `data.len()` bytes.
> +        unsafe { self.write_raw(data.as_ptr(), data.len()) }
> +    }
> +
> +    /// Writes raw data to this user pointer from a DMA coherent allocation.
> +    ///
> +    /// # Arguments
> +    ///
> +    /// * `data` - The DMA coherent allocation to copy from.
> +    /// * `offset` - The byte offset into `data` to start copying from.
> +    /// * `count` - The number of bytes to copy.
> +    ///
> +    /// # Errors

Nit: missing empty line.

Other thank that (and the test robot warnings),

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>