Re: [PATCH] uaccess: rust: add strncpy_from_user

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

On Thu, Apr 24, 2025 at 03:17:48PM +0000, Alice Ryhl wrote:
> This is needed for ioctls that operate on a user-provided string.
> 
> It is somewhat unfortunate that strncpy_from_user does not nul-terminate
> the string when the end of `buf` is reached. This implies that we can't
> return a &CStr from the function, since the buffer may not always be
> nul-terminated.
> 
> That said, we could add more convenient helpers on top that add a NUL
> byte in that case.
> 
> This method isn't defined on UserSliceReader because it complicates the
> semantics. The UserSliceReader type also has its own maximum length, so
> we would have to limit the read by that length too.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/uaccess.rs | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
> index 80a9782b1c6e98ed6eae308ade8551afa7adc188..1bd82045e81ea887008e30241bd6de27f096b639 100644
> --- a/rust/kernel/uaccess.rs
> +++ b/rust/kernel/uaccess.rs
> @@ -369,3 +369,30 @@ pub fn write<T: AsBytes>(&mut self, value: &T) -> Result {
>          Ok(())
>      }
>  }
> +
> +/// Reads a nul-terminated string into `buf` and returns the length.
> +///
> +/// Fails with [`EFAULT`] if the read happens on a bad address. If the end of `buf` is reached,
> +/// then the buffer will not be nul-terminated.
> +#[inline]
> +pub fn strncpy_from_user(ptr: UserPtr, buf: &mut [u8]) -> Result<usize> {

Sorry maybe there is an email I'm missing, but could you provide more
context of the usage?

First the function name is a bit weird, because the 'n' in "strncpy"
means the parameters should have an 'n' (i.e. length) in it, but there
is none in the Rust version. Also, we don't need to replicate the
semantics of C here, we could just do a strncpy_from_user(..., ..., len
- 1), where `len` is the len of the `buf`, and then we would always have
a nul-terminated string. But maybe I'm missing something from the usage
side, so this is not doable?

Regards,
Boqun

> +    // CAST: Slice lengths are guaranteed to be `<= isize::MAX`.
> +    let len = buf.len() as isize;
> +
> +    // SAFETY: `buf` is valid for writing `buf.len()` bytes.
> +    let res = unsafe {
> +        bindings::strncpy_from_user(
> +            buf.as_mut_ptr(),
> +            ptr as *const u8,
> +            len,
> +        )
> +    };
> +
> +    if res < 0 {
> +        Err(Error::from_errno(res as i32))
> +    } else {
> +        #[cfg(CONFIG_RUST_OVERFLOW_CHECKS)]
> +        assert!(res <= len);
> +        Ok(res as usize)
> +    }
> +}
> 
> ---
> base-commit: 9c32cda43eb78f78c73aee4aa344b777714e259b
> change-id: 20250424-strncpy-from-user-1f2d06b0cdde
> 
> Best regards,
> -- 
> Alice Ryhl <aliceryhl@google.com>
>