On Mon, Mar 11, 2024 at 10:47:13AM +0000, Alice Ryhl wrote: > From: Wedson Almeida Filho <wedsonaf@xxxxxxxxx> > [...] > + > +/// A reader for [`UserSlice`]. > +/// > +/// Used to incrementally read from the user slice. > +pub struct UserSliceReader { > + ptr: *mut c_void, > + length: usize, > +} > + > +impl UserSliceReader { [...] > + > + /// Reads raw data from the user slice into a raw kernel buffer. > + /// > + /// Fails with `EFAULT` if the read encounters a page fault. > + /// > + /// # Safety > + /// > + /// The `out` pointer must be valid for writing `len` bytes. > + pub unsafe fn read_raw(&mut self, out: *mut u8, len: usize) -> Result { I don't think we want to promote the pub usage of this unsafe function, right? We can provide a safe version: pub fn read_slice(&mut self, to: &[u8]) -> Result and all users can just use the safe version (with the help of slice::from_raw_parts_mut() if necessary). > + if len > self.length { > + return Err(EFAULT); > + } > + let Ok(len_ulong) = c_ulong::try_from(len) else { > + return Err(EFAULT); > + }; > + // SAFETY: The caller promises that `out` is valid for writing `len` bytes. > + let res = unsafe { bindings::copy_from_user(out.cast::<c_void>(), self.ptr, len_ulong) }; > + if res != 0 { > + return Err(EFAULT); > + } > + // Userspace pointers are not directly dereferencable by the kernel, so > + // we cannot use `add`, which has C-style rules for defined behavior. > + self.ptr = self.ptr.wrapping_byte_add(len); > + self.length -= len; > + Ok(()) > + } > + > + /// Reads the entirety of the user slice, appending it to the end of the > + /// provided buffer. > + /// > + /// Fails with `EFAULT` if the read encounters a page fault. > + pub fn read_all(mut self, buf: &mut Vec<u8>) -> Result { > + let len = self.length; > + buf.try_reserve(len)?; > + > + // SAFETY: The call to `try_reserve` was successful, so the spare > + // capacity is at least `len` bytes long. > + unsafe { self.read_raw(buf.spare_capacity_mut().as_mut_ptr().cast(), len)? }; > + > + // SAFETY: Since the call to `read_raw` was successful, so the next > + // `len` bytes of the vector have been initialized. > + unsafe { buf.set_len(buf.len() + len) }; > + Ok(()) > + } > +} > + > +/// A writer for [`UserSlice`]. > +/// > +/// Used to incrementally write into the user slice. > +pub struct UserSliceWriter { > + ptr: *mut c_void, > + length: usize, > +} > + > +impl UserSliceWriter { > + /// Returns the amount of space remaining in this buffer. > + /// > + /// Note that even writing less than this number of bytes may fail. > + pub fn len(&self) -> usize { > + self.length > + } > + > + /// Returns `true` if no more data can be written to this buffer. > + pub fn is_empty(&self) -> bool { > + self.length == 0 > + } > + > + /// Writes raw data to this user pointer from a raw kernel buffer. > + /// > + /// Fails with `EFAULT` if the write encounters a page fault. > + /// > + /// # Safety > + /// > + /// The `data` pointer must be valid for reading `len` bytes. > + pub unsafe fn write_raw(&mut self, data: *const u8, len: usize) -> Result { Same here, just remove the `pub`, and users should use write_slice() (with the help of slice::from_raw_parts() if necessary). Regards, Boqun > + if len > self.length { > + return Err(EFAULT); > + } > + let Ok(len_ulong) = c_ulong::try_from(len) else { > + return Err(EFAULT); > + }; > + let res = unsafe { bindings::copy_to_user(self.ptr, data.cast::<c_void>(), len_ulong) }; > + if res != 0 { > + return Err(EFAULT); > + } > + // Userspace pointers are not directly dereferencable by the kernel, so > + // we cannot use `add`, which has C-style rules for defined behavior. > + self.ptr = self.ptr.wrapping_byte_add(len); > + self.length -= len; > + Ok(()) > + } > + > + /// Writes the provided slice to this user pointer. > + /// > + /// Fails with `EFAULT` if the write encounters a page fault. > + pub fn write_slice(&mut self, data: &[u8]) -> Result { > + let len = data.len(); > + let ptr = data.as_ptr(); > + // SAFETY: The pointer originates from a reference to a slice of length > + // `len`, so the pointer is valid for reading `len` bytes. > + unsafe { self.write_raw(ptr, len) } > + } > +} > > -- > 2.44.0.278.ge034bb2e1d-goog >