[gentoo-commits] proj/linux-patches:6.12 commit in: /

["Mike Pagano" <mpagano@gentoo.org>]

commit:     f8e6e0a09a78ef67abed5a29f23c6a2db0d259e9
Author:     Mike Pagano <mpagano <AT> gentoo <DOT> org>
AuthorDate: Sun Feb 16 21:48:06 2025 +0000
Commit:     Mike Pagano <mpagano <AT> gentoo <DOT> org>
CommitDate: Sun Feb 16 21:48:06 2025 +0000
URL:        https://gitweb.gentoo.org/proj/linux-patches.git/commit/?id=f8e6e0a0

fortify: Hide run-time copy size from value range tracking

Bug: https://bugs.gentoo.org/947270

Signed-off-by: Mike Pagano <mpagano <AT> gentoo.org>

 0000_README                                        |   4 +
 ...ortify-copy-size-value-range-tracking-fix.patch | 161 +++++++++++++++++++++
 2 files changed, 165 insertions(+)

diff --git a/0000_README b/0000_README
index ceb862e7..499702fa 100644
--- a/0000_README
+++ b/0000_README
@@ -95,6 +95,10 @@ Patch:  1012_linux-6.12.13.patch
 From:   https://www.kernel.org
 Desc:   Linux 6.12.13
 
+Patch:  1500_fortify-copy-size-value-range-tracking-fix.patch
+From:   https://git.kernel.org/
+Desc:   fortify: Hide run-time copy size from value range tracking
+
 Patch:  1510_fs-enable-link-security-restrictions-by-default.patch
 From:   http://sources.debian.net/src/linux/3.16.7-ckt4-3/debian/patches/debian/fs-enable-link-security-restrictions-by-default.patch/
 Desc:   Enable link security restrictions by default.

