On 1/10/24 18:42, Kevin Brodsky wrote:
On 10/01/2024 10:45, Amit Daniel Kachhap wrote:
diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c index 60942cdfd3d2..8f12259d8431 100644 --- a/lib/strnlen_user.c +++ b/lib/strnlen_user.c @@ -18,6 +18,9 @@ static __always_inline long find_zero_aligned(const unsigned long __user *src, long res = 0; unsigned long c; + if (max == 0) + return 0;
unsafe_get_user(c, src++, efault); c |= aligned_byte_mask(align); @@ -42,6 +45,27 @@ static __always_inline long find_zero_aligned(const unsigned long __user *src, return -EFAULT;
I suppose instead of returning negative(-EFAULT) error, returning 0 might be sufficient here. This will make the code slightly simpler as the error value is again later used for not so useful calculation.
There is a difference between what these helpers return and what do_strnlen_user() returns: the former return the *index* of '\0' if found (as per the comment above), while the latter returns the *length* of the string including '\0', in other words index + 1. The helpers return 0 if '\0' is found at index 0, and -EFAULT if uaccess fails.
I initially had the helpers return the length (index + 1), but I realised that it didn't work so well as this value cannot be added directly (do_strnlen_user() may call find_zero_unaligned() first, then add the return value of find_zero_aligned(), then find_zero_unaligned() again). It is possible to subtract 1 every time, but I figured it was clearer to separate out the error case and just add indices.
Hopefully that makes sense!
ok. If returning 0 will not work out for error handling then your approach is fine.
Thanks, Amit
Kevin