From: Mieczyslaw Nalewaj Date: Tue, 1 Apr 2025 10:41:55 +0000 (+0200) Subject: kernel: fortify: Hide run-time copy size from value range tracking X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7e5bccd0112a35d842bc4c06b08addaa5fbc88b8;p=thirdparty%2Fopenwrt.git kernel: fortify: Hide run-time copy size from value range tracking Fix compilation warning treated as an error: ./include/linux/fortify-string.h:114:33: error: '__builtin_memcpy' reading between 65 and 536870904 bytes from a region of size 64 [-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:735:48: note: source object 'pcpumask' of size [0, 64] 735 | cpumask_var_t pcpumask, | ~~~~~~~~~~~~~~^~~~~~~~ Link: https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=239d87327dcd361b0098038995f8908f3296864f Signed-off-by: Mieczyslaw Nalewaj Link: https://github.com/openwrt/openwrt/pull/16547 Signed-off-by: Christian Marangi --- diff --git a/target/linux/generic/backport-6.12/902-v6.13-fortify-Hide-run-time-copy-size-from-value-range-tracking.patch b/target/linux/generic/backport-6.12/902-v6.13-fortify-Hide-run-time-copy-size-from-value-range-tracking.patch new file mode 100644 index 00000000000..8dca6498667 --- /dev/null +++ b/target/linux/generic/backport-6.12/902-v6.13-fortify-Hide-run-time-copy-size-from-value-range-tracking.patch @@ -0,0 +1,155 @@ +From 239d87327dcd361b0098038995f8908f3296864f Mon Sep 17 00:00:00 2001 +From: Kees Cook +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" +Closes: https://lore.kernel.org/all/db7190c8-d17f-4a0d-bc2f-5903c79f36c2@t-8ch.de/ +Reported-by: Nilay Shroff +Closes: https://lore.kernel.org/all/20241112124127.1666300-1-nilay@linux.ibm.com/ +Tested-by: Nilay Shroff +Acked-by: Yury Norov +Acked-by: Greg Kroah-Hartman +Signed-off-by: Kees Cook +--- + include/linux/fortify-string.h | 14 +++++++++++++- + 1 file changed, 13 insertions(+), 1 deletion(-) + +--- a/include/linux/fortify-string.h ++++ b/include/linux/fortify-string.h +@@ -616,6 +616,12 @@ __FORTIFY_INLINE bool fortify_memcpy_chk + 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 + 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 + __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); \ + }) + + /*