diff --git a/1500_fortify-copy-size-value-range-tracking-fix.patch b/1500_fortify-copy-size-value-range-tracking-fix.patch
new file mode 100644
index 00000000..f751e02c
--- /dev/null
+++ b/1500_fortify-copy-size-value-range-tracking-fix.patch
@@ -0,0 +1,161 @@
+From 239d87327dcd361b0098038995f8908f3296864f Mon Sep 17 00:00:00 2001
+From: Kees Cook <kees@kernel.org>
+Date: Thu, 12 Dec 2024 17:28:06 -0800
+Subject: fortify: Hide run-time copy size from value range tracking
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+GCC performs value range tracking for variables as a way to provide better
+diagnostics. One place this is regularly seen is with warnings associated
+with bounds-checking, e.g. -Wstringop-overflow, -Wstringop-overread,
+-Warray-bounds, etc. In order to keep the signal-to-noise ratio high,
+warnings aren't emitted when a value range spans the entire value range
+representable by a given variable. For example:
+
+	unsigned int len;
+	char dst[8];
+	...
+	memcpy(dst, src, len);
+
+If len's value is unknown, it has the full "unsigned int" range of [0,
+UINT_MAX], and GCC's compile-time bounds checks against memcpy() will
+be ignored. However, when a code path has been able to narrow the range:
+
+	if (len > 16)
+		return;
+	memcpy(dst, src, len);
+
+Then the range will be updated for the execution path. Above, len is
+now [0, 16] when reading memcpy(), so depending on other optimizations,
+we might see a -Wstringop-overflow warning like:
+
+	error: '__builtin_memcpy' writing between 9 and 16 bytes into region of size 8 [-Werror=stringop-overflow]
+
+When building with CONFIG_FORTIFY_SOURCE, the fortified run-time bounds
+checking can appear to narrow value ranges of lengths for memcpy(),
+depending on how the compiler constructs the execution paths during
+optimization passes, due to the checks against the field sizes. For
+example:
+
+	if (p_size_field != SIZE_MAX &&
+	    p_size != p_size_field && p_size_field < size)
+
+As intentionally designed, these checks only affect the kernel warnings
+emitted at run-time and do not block the potentially overflowing memcpy(),
+so GCC thinks it needs to produce a warning about the resulting value
+range that might be reaching the memcpy().
+
+We have seen this manifest a few times now, with the most recent being
+with cpumasks:
+
+In function ‘bitmap_copy’,
+    inlined from ‘cpumask_copy’ at ./include/linux/cpumask.h:839:2,
+    inlined from ‘__padata_set_cpumasks’ at kernel/padata.c:730:2:
+./include/linux/fortify-string.h:114:33: error: ‘__builtin_memcpy’ reading between 257 and 536870904 bytes from a region of size 256 [-Werror=stringop-overread]
+  114 | #define __underlying_memcpy     __builtin_memcpy
+      |                                 ^
+./include/linux/fortify-string.h:633:9: note: in expansion of macro ‘__underlying_memcpy’
+  633 |         __underlying_##op(p, q, __fortify_size);                        \
+      |         ^~~~~~~~~~~~~
+./include/linux/fortify-string.h:678:26: note: in expansion of macro ‘__fortify_memcpy_chk’
+  678 | #define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                  \
+      |                          ^~~~~~~~~~~~~~~~~~~~
+./include/linux/bitmap.h:259:17: note: in expansion of macro ‘memcpy’
+  259 |                 memcpy(dst, src, len);
+      |                 ^~~~~~
+kernel/padata.c: In function ‘__padata_set_cpumasks’:
+kernel/padata.c:713:48: note: source object ‘pcpumask’ of size [0, 256]
+  713 |                                  cpumask_var_t pcpumask,
+      |                                  ~~~~~~~~~~~~~~^~~~~~~~
+
+This warning is _not_ emitted when CONFIG_FORTIFY_SOURCE is disabled,
+and with the recent -fdiagnostics-details we can confirm the origin of
+the warning is due to FORTIFY's bounds checking:
+
+../include/linux/bitmap.h:259:17: note: in expansion of macro 'memcpy'
+  259 |                 memcpy(dst, src, len);
+      |                 ^~~~~~
+  '__padata_set_cpumasks': events 1-2
+../include/linux/fortify-string.h:613:36:
+  612 |         if (p_size_field != SIZE_MAX &&
+      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  613 |             p_size != p_size_field && p_size_field < size)
+      |             ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
+      |                                    |
+      |                                    (1) when the condition is evaluated to false
+      |                                    (2) when the condition is evaluated to true
+  '__padata_set_cpumasks': event 3
+  114 | #define __underlying_memcpy     __builtin_memcpy
+      |                                 ^
+      |                                 |
+      |                                 (3) out of array bounds here
+
+Note that the cpumask warning started appearing since bitmap functions
+were recently marked __always_inline in commit ed8cd2b3bd9f ("bitmap:
+Switch from inline to __always_inline"), which allowed GCC to gain
+visibility into the variables as they passed through the FORTIFY
+implementation.
+
+In order to silence these false positives but keep otherwise deterministic
+compile-time warnings intact, hide the length variable from GCC with
+OPTIMIZE_HIDE_VAR() before calling the builtin memcpy.
+
+Additionally add a comment about why all the macro args have copies with
+const storage.
+
+Reported-by: "Thomas Weißschuh" <linux@weissschuh.net>
+Closes: https://lore.kernel.org/all/db7190c8-d17f-4a0d-bc2f-5903c79f36c2@t-8ch.de/
+Reported-by: Nilay Shroff <nilay@linux.ibm.com>
+Closes: https://lore.kernel.org/all/20241112124127.1666300-1-nilay@linux.ibm.com/
+Tested-by: Nilay Shroff <nilay@linux.ibm.com>
+Acked-by: Yury Norov <yury.norov@gmail.com>
+Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+Signed-off-by: Kees Cook <kees@kernel.org>
+---
+ include/linux/fortify-string.h | 14 +++++++++++++-
+ 1 file changed, 13 insertions(+), 1 deletion(-)
+
+(limited to 'include/linux/fortify-string.h')
+
+diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
+index 0d99bf11d260a3..e4ce1cae03bf77 100644
+--- a/include/linux/fortify-string.h
++++ b/include/linux/fortify-string.h
+@@ -616,6 +616,12 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
+ 	return false;
+ }
+ 
++/*
++ * To work around what seems to be an optimizer bug, the macro arguments
++ * need to have const copies or the values end up changed by the time they
++ * reach fortify_warn_once(). See commit 6f7630b1b5bc ("fortify: Capture
++ * __bos() results in const temp vars") for more details.
++ */
+ #define __fortify_memcpy_chk(p, q, size, p_size, q_size,		\
+ 			     p_size_field, q_size_field, op) ({		\
+ 	const size_t __fortify_size = (size_t)(size);			\
+@@ -623,6 +629,8 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
+ 	const size_t __q_size = (q_size);				\
+ 	const size_t __p_size_field = (p_size_field);			\
+ 	const size_t __q_size_field = (q_size_field);			\
++	/* Keep a mutable version of the size for the final copy. */	\
++	size_t __copy_size = __fortify_size;				\
+ 	fortify_warn_once(fortify_memcpy_chk(__fortify_size, __p_size,	\
+ 				     __q_size, __p_size_field,		\
+ 				     __q_size_field, FORTIFY_FUNC_ ##op), \
+@@ -630,7 +638,11 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
+ 		  __fortify_size,					\
+ 		  "field \"" #p "\" at " FILE_LINE,			\
+ 		  __p_size_field);					\
+-	__underlying_##op(p, q, __fortify_size);			\
++	/* Hide only the run-time size from value range tracking to */	\
++	/* silence compile-time false positive bounds warnings. */	\
++	if (!__builtin_constant_p(__copy_size))				\
++		OPTIMIZER_HIDE_VAR(__copy_size);			\
++	__underlying_##op(p, q, __copy_size);				\
+ })
+ 
+ /*
+-- 
+cgit 1.2.3-korg