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 <[email protected]> +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" <[email protected]> +Closes: https://lore.kernel.org/all/[email protected]/ +Reported-by: Nilay Shroff <[email protected]> +Closes: https://lore.kernel.org/all/[email protected]/ +Tested-by: Nilay Shroff <[email protected]> +Acked-by: Yury Norov <[email protected]> +Acked-by: Greg Kroah-Hartman <[email protected]> +Signed-off-by: Kees Cook <[email protected]> +--- + 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
