Re: [PATCH] rust: generalize userSliceReader to support any Vec

[Boqun Feng <boqun.feng@gmail.com>]

On Mon, Dec 30, 2024 at 02:21:39PM +0100, Alice Ryhl wrote:
> On 12/22/24 6:20 PM, Boqun Feng wrote:
> > On Sun, Dec 22, 2024 at 01:37:02PM -0300, Filipe Xavier wrote:
> > > The UserSliceReader::read_all function is currently restricted to use
> > > only Vec with the kmalloc allocator.
> > > However, there is no reason for this limitation.
> > > This patch generalizes the function to accept any Vec regardless of the
> > > allocator used.
> > > 
> > 
> > Interesting idea, however...
> > 
> > > Link: https://github.com/Rust-for-Linux/linux/issues/1136
> > > Signed-off-by: Filipe Xavier <felipeaggger@gmail.com>
> > > ---
> > >   rust/kernel/uaccess.rs | 6 +++---
> > >   1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
> > > index cc044924867b89f6116c7f9ec216d8cea2b3f8d7..719b0a48ff5550acc19f2e607c0a09f818145def 100644
> > > --- a/rust/kernel/uaccess.rs
> > > +++ b/rust/kernel/uaccess.rs
> > > @@ -5,7 +5,7 @@
> > >   //! C header: [`include/linux/uaccess.h`](srctree/include/linux/uaccess.h)
> > >   use crate::{
> > > -    alloc::Flags,
> > > +    alloc::{Allocator, Flags},
> > >       bindings,
> > >       error::Result,
> > >       ffi::c_void,
> > > @@ -127,7 +127,7 @@ pub fn new(ptr: UserPtr, length: usize) -> Self {
> > >       /// Reads the entirety of the user slice, appending it to the end of the provided buffer.
> > >       ///
> > >       /// Fails with [`EFAULT`] if the read happens on a bad address.
> > > -    pub fn read_all(self, buf: &mut KVec<u8>, flags: Flags) -> Result {
> > > +    pub fn read_all<A: Allocator>(self, buf: &mut Vec<u8, A>, flags: Flags) -> Result {
> > >           self.reader().read_all(buf, flags)
> > >       }
> > > @@ -281,7 +281,7 @@ pub fn read<T: FromBytes>(&mut self) -> Result<T> {
> > >       /// Reads the entirety of the user slice, appending it to the end of the provided buffer.
> > >       ///
> > >       /// Fails with [`EFAULT`] if the read happens on a bad address.
> > > -    pub fn read_all(mut self, buf: &mut KVec<u8>, flags: Flags) -> Result {
> > > +    pub fn read_all<A: Allocator>(mut self, buf: &mut Vec<u8, A>, flags: Flags) -> Result {
> > 
> > ... what if the copy hit a page fault because the target memory (e.g. a
> > VVec) is not mapped? Would copy_from_user() handle this? Or would it
> > return a different error code than `EFAULT`?
> How could the memory not be mapped? A vector lets us obtain a &[u8] to the
> memory, which isn't valid if it's not mapped.
> 

I mistakenly thought that vmalloc()'d memory can be swapped out, which
is not the case. So you're right the memory must be mapped.

But the usage question still exists: do you have an usage case for
reading userspace memory into a `VVec`?

Regards,
Boqun

> Alice