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);
+
+ Ok(written)
+ }
+
/// Writes the provided Rust value to this userspace pointer.
///
/// Fails with [`EFAULT`] if the write happens on a bad address, or if the write goes out of
--
2.51.0On Wed, Oct 22, 2025 at 4:32 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> + /// Writes raw data to this user pointer from a kernel buffer partially.
We use "user pointer" in a few places (including another of the added
ones), while "user slice" in others -- shouldn't we just use "user
slice"?
By the way, for new APIs with several arguments and potential paths
due to the `offset`, it wouldn't hurt to have an example (even if
build-only) with a couple asserts to show how the API behaves. But I
can create a good first issue for that if you prefer.
Similarly (another potential good first issue), should we use e.g.
#[doc(alias = "simple_read_from_buffer")]`
?
Thanks!
Cheers,
Miguel
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:
offset + written <= data.len() <= isize::MAX <= Offset::MAX
I can't help but think that maybe this should be a + operation instead?
Alice
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.
Even though I don't expect any of the above to change any time soon
saturating_add_usize() seems reasonable to me.
> I can't help but think that maybe this should be a + operation instead?
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
Alice
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.
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
On Thu, Oct 23, 2025 at 1:20 PM Alice Ryhl <aliceryhl@google.com> wrote: > > 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. I don't have a link at hand now, but we already (recently too, I think) discussed having these "we know it is infallible in the kernel" conversions. Maybe there was a patch posted even. Otherwise, I will create a good first issue. Cheers, Miguel
On Sat Oct 25, 2025 at 3:02 AM JST, Miguel Ojeda wrote: > On Thu, Oct 23, 2025 at 1:20 PM Alice Ryhl <aliceryhl@google.com> wrote: >> >> 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. > > I don't have a link at hand now, but we already (recently too, I > think) discussed having these "we know it is infallible in the kernel" > conversions. Maybe there was a patch posted even. > > Otherwise, I will create a good first issue. Are you referring to this discussion? https://lore.kernel.org/rust-for-linux/DDK4KADWJHMG.1FUPL3SDR26XF@kernel.org/ If so, I have posted something along those lines: https://lore.kernel.org/rust-for-linux/20251029-nova-as-v3-4-6a30c7333ad9@nvidia.com/ We planned to have it stew in Nova for a bit, but I don't mind moving this to the core code if you think that looks good enough.
On Sat, Nov 1, 2025 at 3:27 PM Alexandre Courbot <acourbot@nvidia.com> wrote: > > Are you referring to this discussion? > > https://lore.kernel.org/rust-for-linux/DDK4KADWJHMG.1FUPL3SDR26XF@kernel.org/ I saw that one and the patches -- perhaps it was in meetings, but dealing with guarantees that are only true in the kernel (assumptions, conversions) has come up before several times over the years. Cheers, Miguel
On Thu Oct 23, 2025 at 1:20 PM CEST, Alice Ryhl wrote:
> 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.
We can (and probably should) implement a kernel specific infallible one.
I think we also want a helper for `slice::len() as isize`.
> 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.
Sure, it doesn't change the code required for this case. Yet, I think that if we
agree on having a kernel specific infallible conversions for usize -> u64 and
isize -> i64, it makes this + operation formally more consistent.
Here's the diff I'd apply:
diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs
index 681b8a9e5d52..63478dd7deb8 100644
--- a/rust/kernel/fs/file.rs
+++ b/rust/kernel/fs/file.rs
@@ -125,6 +125,22 @@ pub fn saturating_sub_usize(self, rhs: usize) -> Offset {
}
}
+impl core::ops::Add<isize> for Offset {
+ type Output = Offset;
+
+ #[inline]
+ fn add(self, rhs: isize) -> Offset {
+ Offset(self.0 + rhs as bindings::loff_t)
+ }
+}
+
+impl core::ops::AddAssign<isize> for Offset {
+ #[inline]
+ fn add_assign(&mut self, rhs: isize) {
+ self.0 += rhs as bindings::loff_t;
+ }
+}
+
impl From<bindings::loff_t> for Offset {
#[inline]
fn from(v: bindings::loff_t) -> Self {
diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
index 20ea31781efb..44ee334c4507 100644
--- a/rust/kernel/uaccess.rs
+++ b/rust/kernel/uaccess.rs
@@ -514,7 +514,8 @@ pub fn write_slice_file(&mut self, data: &[u8], offset: &mut file::Offset) -> Re
let written = self.write_slice_partial(data, offset_index)?;
- *offset = offset.saturating_add_usize(written);
+ // OVERFLOW: `offset + written <= data.len() <= isize::MAX <= Offset::MAX`
+ *offset += written as isize;
Ok(written)
}
On Thu, Oct 23, 2025 at 02:43:20PM +0200, Danilo Krummrich wrote:
> On Thu Oct 23, 2025 at 1:20 PM CEST, Alice Ryhl wrote:
> > 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.
>
> We can (and probably should) implement a kernel specific infallible one.
>
> I think we also want a helper for `slice::len() as isize`.
>
> > 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.
>
> Sure, it doesn't change the code required for this case. Yet, I think that if we
> agree on having a kernel specific infallible conversions for usize -> u64 and
> isize -> i64, it makes this + operation formally more consistent.
>
> Here's the diff I'd apply:
>
> diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs
> index 681b8a9e5d52..63478dd7deb8 100644
> --- a/rust/kernel/fs/file.rs
> +++ b/rust/kernel/fs/file.rs
> @@ -125,6 +125,22 @@ pub fn saturating_sub_usize(self, rhs: usize) -> Offset {
> }
> }
>
> +impl core::ops::Add<isize> for Offset {
> + type Output = Offset;
> +
> + #[inline]
> + fn add(self, rhs: isize) -> Offset {
> + Offset(self.0 + rhs as bindings::loff_t)
> + }
> +}
> +
> +impl core::ops::AddAssign<isize> for Offset {
> + #[inline]
> + fn add_assign(&mut self, rhs: isize) {
> + self.0 += rhs as bindings::loff_t;
> + }
> +}
> +
> impl From<bindings::loff_t> for Offset {
> #[inline]
> fn from(v: bindings::loff_t) -> Self {
> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
> index 20ea31781efb..44ee334c4507 100644
> --- a/rust/kernel/uaccess.rs
> +++ b/rust/kernel/uaccess.rs
> @@ -514,7 +514,8 @@ pub fn write_slice_file(&mut self, data: &[u8], offset: &mut file::Offset) -> Re
>
> let written = self.write_slice_partial(data, offset_index)?;
>
> - *offset = offset.saturating_add_usize(written);
> + // OVERFLOW: `offset + written <= data.len() <= isize::MAX <= Offset::MAX`
> + *offset += written as isize;
>
> Ok(written)
> }
>
This LGTM.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
© 2016 - 2026 Red Hat, Inc.