Re: [PATCH v3 05/10] rust: uaccess: add UserSliceWriter::write_slice_file()

[Alice Ryhl <aliceryhl@google.com>]

On Thu, Oct 23, 2025 at 01:03:35PM +0200, Danilo Krummrich wrote:
> On Thu Oct 23, 2025 at 12:37 PM CEST, Alice Ryhl wrote:
> > On Thu, Oct 23, 2025 at 12:35 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >>
> >> On Thu Oct 23, 2025 at 10:30 AM CEST, Alice Ryhl wrote:
> >> > On Wed, Oct 22, 2025 at 04:30:39PM +0200, Danilo Krummrich wrote:
> >> >> Add UserSliceWriter::write_slice_file(), which is the same as
> >> >> UserSliceWriter::write_slice_partial() but updates the given
> >> >> file::Offset by the number of bytes written.
> >> >>
> >> >> This is equivalent to C's `simple_read_from_buffer()` and useful when
> >> >> dealing with file offsets from file operations.
> >> >>
> >> >> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> >> >> ---
> >> >>  rust/kernel/uaccess.rs | 24 ++++++++++++++++++++++++
> >> >>  1 file changed, 24 insertions(+)
> >> >>
> >> >> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
> >> >> index 539e77a09cbc..20ea31781efb 100644
> >> >> --- a/rust/kernel/uaccess.rs
> >> >> +++ b/rust/kernel/uaccess.rs
> >> >> @@ -495,6 +495,30 @@ pub fn write_slice_partial(&mut self, data: &[u8], offset: usize) -> Result<usiz
> >> >>              .map_or(Ok(0), |src| self.write_slice(src).map(|()| src.len()))
> >> >>      }
> >> >>
> >> >> +    /// Writes raw data to this user pointer from a kernel buffer partially.
> >> >> +    ///
> >> >> +    /// This is the same as [`Self::write_slice_partial`] but updates the given [`file::Offset`] by
> >> >> +    /// the number of bytes written.
> >> >> +    ///
> >> >> +    /// This is equivalent to C's `simple_read_from_buffer()`.
> >> >> +    ///
> >> >> +    /// On success, returns the number of bytes written.
> >> >> +    pub fn write_slice_file(&mut self, data: &[u8], offset: &mut file::Offset) -> Result<usize> {
> >> >> +        if offset.is_negative() {
> >> >> +            return Err(EINVAL);
> >> >> +        }
> >> >> +
> >> >> +        let Ok(offset_index) = (*offset).try_into() else {
> >> >> +            return Ok(0);
> >> >> +        };
> >> >> +
> >> >> +        let written = self.write_slice_partial(data, offset_index)?;
> >> >> +
> >> >> +        *offset = offset.saturating_add_usize(written);
> >> >
> >> > This addition should never overflow:
> >>
> >> It probably never will (which is why this was a + operation in v1).
> >>
> >> >       offset + written <= data.len() <= isize::MAX <= Offset::MAX
> >>
> >> However, this would rely on implementation details you listed, i.e. the
> >> invariant that a slice length should be at most isize::MAX and what's the
> >> maximum size of file::Offset::MAX.
> >
> > It's not an implementation detail. All Rust allocations are guaranteed
> > to fit in isize::MAX bytes:
> > https://doc.rust-lang.org/stable/std/ptr/index.html#allocation
> 
> Yeah, I'm aware -- I expressed this badly.
> 
> What I meant is that for the kernel we obviously know that there's no
> architecture where isize::MAX > file::Offset::MAX.
> 
> However, in the core API the conversion from usize to u128 is considered
> fallible. So, applying the assumption that isize::MAX <= file::Offset::MAX is at
> least inconsistent.

I would love to have infallible conversions from usize to u64 (and u32
to usize), but we can't really modify the stdlib to add them.

But even if we had them, it wouldn't help here since the target type is
i64, not u64. And there are usize values that don't fit in i64 - it's
just that in this case the usize fits in isize.

Alice