- 02 Mar, 2024 2 commits
-
-
Kees Cook authored
A common use of type_max() is to find the max for the type of a variable. Using the pattern type_max(typeof(var)) is needlessly verbose. Instead, since typeof(type) == type we can just explicitly call typeof() on the argument to type_max() and type_min(). Add wrappers for readability. We can do some replacements right away: $ git grep '\btype_\(min\|max\)(typeof' | wc -l 11 Link: https://lore.kernel.org/r/20240301062221.work.840-kees@kernel.orgSigned-off-by:
Kees Cook <keescook@chromium.org>
-
Vasiliy Kovalev authored
The changes are similar to those given in the commit 19b070fe ("VMCI: Fix memcpy() run-time warning in dg_dispatch_as_host()"). Fix filling of the msg and msg_payload in dg_info struct, which prevents a possible "detected field-spanning write" of memcpy warning that is issued by the tracking mechanism __fortify_memcpy_chk. Signed-off-by:
Vasiliy Kovalev <kovalev@altlinux.org> Link: https://lore.kernel.org/r/20240219105315.76955-1-kovalev@altlinux.orgSigned-off-by:
-
- 01 Mar, 2024 3 commits
-
-
Andy Shevchenko authored
The new flags parameter allows controlling - Whether or not the units suffix is separated by a space, for compatibility with sort -h - Whether or not to append a B suffix - we're not always printing bytes. Co-developed-by:
Kent Overstreet <kent.overstreet@linux.dev> Signed-off-by:
Andy Shevchenko <andriy.shevchenko@linux.intel.com> Reviewed-by:
-
Kees Cook authored
When building with CONFIG_XEN_PV=y, .text symbols are emitted into the .notes section so that Xen can find the "startup_xen" entry point. This information is used prior to booting the kernel, so relocations are not useful. In fact, performing relocations against the .notes section means that the KASLR base is exposed since /sys/kernel/notes is world-readable. To avoid leaking the KASLR base without breaking unprivileged tools that are expecting to read /sys/kernel/notes, skip performing relocations in the .notes section. The values readable in .notes are then identical to those found in System.map. Reported-by:
Guixiong Wei <guixiongwei@gmail.com> Closes: https://lore.kernel.org/all/20240218073501.54555-1-guixiongwei@gmail.com/ Fixes: 5ead97c8 ("xen: Core Xen implementation") Fixes: da1a679c ("Add /sys/kernel/notes") Reviewed-by:
Juergen Gross <jgross@suse.com> Signed-off-by:
-
Josh Poimboeuf authored
If SAVE and RESTORE unwind hints are in different basic blocks, and objtool sees the RESTORE before the SAVE, it errors out with: vmlinux.o: warning: objtool: vmw_port_hb_in+0x242: objtool isn't smart enough to handle this CFI save/restore combo In such a case, defer following the RESTORE block until the straight-line path gets followed later. Fixes: 8faea26e ("objtool: Re-add UNWIND_HINT_{SAVE_RESTORE}") Reported-by:
kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202402240702.zJFNmahW-lkp@intel.com/Signed-off-by:
Josh Poimboeuf <jpoimboe@kernel.org> Link: https://lore.kernel.org/r/20240227073527.avcm5naavbv3cj5s@trebleSigned-off-by:
-
- 29 Feb, 2024 21 commits
-
-
Andy Shevchenko authored
The check_shl_overflow() uses u64 type that is defined in types.h. Instead of including that header, just switch to use POD type directly. Signed-off-by:
-
Kees Cook authored
For reasons I cannot understand, m68k moves the start of the stack frame for consecutive calls to the same function if the function's test variable is larger than 8 bytes. This was only happening for the char array test (obviously), so adjust the length of the string for m68k only. I want the array size to be longer than "unsigned long" for every given architecture, so the other remain unchanged. Additionally adjust the error message to be a bit more clear about what's happened, and move the KUNIT check outside of the consecutive calls to minimize what happens between them. Reported-by:
Guenter Roeck <linux@roeck-us.net> Closes: https://lore.kernel.org/lkml/a0d10d50-2720-4ecd-a2c6-c2c5e5aeee65@roeck-us.net/Tested-by:
Geert Uytterhoeven <geert@linux-m68k.org> Closes: https://lore.kernel.org/r/CAMuHMdX_g1tbiUL9PUQdqaegrEzCNN3GtbSvSBFYAL4TzvstFg@mail.gmail.com Closes: https://lore.kernel.org/r/CAMuHMdW6N40+0gGQ+LSrN64Mo4A0-ELAm0pR3gWQ0mNanyBuUQ@mail.gmail.comTested-by:
-
Kees Cook authored
The UBSAN instrumentation cannot work in the vDSO since it is executing in userspace, so disable it in the Makefile. Fixes the build failures such as: arch/sparc/vdso/vclock_gettime.c:217: undefined reference to `__ubsan_handle_shift_out_of_bounds' Acked-by:
Sam Ravnborg <sam@ravnborg.org> Link: https://lore.kernel.org/all/20240224073617.GA2959352@ravnborg.orgSigned-off-by:
-
Andy Shevchenko authored
The lib/cmdline.c is basically a set of some small string parsers which are wide used in the kernel. Their prototypes belong to the string.h rather then kernel.h. Signed-off-by:
-
Kees Cook authored
Introduce --kallsyms argument for scanning binary files for known symbol addresses. This would have found the exposure in /sys/kernel/notes: $ scripts/leaking_addresses.pl --kallsyms=<(sudo cat /proc/kallsyms) /sys/kernel/notes: hypercall_page @ 156 /sys/kernel/notes: xen_hypercall_set_trap_table @ 156 /sys/kernel/notes: startup_xen @ 132 Acked-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org> Reviewed-by:
Tycho Andersen <tandersen@netflix.com> Link: https://lore.kernel.org/r/20240222220053.1475824-4-keescook@chromium.orgSigned-off-by:
-
Kees Cook authored
These are false positives from the input subsystem: /proc/bus/input/devices: B: KEY=402000000 3803078f800d001 feffffdfffefffff fffffffffffffffe /sys/devices/platform/i8042/serio0/input/input1/uevent: KEY=402000000 3803078f800d001 feffffdfffefffff fffffffffffffffe /sys/devices/platform/i8042/serio0/input/input1/capabilities/key: 402000000 3803078f800d001 feffffdf Pass in the filename for more context and expand the "ignored pattern" matcher to notice these. Reviewed-by:
-
Kees Cook authored
Instead of using a statically named path in /tmp, use File::Temp to create (and remove) the temporary file used for parsing /proc/config.gz. Reviewed-by:
-
Kees Cook authored
Tobin hasn't been involved lately, and I can step up to be a reviewer with Tycho. I'll carry changes via the hardening tree. Reviewed-by:
-
Kees Cook authored
Improve the reporting of buffer overflows under CONFIG_FORTIFY_SOURCE to help accelerate debugging efforts. The calculations are all just sitting in registers anyway, so pass them along to the function to be reported. For example, before: detected buffer overflow in memcpy and after: memcpy: detected buffer overflow: 4096 byte read of buffer size 1 Link: https://lore.kernel.org/r/20230407192717.636137-10-keescook@chromium.orgSigned-off-by:
-
Kees Cook authored
With fortify overflows able to be redirected, we can use KUnit to exercise the overflow conditions. Add tests for every API covered by CONFIG_FORTIFY_SOURCE, except for memset() and memcpy(), which are special-cased for now. Disable warnings in the Makefile since we're explicitly testing known-bad string handling code patterns. Note that this makes the LKDTM FORTIFY_STR* tests obsolete, but those can be removed separately. Signed-off-by: -
Kees Cook authored
The standard C string APIs were not designed to have a failure mode; they were expected to always succeed without memory safety issues. Normally, CONFIG_FORTIFY_SOURCE will use fortify_panic() to stop processing, as truncating a read or write may provide an even worse system state. However, this creates a problem for testing under things like KUnit, which needs a way to survive failures. When building with CONFIG_KUNIT, provide a failure path for all users of fortify_panic, and track whether the failure was a read overflow or a write overflow, for KUnit tests to examine. Inspired by similar logic in the slab tests. Signed-off-by: -
Kees Cook authored
In order for CI systems to notice all the skipped tests related to CONFIG_FORTIFY_SOURCE, allow the FORTIFY_SOURCE KUnit tests to build with or without CONFIG_FORTIFY_SOURCE. Signed-off-by: -
Kees Cook authored
In preparation for KUnit testing and further improvements in fortify failure reporting, split out the report and encode the function and access failure (read or write overflow) into a single u8 argument. This mainly ends up saving a tiny bit of space in the data segment. For a defconfig with FORTIFY_SOURCE enabled: $ size gcc/vmlinux.before gcc/vmlinux.after text data bss dec hex filename 26132309 9760658 2195460 38088427 2452eeb gcc/vmlinux.before 26132386 9748382 2195460 38076228 244ff44 gcc/vmlinux.after Reviewed-by:
Alexander Lobakin <aleksander.lobakin@intel.com> Signed-off-by:
-
Kees Cook authored
Mark the various refcount_t functions with __signed_wrap, as we depend on the wrapping behavior to detect the overflow and perform saturation. Silences warnings seen with the LKDTM REFCOUNT_* tests: UBSAN: signed-integer-overflow in ../include/linux/refcount.h:189:11 2147483647 + 1 cannot be represented in type 'int' Reviewed-by:
Miguel Ojeda <ojeda@kernel.org> Link: https://lore.kernel.org/r/20240221051634.work.287-kees@kernel.orgSigned-off-by:
-
Jacob Keller authored
include/linux/overflow.h includes helper macros intended for calculating sizes of allocations. These macros prevent accidental overflow by saturating at SIZE_MAX. In general when calculating such sizes use of the macros is preferred. Add a semantic patch which can detect code patterns which can be replaced by struct_size. Note that I set the confidence to medium because this patch doesn't make an attempt to ensure that the relevant array is actually a flexible array. The struct_size macro does specifically require a flexible array. In many cases the detected code could be refactored to a flexible array, but this is not always possible (such as if there are multiple over-allocations). Signed-off-by:
Jacob Keller <jacob.e.keller@intel.com> Link: https://lore.kernel.org/r/20230227202428.3657443-1-jacob.e.keller@intel.comSigned-off-by:
-
Kees Cook authored
Add rules for finding places where str_plural() can be used. This currently finds: 54 files changed, 62 insertions(+), 61 deletions(-) Co-developed-by:
Michal Wajdeczko <michal.wajdeczko@intel.com> Signed-off-by:
-
Michal Wajdeczko authored
Add str_plural() helper to replace existing open implementations used by many drivers and help improve future user facing messages. Signed-off-by:
-
Kees Cook authored
This allows replacements of the idioms "var += offset" and "var -= offset" with the wrapping_assign_add() and wrapping_assign_sub() helpers respectively. They will avoid wrap-around sanitizer instrumentation. Add to the selftests to validate behavior and lack of side-effects. Reviewed-by:
Marco Elver <elver@google.com> Acked-by:
Mark Rutland <mark.rutland@arm.com> Signed-off-by:
-
Kees Cook authored
Provide helpers that will perform wrapping addition, subtraction, or multiplication without tripping the arithmetic wrap-around sanitizers. The first argument is the type under which the wrap-around should happen with. In other words, these two calls will get very different results: wrapping_mul(int, 50, 50) == 2500 wrapping_mul(u8, 50, 50) == 196 Add to the selftests to validate behavior and lack of side-effects. Reviewed-by:
Gustavo A. R. Silva <gustavoars@kernel.org> Reviewed-by:
-
Kees Cook authored
The check_*_overflow() helpers will return results with potentially wrapped-around values. These values have always been checked by the selftests, so avoid the confusing language in the kern-doc. The idea of "safe for use" was relative to the expectation of whether or not the caller wants a wrapped value -- the calculation itself will always follow arithmetic wrapping rules. Reviewed-by:
-
Andy Shevchenko authored
The wordpart.h header is collecting APIs related to the handling parts of the word (usually in byte granularity). The upper_*_bits() and lower_*_bits() are good candidates to be moved to there. This helps to clean up header dependency hell with regard to kernel.h as the latter gathers completely unrelated stuff together and slows down compilation (especially when it's included into other header). Signed-off-by:
Randy Dunlap <rdunlap@infradead.org> Signed-off-by:
-
- 21 Feb, 2024 8 commits
-
-
Marco Elver authored
KFENCE is not a security mitigation mechanism (due to sampling), but has the performance characteristics of unintrusive hardening techniques. When used at scale, however, it improves overall security by allowing kernel developers to detect heap memory-safety bugs cheaply. Link: https://lkml.kernel.org/r/79B9A832-B3DE-4229-9D87-748B2CFB7D12@kernel.org Cc: Matthieu Baerts <matttbe@kernel.org> Cc: Jakub Kicinski <kuba@kernel.org> Signed-off-by:
-
Lukas Bulwahn authored
Commit 94f8f319 ("drm: Remove Kconfig option for legacy support (CONFIG_DRM_LEGACY)") removes the config DRM_LEGACY, but one reference to that config is left in the hardening.config fragment. As there is no drm legacy driver left, we do not need to recommend this attack surface reduction anymore. Drop this reference in hardening.config fragment. Signed-off-by:
Lukas Bulwahn <lukas.bulwahn@gmail.com> Link: https://lore.kernel.org/r/20240208091045.9219-3-lukas.bulwahn@gmail.comSigned-off-by:
-
Lukas Bulwahn authored
Commit 7a628f818499 ("ubsan: Remove CONFIG_UBSAN_SANITIZE_ALL") removes the config UBSAN_SANITIZE_ALL, but one reference to that config is left in the hardening.config fragment. Drop this reference in hardening.config fragment. Note that CONFIG_UBSAN is still enabled in the hardening.config fragment, so the functionality when using this fragment remains the same. Signed-off-by: -
Kees Cook authored
The ARCH=um build has its own idea about strscpy()'s definition. Adjust the callers to remove the redundant sizeof() arguments ahead of treewide changes, since it needs a manual adjustment for the newly named sized_strscpy() export. Cc: Richard Weinberger <richard@nod.at> Cc: linux-um@lists.infradead.org Signed-off-by: -
Kees Cook authored
Similar to strscpy(), update strscpy_pad()'s 3rd argument to be optional when the destination is a compile-time known size array. Cc: Andy Shevchenko <andy@kernel.org> Cc: <linux-hardening@vger.kernel.org> Reviewed-by:
Justin Stitt <justinstitt@google.com> Signed-off-by:
-
Kees Cook authored
Using sizeof(dst) for the "size" argument in strscpy() is the overwhelmingly common case. Instead of requiring this everywhere, allow a 2-argument version to be used that will use the sizeof() internally. There are other functions in the kernel with optional arguments[1], so this isn't unprecedented, and improves readability. Update and relocate the kern-doc for strscpy() too, and drop __HAVE_ARCH_STRSCPY as it is unused. Adjust ARCH=um build to notice the changed export name, as it doesn't do full header includes for the string helpers. This could additionally let us save a few hundred lines of code: 1177 files changed, 2455 insertions(+), 3026 deletions(-) with a treewide cleanup using Coccinelle: @needless_arg@ expression DST, SRC; @@ strscpy(DST, SRC -, sizeof(DST) ) Link: https://elixir.bootlin.com/linux/v6.7/source/include/linux/pci.h#L1517 [1] Reviewed-by: -
Kees Cook authored
In preparation for making strscpy_pad()'s 3rd argument optional, redefine it as a macro. This also has the benefit of allowing greater FORITFY introspection, as it couldn't see into the strscpy() nor the memset() within strscpy_pad(). Cc: Andy Shevchenko <andy@kernel.org> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: <linux-hardening@vger.kernel.org> Reviewed-by:
-
Kees Cook authored
In order to mitigate unexpected signed wrap-around[1], bring back the signed integer overflow sanitizer. It was removed in commit 6aaa31ae ("ubsan: remove overflow checks") because it was effectively a no-op when combined with -fno-strict-overflow (which correctly changes signed overflow from being "undefined" to being explicitly "wrap around"). Compilers are adjusting their sanitizers to trap wrap-around and to detecting common code patterns that should not be instrumented (e.g. "var + offset < var"). Prepare for this and explicitly rename the option from "OVERFLOW" to "WRAP" to more accurately describe the behavior. To annotate intentional wrap-around arithmetic, the helpers wrapping_add/sub/mul_wrap() can be used for individual statements. At the function level, the __signed_wrap attribute can be used to mark an entire function as expecting its signed arithmetic to wrap around. For a single object file the Makefile can use "UBSAN_SIGNED_WRAP_target.o := n" to mark it as wrapping, and for an entire directory, "UBSAN_SIGNED_WRAP := n" can be used. Additionally keep these disabled under CONFIG_COMPILE_TEST for now. Link: https://github.com/KSPP/linux/issues/26 [1] Cc: Miguel Ojeda <ojeda@kernel.org> Cc: Nathan Chancellor <nathan@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Hao Luo <haoluo@google.com> Reviewed-by:
-
- 06 Feb, 2024 4 commits
-
-
Kees Cook authored
For simplicity in splitting out UBSan options into separate rules, remove CONFIG_UBSAN_SANITIZE_ALL, effectively defaulting to "y", which is how it is generally used anyway. (There are no ":= y" cases beyond where a specific file is enabled when a top-level ":= n" is in effect.) Cc: Andrey Konovalov <andreyknvl@gmail.com> Cc: Marco Elver <elver@google.com> Cc: linux-doc@vger.kernel.org Cc: linux-kbuild@vger.kernel.org Signed-off-by: -
Kees Cook authored
Silence a handful of W=1 warnings in the UBSan selftest, which set variables without using them. For example: lib/test_ubsan.c:101:6: warning: variable 'val1' set but not used [-Wunused-but-set-variable] 101 | int val1 = 10; | ^ Reported-by: -
Kees Cook authored
Clang changed the way it enables UBSan trapping mode. Update the Makefile logic to discover it. Suggested-by:
Fangrui Song <maskray@google.com> Link: https://lore.kernel.org/lkml/CAFP8O3JivZh+AAV7N90Nk7U2BHRNST6MRP0zHtfQ-Vj0m4+pDA@mail.gmail.com/Reviewed-by:
-
Kees Cook authored
The kernel hardening efforts have continued to depend more and more heavily on UBSAN, so make an actual MAINTAINERS entry for it. Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com> Acked-by:
Andrey Konovalov <andreyknvl@gmail.com> Acked-by:
-
- 01 Feb, 2024 2 commits
-
-
Harshit Mogalapalli authored
Syzkaller hit 'WARNING in dg_dispatch_as_host' bug. memcpy: detected field-spanning write (size 56) of single field "&dg_info->msg" at drivers/misc/vmw_vmci/vmci_datagram.c:237 (size 24) WARNING: CPU: 0 PID: 1555 at drivers/misc/vmw_vmci/vmci_datagram.c:237 dg_dispatch_as_host+0x88e/0xa60 drivers/misc/vmw_vmci/vmci_datagram.c:237 Some code commentry, based on my understanding: 544 #define VMCI_DG_SIZE(_dg) (VMCI_DG_HEADERSIZE + (size_t)(_dg)->payload_size) /// This is 24 + payload_size memcpy(&dg_info->msg, dg, dg_size); Destination = dg_info->msg ---> this is a 24 byte structure(struct vmci_datagram) Source = dg --> this is a 24 byte structure (struct vmci_datagram) Size = dg_size = 24 + payload_size {payload_size = 56-24 =32} -- Syzkaller managed to set payload_size to 32. 35 struct delayed_datagram_info { 36 struct datagram_entry *entry; 37 struct work_struct work; 38 bool in_dg_host_queue; 39 /* msg and msg_payload must be together. */ 40 struct vmci_datagram msg; 41 u8 msg_payload[]; 42 }; So those extra bytes of payload are copied into msg_payload[], a run time warning is seen while fuzzing with Syzkaller. One possible way to fix the warning is to split the memcpy() into two parts -- one -- direct assignment of msg and second taking care of payload. Gustavo quoted: "Under FORTIFY_SOURCE we should not copy data across multiple members in a structure." Reported-by:syzkaller <syzkaller@googlegroups.com> Suggested-by:
Vegard Nossum <vegard.nossum@oracle.com> Suggested-by:
Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> Reviewed-by:
Dan Carpenter <dan.carpenter@linaro.org> Link: https://lore.kernel.org/r/20240105164001.2129796-2-harshit.m.mogalapalli@oracle.comSigned-off-by:
-
Harshit Mogalapalli authored
Use struct_size() instead of open coding. Suggested-by:
-
