linux-kernel - Re: [PATCH v2 3/8] rust: uaccess: add UserSliceWriter::write_slice

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DDO2PI0D2L6Q.3OPXNQOV7Y0H6@kernel.org>
Date: Tue, 21 Oct 2025 16:34:49 +0200
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Alice Ryhl" <aliceryhl@...gle.com>
Cc: <gregkh@...uxfoundation.org>, <rafael@...nel.org>, <ojeda@...nel.org>,
 <alex.gaynor@...il.com>, <boqun.feng@...il.com>, <gary@...yguo.net>,
 <bjorn3_gh@...tonmail.com>, <lossin@...nel.org>, <a.hindborg@...nel.org>,
 <tmgross@...ch.edu>, <mmaurer@...gle.com>,
 <rust-for-linux@...r.kernel.org>, <linux-fsdevel@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 3/8] rust: uaccess: add
 UserSliceWriter::write_slice_partial()

On Tue Oct 21, 2025 at 4:18 PM CEST, Alice Ryhl wrote:
> On Tue, Oct 21, 2025 at 04:14:22PM +0200, Danilo Krummrich wrote:
>> On Tue Oct 21, 2025 at 4:00 PM CEST, Alice Ryhl wrote:
>> > On Tue, Oct 21, 2025 at 12:26:15AM +0200, Danilo Krummrich wrote:
>> >> The existing write_slice() method is a wrapper around copy_to_user() and
>> >> expects the user buffer to be larger than the source buffer.
>> >> 
>> >> However, userspace may split up reads in multiple partial operations
>> >> providing an offset into the source buffer and a smaller user buffer.
>> >> 
>> >> In order to support this common case, provide a helper for partial
>> >> writes.
>> >> 
>> >> Signed-off-by: Danilo Krummrich <dakr@...nel.org>
>> >>  rust/kernel/uaccess.rs | 24 ++++++++++++++++++++++++
>> >>  1 file changed, 24 insertions(+)
>> >> 
>> >> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
>> >> index 2061a7e10c65..40d47e94b54f 100644
>> >> --- a/rust/kernel/uaccess.rs
>> >> +++ b/rust/kernel/uaccess.rs
>> >> @@ -463,6 +463,30 @@ pub fn write_slice(&mut self, data: &[u8]) -> Result {
>> >>          Ok(())
>> >>      }
>> >>  
>> >> +    /// Writes raw data to this user pointer from a kernel buffer partially.
>> >> +    ///
>> >> +    /// This is the same as [`Self::write_slice`] but considers the given `offset` into `data` and
>> >> +    /// truncates the write to the boundaries of `self` and `data`.
>> >> +    ///
>> >> +    /// On success, returns the number of bytes written.
>> >> +    pub fn write_slice_partial(&mut self, data: &[u8], offset: file::Offset) -> Result<usize> {
>> >
>> > I think for the current function signature, it's kind of weird to take a
>> > file::Offset parameter
>> >
>> > On one hand, it is described like a generic function for writing a
>> > partial slice, and if that's what it is, then I would argue it should
>> > take usize because it's an offset into the slice.
>> >
>> > On another hand, I think what you're actually trying to do is implement
>> > the simple_[read_from|write_to]_buffer utilities for user slices, but
>> > it's only a "partial" version of those utilities. The full utility takes
>> > a `&mut loff_t` so that it can also perform the required modification to
>> > the offset.
>> 
>> Originally, it was intended to be the latter. And, in fact, earlier code (that
>> did not git the mailing list) had a &mut file::Offset argument (was &mut i64
>> back then).
>> 
>> However, for the version I sent to the list I chose the former because I
>> considered it to be more flexible.
>> 
>> Now, in v2, it's indeed a bit mixed up. I think what we should do is to have
>> both
>> 
>> 	fn write_slice_partial(&mut self, data: &[u8], offset: usize) -> Result<usize>
>> 
>> and
>> 
>> 	fn write_slice_???(&mut self, data: &[u8], offset: &mut file::Offset) -> Result<usize>
>> 
>> which can forward to write_slice_partial() and update the buffer.
>
> SGTM.
>
>> Any name suggestions?
>
> I would suggest keeping the name of the equivalent C method:
> simple_read_from_buffer/simple_write_to_buffer

Hm..that's an option, but UserSliceWriter corresponds to
simple_read_from_buffer() and UserSliceReader corresponds to
simple_write_to_buffer().

I think having UserSliceWriter::simple_read_from_buffer() while we have
UserSliceWriter::write_slice() is confusing. But swapping the semantics of
simple_read_from_buffer() and simple_write_to_buffer() is even more confusing.

So, I think using the existing names is not a great fit.

Maybe something like write_file_slice() or write_slice_file()? The former could
be read as "slice of files" which would be misleading though.