On Thu, Apr 30, 2020 at 04:05:07PM +0200, Arnd Bergmann wrote:
> lib/strncpy_from_user.o: warning: objtool: strncpy_from_user()+0x133: call to 
> do_strncpy_from_user() with UACCESS enabled
> lib/strnlen_user.o: warning: objtool: strnlen_user()+0x122: call to 
> do_strnlen_user() with UACCESS enabled

Does this fix it?

diff --git a/lib/strncpy_from_user.c b/lib/strncpy_from_user.c
index 706020b06617..cb3ae7a093c3 100644
--- a/lib/strncpy_from_user.c
+++ b/lib/strncpy_from_user.c
@@ -30,6 +30,9 @@ static inline long do_strncpy_from_user(char *dst, const char 
__user *src,
        const struct word_at_a_time constants = WORD_AT_A_TIME_CONSTANTS;
        unsigned long res = 0;
 
+       if (!user_access_begin(src, max))
+               return -EFAULT;
+
        if (IS_UNALIGNED(src, dst))
                goto byte_at_a_time;
 
@@ -43,7 +46,8 @@ static inline long do_strncpy_from_user(char *dst, const char 
__user *src,
                if (has_zero(c, &data, &constants)) {
                        data = prep_zero_mask(c, data, &constants);
                        data = create_zero_mask(data);
-                       return res + find_zero(data);
+                       res += find_zero(data);
+                       goto done;
                }
                res += sizeof(unsigned long);
                max -= sizeof(unsigned long);
@@ -56,7 +60,7 @@ static inline long do_strncpy_from_user(char *dst, const char 
__user *src,
                unsafe_get_user(c,src+res, efault);
                dst[res] = c;
                if (!c)
-                       return res;
+                       goto done;
                res++;
                max--;
        }
@@ -65,14 +69,20 @@ static inline long do_strncpy_from_user(char *dst, const 
char __user *src,
         * Uhhuh. We hit 'max'. But was that the user-specified maximum
         * too? If so, that's ok - we got as much as the user asked for.
         */
-       if (res >= count)
-               return res;
+       if (res < count) {
+               /*
+                * Nope: we hit the address space limit, and we still had more
+                * characters the caller would have wanted. That's an EFAULT.
+                */
+               goto efault;
+       }
+
+done:
+       user_access_end();
+       return res;
 
-       /*
-        * Nope: we hit the address space limit, and we still had more
-        * characters the caller would have wanted. That's an EFAULT.
-        */
 efault:
+       user_access_end();
        return -EFAULT;
 }
 
@@ -105,7 +115,6 @@ long strncpy_from_user(char *dst, const char __user *src, 
long count)
        src_addr = (unsigned long)untagged_addr(src);
        if (likely(src_addr < max_addr)) {
                unsigned long max = max_addr - src_addr;
-               long retval;
 
                /*
                 * Truncate 'max' to the user-specified limit, so that
@@ -116,11 +125,7 @@ long strncpy_from_user(char *dst, const char __user *src, 
long count)
 
                kasan_check_write(dst, count);
                check_object_size(dst, count, false);
-               if (user_access_begin(src, max)) {
-                       retval = do_strncpy_from_user(dst, src, count, max);
-                       user_access_end();
-                       return retval;
-               }
+               return do_strncpy_from_user(dst, src, count, max);
        }
        return -EFAULT;
 }
diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c
index 41670d4a5816..f1e9e447bef1 100644
--- a/lib/strnlen_user.c
+++ b/lib/strnlen_user.c
@@ -26,6 +26,9 @@ static inline long do_strnlen_user(const char __user *src, 
unsigned long count,
        unsigned long align, res = 0;
        unsigned long c;
 
+       if (!user_access_begin(src, max))
+               return 0;
+
        /*
         * Do everything aligned. But that means that we
         * need to also expand the maximum..
@@ -39,10 +42,12 @@ static inline long do_strnlen_user(const char __user *src, 
unsigned long count,
 
        for (;;) {
                unsigned long data;
+
                if (has_zero(c, &data, &constants)) {
                        data = prep_zero_mask(c, data, &constants);
                        data = create_zero_mask(data);
-                       return res + find_zero(data) + 1 - align;
+                       res += find_zero(data) + 1 - align;
+                       goto done;
                }
                res += sizeof(unsigned long);
                /* We already handled 'unsigned long' bytes. Did we do it all ? 
*/
@@ -58,13 +63,21 @@ static inline long do_strnlen_user(const char __user *src, 
unsigned long count,
         * too? If so, return the marker for "too long".
         */
        if (res >= count)
-               return count+1;
+               res = count + 1;
+       else {
+               /*
+                * Nope: we hit the address space limit, and we still had more
+                * characters the caller would have wanted. That's 0.
+                */
+               goto efault;
+       }
+
+done:
+       user_access_end();
+       return res;
 
-       /*
-        * Nope: we hit the address space limit, and we still had more
-        * characters the caller would have wanted. That's 0.
-        */
 efault:
+       user_access_end();
        return 0;
 }
 
@@ -100,7 +113,6 @@ long strnlen_user(const char __user *str, long count)
        src_addr = (unsigned long)untagged_addr(str);
        if (likely(src_addr < max_addr)) {
                unsigned long max = max_addr - src_addr;
-               long retval;
 
                /*
                 * Truncate 'max' to the user-specified limit, so that
@@ -109,11 +121,7 @@ long strnlen_user(const char __user *str, long count)
                if (max > count)
                        max = count;
 
-               if (user_access_begin(str, max)) {
-                       retval = do_strnlen_user(str, count, max);
-                       user_access_end();
-                       return retval;
-               }
+               return do_strnlen_user(str, count, max);
        }
        return 0;
 }

Reply via email to