linux-kernel - Re: [PATCH] uaccess: rust: add strncpy_from

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: <680a5f5d.050a0220.2035d2.545b@mx.google.com>
Date: Thu, 24 Apr 2025 08:57:13 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Miguel Ojeda <ojeda@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <benno.lossin@...ton.me>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Trevor Gross <tmgross@...ch.edu>,
	Danilo Krummrich <dakr@...nel.org>, rust-for-linux@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] uaccess: rust: add strncpy_from_user

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@...gle.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@...gle.com>
>