aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2023-07-03Revert "fortify: Allow KUnit test to build without FORTIFY"Kees Cook2-15/+1
This reverts commit a9dc8d0442294b426b1ebd4ec6097c82ebe282e0. The standard for KUnit is to not build tests at all when required functionality is missing, rather than doing test "skip". Restore this for the fortify tests, so that architectures without CONFIG_ARCH_HAS_FORTIFY_SOURCE do not emit unsolvable warnings. Reported-by: Geert Uytterhoeven <[email protected]> Closes: https://lore.kernel.org/all/CAMuHMdUrxOEroHVUt7-mAnKSBjY=a-D3jr+XiAifuwv06Ob9Pw@mail.gmail.com Signed-off-by: Kees Cook <[email protected]>
2023-06-28dm: verity-loadpin: Add NULL pointer check for 'bdev' parameterMatthias Kaehlcke1-0/+3
Add a NULL check for the 'bdev' parameter of dm_verity_loadpin_is_bdev_trusted(). The function is called by loadpin_check(), which passes the block device that corresponds to the super block of the file system from which a file is being loaded. Generally a super_block structure has an associated block device, however that is not always the case (e.g. tmpfs). Cc: [email protected] # v6.0+ Fixes: b6c1c5745ccc ("dm: Add verity helpers for LoadPin") Signed-off-by: Matthias Kaehlcke <[email protected]> Link: https://lore.kernel.org/r/20230627202800.1.Id63f7f59536d20f1ab83e1abdc1fda1471c7d031@changeid Signed-off-by: Kees Cook <[email protected]>
2023-06-20netfilter: ipset: Replace strlcpy with strscpyAzeem Shaikh1-5/+5
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). Direct replacement is safe here since return value from all callers of STRLCPY macro were ignored. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Acked-by: Jozsef Kadlecsik <[email protected]> Reviewed-by: Kees Cook <[email protected]> Reviewed-by: Simon Horman <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-20uml: Replace strlcpy with strscpyAzeem Shaikh2-1/+2
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-14um: Use HOST_DIR for mrproperKees Cook1-1/+1
When HEADER_ARCH was introduced, the MRPROPER_FILES (then MRPROPER_DIRS) list wasn't adjusted, leaving SUBARCH as part of the path argument. This resulted in the "mrproper" target not cleaning up arch/x86/... when SUBARCH was specified. Since HOST_DIR is arch/$(HEADER_ARCH), use it instead to get the correct path. Cc: Richard Weinberger <[email protected]> Cc: Anton Ivanov <[email protected]> Cc: Johannes Berg <[email protected]> Cc: Azeem Shaikh <[email protected]> Cc: [email protected] Fixes: 7bbe7204e937 ("um: merge Makefile-{i386,x86_64}") Cc: [email protected] Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-14kallsyms: Replace all non-returning strlcpy with strscpyAzeem Shaikh2-3/+3
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-14sh: Replace all non-returning strlcpy with strscpyAzeem Shaikh2-3/+3
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Acked-by: John Paul Adrian Glaubitz <[email protected]> Tested-by: John Paul Adrian Glaubitz <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-14of/flattree: Replace all non-returning strlcpy with strscpyAzeem Shaikh1-1/+1
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-14sparc64: Replace all non-returning strlcpy with strscpyAzeem Shaikh4-4/+4
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-14Hexagon: Replace all non-returning strlcpy with strscpyAzeem Shaikh1-3/+3
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-05kobject: Use return value of strreplace()Andy Shevchenko1-2/+1
Since strreplace() returns the pointer to the string itself, we may use it directly in the code. Signed-off-by: Andy Shevchenko <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-05lib/string_helpers: Change returned value of the strreplace()Andy Shevchenko2-5/+9
It's more useful to return the pointer to the string itself with strreplace(), so it may be used like attr->name = strreplace(name, '/', '_'); While at it, amend the kernel documentation. Signed-off-by: Andy Shevchenko <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-05jbd2: Avoid printing outside the boundary of the bufferAndy Shevchenko1-4/+2
Theoretically possible that "%pg" will take all room for the j_devname and hence the "-%lu" will go outside the boundary due to unconditional sprintf() in use. To make this code more robust, replace two sequential s*printf():s by a single call and then replace forbidden character. It's possible to do this way, because '/' won't ever be in the result of "-%lu". Reviewed-by: Jan Kara <[email protected]> Signed-off-by: Andy Shevchenko <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-05checkpatch: Check for 0-length and 1-element arraysKees Cook1-0/+10
Fake flexible arrays have been deprecated since last millennium. Proper C99 flexible arrays must be used throughout the kernel so CONFIG_FORTIFY_SOURCE and CONFIG_UBSAN_BOUNDS can provide proper array bounds checking. Cc: Andy Whitcroft <[email protected]> Cc: Dwaipayan Ray <[email protected]> Cc: Lukas Bulwahn <[email protected]> Fixed-by: Joe Perches <[email protected]> Signed-off-by: Kees Cook <[email protected]> Acked-by: Gustavo A. R. Silva <[email protected]> Acked-by: Joe Perches <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-05riscv/purgatory: Do not use fortified string functionsKees Cook1-1/+1
With the addition of -fstrict-flex-arrays=3, struct sha256_state's trailing array is no longer ignored by CONFIG_FORTIFY_SOURCE: struct sha256_state { u32 state[SHA256_DIGEST_SIZE / 4]; u64 count; u8 buf[SHA256_BLOCK_SIZE]; }; This means that the memcpy() calls with "buf" as a destination in sha256.c's code will attempt to perform run-time bounds checking, which could lead to calling missing functions, specifically a potential WARN_ONCE, which isn't callable from purgatory. Reported-by: Thorsten Leemhuis <[email protected]> Closes: https://lore.kernel.org/lkml/[email protected]/ Bisected-by: "Joan Bruguera Micó" <[email protected]> Fixes: df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") Cc: Paul Walmsley <[email protected]> Cc: Palmer Dabbelt <[email protected]> Cc: Albert Ou <[email protected]> Cc: Masahiro Yamada <[email protected]> Cc: Conor Dooley <[email protected]> Cc: Nick Desaulniers <[email protected]> Cc: Alyssa Ross <[email protected]> Cc: Heiko Stuebner <[email protected]> Cc: "Gustavo A. R. Silva" <[email protected]> Cc: [email protected] Signed-off-by: Kees Cook <[email protected]> Reviewed-by: Conor Dooley <[email protected]> Reviewed-by: Palmer Dabbelt <[email protected]> Acked-by: Palmer Dabbelt <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-01s390/purgatory: Do not use fortified string functionsKees Cook1-1/+1
With the addition of -fstrict-flex-arrays=3, struct sha256_state's trailing array is no longer ignored by CONFIG_FORTIFY_SOURCE: struct sha256_state { u32 state[SHA256_DIGEST_SIZE / 4]; u64 count; u8 buf[SHA256_BLOCK_SIZE]; }; This means that the memcpy() calls with "buf" as a destination in sha256.c's code will attempt to perform run-time bounds checking, which could lead to calling missing functions, specifically a potential WARN_ONCE, which isn't callable from purgatory. Reported-by: Thorsten Leemhuis <[email protected]> Closes: https://lore.kernel.org/lkml/[email protected]/ Bisected-by: "Joan Bruguera Micó" <[email protected]> Fixes: df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") Cc: Heiko Carstens <[email protected]> Cc: Vasily Gorbik <[email protected]> Cc: Alexander Gordeev <[email protected]> Cc: Christian Borntraeger <[email protected]> Cc: Sven Schnelle <[email protected]> Cc: Masahiro Yamada <[email protected]> Cc: Linux Kernel Functional Testing <[email protected]> Cc: Nathan Chancellor <[email protected]> Cc: "Gustavo A. R. Silva" <[email protected]> Cc: [email protected] Signed-off-by: Kees Cook <[email protected]> Reviewed-by: Nathan Chancellor <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-01x86/purgatory: Do not use fortified string functionsKees Cook1-1/+1
With the addition of -fstrict-flex-arrays=3, struct sha256_state's trailing array is no longer ignored by CONFIG_FORTIFY_SOURCE: struct sha256_state { u32 state[SHA256_DIGEST_SIZE / 4]; u64 count; u8 buf[SHA256_BLOCK_SIZE]; }; This means that the memcpy() calls with "buf" as a destination in sha256.c's code will attempt to perform run-time bounds checking, which could lead to calling missing functions, specifically a potential WARN_ONCE, which isn't callable from purgatory. Reported-by: Thorsten Leemhuis <[email protected]> Closes: https://lore.kernel.org/lkml/[email protected]/ Bisected-by: "Joan Bruguera Micó" <[email protected]> Fixes: df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") Cc: Thomas Gleixner <[email protected]> Cc: Ingo Molnar <[email protected]> Cc: Borislav Petkov <[email protected]> Cc: Dave Hansen <[email protected]> Cc: [email protected] Cc: "H. Peter Anvin" <[email protected]> Cc: Nick Desaulniers <[email protected]> Cc: Masahiro Yamada <[email protected]> Cc: "Peter Zijlstra (Intel)" <[email protected]> Cc: Alyssa Ross <[email protected]> Cc: Sami Tolvanen <[email protected]> Cc: Alexander Potapenko <[email protected]> Signed-off-by: Kees Cook <[email protected]> Tested-by: Thorsten Leemhuis <[email protected]> Acked-by: Dave Hansen <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-01acpi: Replace struct acpi_table_slit 1-element array with flex-arrayWyes Karny1-1/+1
struct acpi_table_slit is used for copying System Locality Information Table data from ACPI tables. Here `entry` is a flex array but it was using ancient 1-element fake flexible array, which has been deprecated. Replace it with a C99 flexible array. Signed-off-by: Wyes Karny <[email protected]> Reviewed-by: Kees Cook <[email protected]> Fixes: df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-01clocksource: Replace all non-returning strlcpy with strscpyAzeem Shaikh1-1/+1
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Acked-by: John Stultz <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-01string: use __builtin_memcpy() in strlcpy/strlcatAlexander Potapenko1-2/+2
lib/string.c is built with -ffreestanding, which prevents the compiler from replacing certain functions with calls to their library versions. On the other hand, this also prevents Clang and GCC from instrumenting calls to memcpy() when building with KASAN, KCSAN or KMSAN: - KASAN normally replaces memcpy() with __asan_memcpy() with the additional cc-param,asan-kernel-mem-intrinsic-prefix=1; - KCSAN and KMSAN replace memcpy() with __tsan_memcpy() and __msan_memcpy() by default. To let the tools catch memory accesses from strlcpy/strlcat, replace the calls to memcpy() with __builtin_memcpy(), which KASAN, KCSAN and KMSAN are able to replace even in -ffreestanding mode. This preserves the behavior in normal builds (__builtin_memcpy() ends up being replaced with memcpy()), and does not introduce new instrumentation in unwanted places, as strlcpy/strlcat are already instrumented. Suggested-by: Marco Elver <[email protected]> Signed-off-by: Alexander Potapenko <[email protected]> Reviewed-by: Marco Elver <[email protected]> Link: https://lore.kernel.org/all/[email protected]/ Acked-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-01staging: most: Replace all non-returning strlcpy with strscpyAzeem Shaikh1-4/+4
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-01drm/i2c: tda998x: Replace all non-returning strlcpy with strscpyAzeem Shaikh1-1/+1
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-01drm/sun4i: hdmi: Replace all non-returning strlcpy with strscpyAzeem Shaikh1-1/+1
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-01drm/mediatek: Replace all non-returning strlcpy with strscpyAzeem Shaikh1-1/+1
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-01drm/rockchip: Replace all non-returning strlcpy with strscpyAzeem Shaikh2-2/+2
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-06-01drm/display/dp_mst: Replace all non-returning strlcpy with strscpyAzeem Shaikh3-3/+3
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-05-30ubsan: add prototypes for internal functionsArnd Bergmann2-3/+11
Most of the functions in ubsan that are only called from generated code don't have a prototype, which W=1 builds warn about: lib/ubsan.c:226:6: error: no previous prototype for '__ubsan_handle_divrem_overflow' [-Werror=missing-prototypes] lib/ubsan.c:307:6: error: no previous prototype for '__ubsan_handle_type_mismatch' [-Werror=missing-prototypes] lib/ubsan.c:321:6: error: no previous prototype for '__ubsan_handle_type_mismatch_v1' [-Werror=missing-prototypes] lib/ubsan.c:335:6: error: no previous prototype for '__ubsan_handle_out_of_bounds' [-Werror=missing-prototypes] lib/ubsan.c:352:6: error: no previous prototype for '__ubsan_handle_shift_out_of_bounds' [-Werror=missing-prototypes] lib/ubsan.c:394:6: error: no previous prototype for '__ubsan_handle_builtin_unreachable' [-Werror=missing-prototypes] lib/ubsan.c:404:6: error: no previous prototype for '__ubsan_handle_load_invalid_value' [-Werror=missing-prototypes] Add prototypes for all of these to lib/ubsan.h, and remove the one that was already present in ubsan.c. Signed-off-by: Arnd Bergmann <[email protected]> Reviewed-by: Fangrui Song <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-05-30checkpatch: Check for strcpy and strncpy tooKees Cook1-1/+13
Warn about strcpy(), strncpy(), and strlcpy(). Suggest strscpy() and include pointers to the open KSPP issues for each, which has further details and replacement procedures. Cc: Andy Whitcroft <[email protected]> Cc: Joe Perches <[email protected]> Cc: Dwaipayan Ray <[email protected]> Cc: Lukas Bulwahn <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-05-30ftrace: Replace all non-returning strlcpy with strscpyAzeem Shaikh1-9/+9
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Reviewed-by: Kees Cook <[email protected]> Acked-by: Masami Hiramatsu (Google) <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-05-30Compiler Attributes: Add __counted_by macroKees Cook1-0/+13
In an effort to annotate all flexible array members with their run-time size information, the "element_count" attribute is being introduced by Clang[1] and GCC[2] in future releases. This annotation will provide the CONFIG_UBSAN_BOUNDS and CONFIG_FORTIFY_SOURCE features the ability to perform run-time bounds checking on otherwise unknown-size flexible arrays. Even though the attribute is under development, we can start the annotation process in the kernel. This requires defining a macro for it, even if we have to change the name of the actual attribute later. Since it is likely that this attribute may change its name to "counted_by" in the future (to better align with a future total bytes "sized_by" attribute), name the wrapper macro "__counted_by", which also reads more clearly (and concisely) in structure definitions. [1] https://reviews.llvm.org/D148381 [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896 Cc: Bill Wendling <[email protected]> Cc: Qing Zhao <[email protected]> Cc: Gustavo A. R. Silva <[email protected]> Cc: Nick Desaulniers <[email protected]> Cc: Nathan Chancellor <[email protected]> Cc: Tom Rix <[email protected]> Cc: [email protected] Signed-off-by: Kees Cook <[email protected]> Reviewed-by: Gustavo A. R. Silva <[email protected]> Reviewed-by: Nathan Chancellor <[email protected]> Acked-by: Miguel Ojeda <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-05-30autofs: use flexible array in ioctl structureArnd Bergmann3-3/+3
Commit df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") introduced a warning for the autofs_dev_ioctl structure: In function 'check_name', inlined from 'validate_dev_ioctl' at fs/autofs/dev-ioctl.c:131:9, inlined from '_autofs_dev_ioctl' at fs/autofs/dev-ioctl.c:624:8: fs/autofs/dev-ioctl.c:33:14: error: 'strchr' reading 1 or more bytes from a region of size 0 [-Werror=stringop-overread] 33 | if (!strchr(name, '/')) | ^~~~~~~~~~~~~~~~~ In file included from include/linux/auto_dev-ioctl.h:10, from fs/autofs/autofs_i.h:10, from fs/autofs/dev-ioctl.c:14: include/uapi/linux/auto_dev-ioctl.h: In function '_autofs_dev_ioctl': include/uapi/linux/auto_dev-ioctl.h:112:14: note: source object 'path' of size 0 112 | char path[0]; | ^~~~ This is easily fixed by changing the gnu 0-length array into a c99 flexible array. Since this is a uapi structure, we have to be careful about possible regressions but this one should be fine as they are equivalent here. While it would break building with ancient gcc versions that predate c99, it helps building with --std=c99 and -Wpedantic builds in user space, as well as non-gnu compilers. This means we probably also want it fixed in stable kernels. Cc: [email protected] Cc: Kees Cook <[email protected]> Cc: Gustavo A. R. Silva" <[email protected]> Signed-off-by: Arnd Bergmann <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-05-30lkdtm/bugs: Switch from 1-element array to flexible arrayKees Cook1-2/+2
The testing for ARRAY_BOUNDS just wants an uninstrumented array, and the proper flexible array definition is fine for that. Cc: Arnd Bergmann <[email protected]> Cc: Greg Kroah-Hartman <[email protected]> Reviewed-by: Bill Wendling <[email protected]> Signed-off-by: Kees Cook <[email protected]>
2023-05-30befs: Replace all non-returning strlcpy with strscpyAzeem Shaikh2-2/+2
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated. In an effort to remove strlcpy() completely, replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-05-30md/raid5: Convert stripe_head's "dev" to flexible array memberKees Cook2-3/+3
Replace old-style 1-element array of "dev" in struct stripe_head with modern C99 flexible array. In the future, we can additionally annotate it with the run-time size, found in the "disks" member. Cc: Song Liu <[email protected]> Cc: [email protected] Reviewed-by: Christoph Hellwig <[email protected]> Acked-by: Song Liu <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/lkml/[email protected]/ --- It looks like this memory calculation: memory = conf->min_nr_stripes * (sizeof(struct stripe_head) + max_disks * ((sizeof(struct bio) + PAGE_SIZE))) / 1024; ... was already buggy (i.e. it included the single "dev" bytes in the result). However, I'm not entirely sure if that is the right analysis, since "dev" is not related to struct bio nor PAGE_SIZE?
2023-05-26overflow: Add struct_size_t() helperKees Cook10-25/+40
While struct_size() is normally used in situations where the structure type already has a pointer instance, there are places where no variable is available. In the past, this has been worked around by using a typed NULL first argument, but this is a bit ugly. Add a helper to do this, and replace the handful of instances of the code pattern with it. Instances were found with this Coccinelle script: @struct_size_t@ identifier STRUCT, MEMBER; expression COUNT; @@ - struct_size((struct STRUCT *)\(0\|NULL\), + struct_size_t(struct STRUCT, MEMBER, COUNT) Suggested-by: Christoph Hellwig <[email protected]> Cc: Jesse Brandeburg <[email protected]> Cc: Tony Nguyen <[email protected]> Cc: "David S. Miller" <[email protected]> Cc: Eric Dumazet <[email protected]> Cc: Paolo Abeni <[email protected]> Cc: James Smart <[email protected]> Cc: Keith Busch <[email protected]> Cc: Jens Axboe <[email protected]> Cc: Sagi Grimberg <[email protected]> Cc: HighPoint Linux Team <[email protected]> Cc: "James E.J. Bottomley" <[email protected]> Cc: "Martin K. Petersen" <[email protected]> Cc: Kashyap Desai <[email protected]> Cc: Sumit Saxena <[email protected]> Cc: Shivasharan S <[email protected]> Cc: Don Brace <[email protected]> Cc: "Darrick J. Wong" <[email protected]> Cc: Dave Chinner <[email protected]> Cc: Guo Xuenan <[email protected]> Cc: Gwan-gyeong Mun <[email protected]> Cc: Nick Desaulniers <[email protected]> Cc: Daniel Latypov <[email protected]> Cc: kernel test robot <[email protected]> Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Signed-off-by: Kees Cook <[email protected]> Acked-by: Martin K. Petersen <[email protected]> Reviewed-by: Darrick J. Wong <[email protected]> Reviewed-by: Gustavo A. R. Silva <[email protected]> Reviewed-by: Christoph Hellwig <[email protected]> Acked-by: Jakub Kicinski <[email protected]> Reviewed-by: Alexander Lobakin <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-05-26drm/amd/pm: Replace all non-returning strlcpy with strscpyAzeem Shaikh1-1/+1
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Acked-by: Alex Deucher <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-05-26drm/radeon: Replace all non-returning strlcpy with strscpyAzeem Shaikh3-5/+5
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Acked-by: Alex Deucher <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-05-26tracing: Replace all non-returning strlcpy with strscpyAzeem Shaikh5-10/+10
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement with strlcpy is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Acked-by: Masami Hiramatsu (Google) <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-05-26scsi: 3w-9xxx: Replace all non-returning strlcpy with strscpyAzeem Shaikh1-1/+1
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-05-26scsi: aacraid: Replace all non-returning strlcpy with strscpyAzeem Shaikh1-1/+1
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-05-26scsi: bnx2i: Replace all non-returning strlcpy with strscpyAzeem Shaikh1-1/+1
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-05-26scsi: qedi: Replace all non-returning strlcpy with strscpyAzeem Shaikh1-1/+1
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-05-26scsi: ibmvscsi: Replace all non-returning strlcpy with strscpyAzeem Shaikh1-3/+3
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Acked-by: Tyrel Datwyler <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-05-22vboxsf: Replace all non-returning strlcpy with strscpyAzeem Shaikh1-1/+1
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Reviewed-by: Hans de Goede <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-05-22NFS: Prefer strscpy over strlcpy callsAzeem Shaikh1-1/+1
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. Check for strscpy()'s return value of -E2BIG on truncate for safe replacement with strlcpy(). This is part of a tree-wide cleanup to remove the strlcpy() function entirely from the kernel [2]. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-05-22dlm: Replace all non-returning strlcpy with strscpyAzeem Shaikh1-2/+2
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh <[email protected]> Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-05-22kbuild: Enable -fstrict-flex-arrays=3Kees Cook1-0/+6
The -fstrict-flex-arrays=3 option is now available with the release of GCC 13[1] and Clang 16[2]. This feature instructs the compiler to treat only C99 flexible arrays as dynamically sized for the purposes of object size calculations. In other words, the ancient practice of using 1-element arrays, or the GNU extension of using 0-sized arrays, as a dynamically sized array is disabled. This allows CONFIG_UBSAN_BOUNDS, CONFIG_FORTIFY_SOURCE, and other object-size aware features to behave unambiguously in the face of trailing arrays: only C99 flexible arrays are considered to be dynamically sized. For yet more detail, see: https://people.kernel.org/kees/bounded-flexible-arrays-in-c Enabling this will help track down any outstanding cases of fake flexible arrays that need attention in kernel code. [1] https://gcc.gnu.org/onlinedocs/gcc/C-Dialect-Options.html#index-fstrict-flex-arrays [2] https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fstrict-flex-arrays Cc: Masahiro Yamada <[email protected]> Cc: Nathan Chancellor <[email protected]> Cc: Nick Desaulniers <[email protected]> Cc: Nicolas Schier <[email protected]> Cc: [email protected] Co-developed-by: "Gustavo A. R. Silva" <[email protected]> Signed-off-by: "Gustavo A. R. Silva" <[email protected]> Signed-off-by: Kees Cook <[email protected]>
2023-05-17ubsan: remove cc-option test for UBSAN_TRAPNick Desaulniers1-1/+0
-fsanitize-undefined-trap-on-error has been supported since GCC 5.1 and Clang 3.2. The minimum supported version of these according to Documentation/process/changes.rst is 5.1 and 11.0.0 respectively. Drop this cc-option check. Signed-off-by: Nick Desaulniers <[email protected]> Reviewed-by: Nathan Chancellor <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
2023-05-16fortify: strcat: Move definition to use fortified strlcat()Kees Cook1-27/+26
Move the definition of fortified strcat() to after strlcat() to use it for bounds checking. Signed-off-by: Kees Cook <[email protected]>
2023-05-16fortify: Add protection for strlcat()Kees Cook1-0/+64
The definition of strcat() was defined in terms of unfortified strlcat(), but that meant there was no bounds checking done on the internal strlen() calls, and the (bounded) copy would be performed before reporting a failure. Additionally, pathological cases (i.e. unterminated destination buffer) did not make calls to fortify_panic(), which will make future unit testing more difficult. Instead, explicitly define a fortified strlcat() wrapper for strcat() to use. Signed-off-by: Kees Cook <[email protected]>