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

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]
Date:   Thu, 30 Apr 2020 20:07:33 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        the arch/x86 maintainers <x86@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Remaining randconfig objtool warnings, linux-next-20200428

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