[PATCH] rust: uaccess: avoid unsafe unwrap_unchecked() in strcpy_into_buf()

[PATCH] rust: uaccess: avoid unsafe unwrap_unchecked() in strcpy_into_buf()

[Thorsten Blum]

Since strcpy_into_buf() already rejects empty buffers, use ok_or()
instead of unwrap_unchecked() when NUL-terminating the buffer.

Signed-off-by: Thorsten Blum <blum@kernel.org>
---
 rust/kernel/uaccess.rs | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
index 5f6c4d7a1a51..2a0af795e75d 100644
--- a/rust/kernel/uaccess.rs
+++ b/rust/kernel/uaccess.rs
@@ -422,9 +422,7 @@ pub fn strcpy_into_buf<'buf>(self, buf: &'buf mut [u8]) -> Result<&'buf CStr> {
             // This means that we filled the buffer exactly. In this case, we add a NUL-terminator
             // and return it. Unlike the `len < dst.len()` branch, don't modify `len` because it
             // already represents the length including the NUL-terminator.
-            //
-            // SAFETY: Due to the check at the beginning, the buffer is not empty.
-            unsafe { *buf.last_mut().unwrap_unchecked() = 0 };
+            *buf.last_mut().ok_or(EINVAL)? = 0;
         }
 
         // This method consumes `self`, so it can only be called once, thus we do not need to

Re: [PATCH] rust: uaccess: avoid unsafe unwrap_unchecked() in strcpy_into_buf()

[Alexandre Courbot]

On Thu Oct 8, 2026 at 3:13 PM JST, Thorsten Blum wrote:
> Since strcpy_into_buf() already rejects empty buffers, use ok_or()
> instead of unwrap_unchecked() when NUL-terminating the buffer.
>
> Signed-off-by: Thorsten Blum <blum@kernel.org>
> ---
>  rust/kernel/uaccess.rs | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
> index 5f6c4d7a1a51..2a0af795e75d 100644
> --- a/rust/kernel/uaccess.rs
> +++ b/rust/kernel/uaccess.rs
> @@ -422,9 +422,7 @@ pub fn strcpy_into_buf<'buf>(self, buf: &'buf mut [u8]) -> Result<&'buf CStr> {
>              // This means that we filled the buffer exactly. In this case, we add a NUL-terminator
>              // and return it. Unlike the `len < dst.len()` branch, don't modify `len` because it
>              // already represents the length including the NUL-terminator.
> -            //
> -            // SAFETY: Due to the check at the beginning, the buffer is not empty.
> -            unsafe { *buf.last_mut().unwrap_unchecked() = 0 };
> +            *buf.last_mut().ok_or(EINVAL)? = 0;

I am not sure this gives us much - we are trading an unsafe statement
that is well-controlled (enforced by the first two lines of the method)
for a runtime check. I'd say this is working as intended here.

Re: [PATCH] rust: uaccess: avoid unsafe unwrap_unchecked() in strcpy_into_buf()

[Thorsten Blum]

On Thu, Oct 08, 2026 at 10:15:49PM +0900, Alexandre Courbot wrote:
> On Thu Oct 8, 2026 at 3:13 PM JST, Thorsten Blum wrote:
> > Since strcpy_into_buf() already rejects empty buffers, use ok_or()
> > instead of unwrap_unchecked() when NUL-terminating the buffer.
> >
> > Signed-off-by: Thorsten Blum <blum@kernel.org>
> > ---
> >  rust/kernel/uaccess.rs | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
> > index 5f6c4d7a1a51..2a0af795e75d 100644
> > --- a/rust/kernel/uaccess.rs
> > +++ b/rust/kernel/uaccess.rs
> > @@ -422,9 +422,7 @@ pub fn strcpy_into_buf<'buf>(self, buf: &'buf mut [u8]) -> Result<&'buf CStr> {
> >              // This means that we filled the buffer exactly. In this case, we add a NUL-terminator
> >              // and return it. Unlike the `len < dst.len()` branch, don't modify `len` because it
> >              // already represents the length including the NUL-terminator.
> > -            //
> > -            // SAFETY: Due to the check at the beginning, the buffer is not empty.
> > -            unsafe { *buf.last_mut().unwrap_unchecked() = 0 };
> > +            *buf.last_mut().ok_or(EINVAL)? = 0;
> 
> I am not sure this gives us much - we are trading an unsafe statement
> that is well-controlled (enforced by the first two lines of the method)
> for a runtime check. I'd say this is working as intended here.

I checked the generated code before and after the patch and it is
identical since the compiler is able to remove the additional check.
Therefore, this removes an unsafe block without adding runtime cost.

It also avoids relying on the buf.is_empty() check to prevent undefined
behavior.

Re: [PATCH] rust: uaccess: avoid unsafe unwrap_unchecked() in strcpy_into_buf()

[Gary Guo]

On Thu Oct 8, 2026 at 7:49 PM BST, Thorsten Blum wrote:
> On Thu, Oct 08, 2026 at 10:15:49PM +0900, Alexandre Courbot wrote:
>> On Thu Oct 8, 2026 at 3:13 PM JST, Thorsten Blum wrote:
>> > Since strcpy_into_buf() already rejects empty buffers, use ok_or()
>> > instead of unwrap_unchecked() when NUL-terminating the buffer.
>> >
>> > Signed-off-by: Thorsten Blum <blum@kernel.org>
>> > ---
>> >  rust/kernel/uaccess.rs | 4 +---
>> >  1 file changed, 1 insertion(+), 3 deletions(-)
>> >
>> > diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
>> > index 5f6c4d7a1a51..2a0af795e75d 100644
>> > --- a/rust/kernel/uaccess.rs
>> > +++ b/rust/kernel/uaccess.rs
>> > @@ -422,9 +422,7 @@ pub fn strcpy_into_buf<'buf>(self, buf: &'buf mut [u8]) -> Result<&'buf CStr> {
>> >              // This means that we filled the buffer exactly. In this case, we add a NUL-terminator
>> >              // and return it. Unlike the `len < dst.len()` branch, don't modify `len` because it
>> >              // already represents the length including the NUL-terminator.
>> > -            //
>> > -            // SAFETY: Due to the check at the beginning, the buffer is not empty.
>> > -            unsafe { *buf.last_mut().unwrap_unchecked() = 0 };
>> > +            *buf.last_mut().ok_or(EINVAL)? = 0;
>> 
>> I am not sure this gives us much - we are trading an unsafe statement
>> that is well-controlled (enforced by the first two lines of the method)
>> for a runtime check. I'd say this is working as intended here.
>
> I checked the generated code before and after the patch and it is
> identical since the compiler is able to remove the additional check.
> Therefore, this removes an unsafe block without adding runtime cost.
>
> It also avoids relying on the buf.is_empty() check to prevent undefined
> behavior.

Adding an error returning path is worse for something that cannot happen is
worse than invoking unsafe in my opinion.

Why not just unwrap?

    *buf.last_mut().unwrap() = 0;

Best,
Gary