Re: Remaining randconfig objtool warnings, linux-next-20200428

From: Josh Poimboeuf
Date: Thu Apr 30 2020 - 21:07:44 EST


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;
}