Re: [PATCH v3 1/2] uaccess: rust: add strncpy_from_user

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Mon, May 05, 2025 at 12:17:31PM +0000, Alice Ryhl wrote:
> This patch adds a direct wrapper around the C function of the same name.
> It's not really intended for direct use by Rust code since
> strncpy_from_user has a somewhat unfortunate API where it only
> nul-terminates the buffer if there's space for the nul-terminator. This
> means that a direct Rust wrapper around it could not return a &CStr
> since the buffer may not be a cstring. However, we still add the method
> to build more convenient APIs on top of it, which will happen in
> subsequent patches.
> 
> Reviewed-by: Danilo Krummrich <dakr@kernel.org>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/uaccess.rs | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
> index 80a9782b1c6e98ed6eae308ade8551afa7adc188..a7b123915e9aa2329f376d67cad93e2fc17ae017 100644
> --- a/rust/kernel/uaccess.rs
> +++ b/rust/kernel/uaccess.rs
> @@ -8,7 +8,7 @@
>      alloc::{Allocator, Flags},
>      bindings,
>      error::Result,
> -    ffi::c_void,
> +    ffi::{c_char, c_void},
>      prelude::*,
>      transmute::{AsBytes, FromBytes},
>  };
> @@ -369,3 +369,36 @@ pub fn write<T: AsBytes>(&mut self, value: &T) -> Result {
>          Ok(())
>      }
>  }
> +
> +/// Reads a nul-terminated string into `buf` and returns the length.
> +///
> +/// This reads from userspace until a NUL byte is encountered, or until `buf.len()` bytes have been
> +/// read. Fails with [`EFAULT`] if a read happens on a bad address (some data may have been
> +/// copied). When the end of the buffer is encountered, no NUL byte is added, so the string is
> +/// *not* guaranteed to be NUL-terminated when `Ok(buf.len())` is returned.
> +///
> +/// # Guarantees
> +///
> +/// When this function returns `Ok(len)`, it is guaranteed that the first `len` of `buf` bytes are
> +/// initialized and non-zero. Furthermore, if `len < buf.len()`, then `buf[len]` is a NUL byte.
> +/// Unsafe code may rely on these guarantees.
> +#[inline]
> +#[expect(dead_code)]
> +fn raw_strncpy_from_user(ptr: UserPtr, buf: &mut [MaybeUninit<u8>]) -> Result<usize> {

Nit, the parameters here are backwards from the C version of
strncpy_from_user(), which is going to cause us no end of grief when
reviewing code between the two languages :(

Also, it's not your fault, but we don't have any type of __user tag for
data coming from userspace yet to track this type of thing?  The
compiler (well sparse) can catch this type of thing in C, any hints on
what we could do in Rust for the same type of guarantee (i.e. don't
touch user data before it's been copied, and then we need to treat it as
"unverified" but that's a different patch series...)

thanks,

greg k-h