Skip to content

L
linux
Switch branch/tag
  1. 26 Sep, 2022 4 commits
  3. 22 Sep, 2022 1 commit
    • Kees Cook's avatar
      ARM: decompressor: Include .data.rel.ro.local · 1b64daf4
      Kees Cook authored 
      The .data.rel.ro.local section has the same semantics as .data.rel.ro
here, so include it in the .rodata section of the decompressor.
Additionally since the .printk_index section isn't usable outside of
the core kernel, discard it in the decompressor. Avoids these warnings:

arm-linux-gnueabi-ld: warning: orphan section `.data.rel.ro.local' from `arch/arm/boot/compressed/fdt_rw.o' being placed in section `.data.rel.ro.local'
arm-linux-gnueabi-ld: warning: orphan section `.printk_index' from `arch/arm/boot/compressed/fdt_rw.o' being placed in section `.printk_index'
Reported-by: default avatarkernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/linux-mm/202209080545.qMIVj7YM-lkp@intel.com
Cc: Russell King <linux@armlinux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      1b64daf4
  5. 14 Sep, 2022 1 commit
  7. 13 Sep, 2022 1 commit
  9. 07 Sep, 2022 14 commits
    • Kees Cook's avatar
      kunit/memcpy: Avoid pathological compile-time string size · 66cb2a36
      Kees Cook authored 
      The memcpy() KUnit tests are trying to sanity-check run-time behaviors,
but tripped compile-time warnings about a pathological condition of a
too-small buffer being used for input. Avoid this by explicitly resizing
the buffer, but leaving the string short. Avoid the following warning:

lib/memcpy_kunit.c: In function 'strtomem_test':
include/linux/string.h:303:42: warning: 'strnlen' specified bound 4 exceeds source size 3 [-Wstringop-overread]
  303 |         memcpy(dest, src, min(_dest_len, strnlen(src, _dest_len)));     \
include/linux/minmax.h:32:39: note: in definition of macro '__cmp_once'
   32 |                 typeof(y) unique_y = (y);               \
      |                                       ^
include/linux/minmax.h:45:25: note: in expansion of macro '__careful_cmp'
   45 | #define min(x, y)       __careful_cmp(x, y, <)
      |                         ^~~~~~~~~~~~~
include/linux/string.h:303:27: note: in expansion of macro 'min'
  303 |         memcpy(dest, src, min(_dest_len, strnlen(src, _dest_len)));     \
      |                           ^~~
lib/memcpy_kunit.c:290:9: note: in expansion of macro 'strtomem'
  290 |         strtomem(wrap.output, input);
      |         ^~~~~~~~
lib/memcpy_kunit.c:275:27: note: source object allocated here
  275 |         static const char input[] = "hi";
      |                           ^~~~~
Reported-by: default avatarkernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/linux-mm/202209070728.o3stvgVt-lkp@intel.com
Fixes: dfbafa70 ("string: Introduce strtomem() and strtomem_pad()")
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      66cb2a36
    • Bart Van Assche's avatar
      lib: Improve the is_signed_type() kunit test · 98388bda
      Bart Van Assche authored 
      Since the definition of is_signed_type() has been moved from
<linux/overflow.h> to <linux/compiler.h>, include the latter header file
instead of the former. Additionally, add a test for the type 'char'.

Cc: Isabella Basso <isabbasso@riseup.net>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: default avatarBart Van Assche <bvanassche@acm.org>
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/r/20220907180329.3825417-1-bvanassche@acm.org
      98388bda
    • Matthias Kaehlcke's avatar
      LoadPin: Require file with verity root digests to have a header · 6e42aec7
      Matthias Kaehlcke authored 
      LoadPin expects the file with trusted verity root digests to be
an ASCII file with one digest (hex value) per line. A pinned
root could contain files that meet these format requirements,
even though the hex values don't represent trusted root
digests.

Add a new requirement to the file format which consists in
the first line containing a fixed string. This prevents
attackers from feeding files with an otherwise valid format
to LoadPin.
Suggested-by: default avatarSarthak Kukreti <sarthakkukreti@chromium.org>
Signed-off-by: default avatarMatthias Kaehlcke <mka@chromium.org>
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/r/20220906181725.1.I3f51d1bb0014e5a5951be4ad3c5ad7c7ca1dfc32@changeid
      6e42aec7
    • Matthias Kaehlcke's avatar
      dm: verity-loadpin: Only trust verity targets with enforcement · 916ef623
      Matthias Kaehlcke authored 
      Verity targets can be configured to ignore corrupted data blocks.
LoadPin must only trust verity targets that are configured to
perform some kind of enforcement when data corruption is detected,
like returning an error, restarting the system or triggering a
panic.

Fixes: b6c1c574 ("dm: Add verity helpers for LoadPin")
Reported-by: default avatarSarthak Kukreti <sarthakkukreti@chromium.org>
Signed-off-by: default avatarMatthias Kaehlcke <mka@chromium.org>
Reviewed-by: default avatarSarthak Kukreti <sarthakkukreti@chromium.org>
Cc: stable@vger.kernel.org
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/r/20220907133055.1.Ic8a1dafe960dc0f8302e189642bc88ebb785d274@changeid
      916ef623
    • Matthias Kaehlcke's avatar
      LoadPin: Fix Kconfig doc about format of file with verity digests · aafc203b
      Matthias Kaehlcke authored 
      The doc for CONFIG_SECURITY_LOADPIN_VERITY says that the file with verity
digests must contain a comma separated list of digests. That was the case
at some stage of the development, but was changed during the review
process to one digest per line. Update the Kconfig doc accordingly.
Reported-by: default avatarJae Hoon Kim <kimjae@chromium.org>
Signed-off-by: default avatarMatthias Kaehlcke <mka@chromium.org>
Fixes: 3f805f8c ("LoadPin: Enable loading from trusted dm-verity devices")
Cc: stable@vger.kernel.org
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/r/20220829174557.1.I5d202d1344212a3800d9828f936df6511eb2d0d1@changeid
      aafc203b
    • Kees Cook's avatar
      um: Enable FORTIFY_SOURCE · ba38961a
      Kees Cook authored 
      Enable FORTIFY_SOURCE so running Kunit tests can test fortified
functions.
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
Tested-by: default avatarDavid Gow <davidgow@google.com>
Link: https://lore.kernel.org/r/20220210003224.773957-1-keescook@chromium.org
      ba38961a
    • Kees Cook's avatar
      lkdtm: Update tests for memcpy() run-time warnings · 325bf6d8
      Kees Cook authored 
      Clarify the LKDTM FORTIFY tests, and add tests for the mem*() family of
functions, now that run-time checking is distinct.

Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Shuah Khan <shuah@kernel.org>
Cc: linux-kselftest@vger.kernel.org
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      325bf6d8
    • Kees Cook's avatar
      fortify: Add run-time WARN for cross-field memcpy() · 54d9469b
      Kees Cook authored 
      Enable run-time checking of dynamic memcpy() and memmove() lengths,
issuing a WARN when a write would exceed the size of the target struct
member, when built with CONFIG_FORTIFY_SOURCE=y. This would have
caught all of the memcpy()-based buffer overflows in the last 3 years,
specifically covering all the cases where the destination buffer size
is known at compile time.

This change ONLY adds a run-time warning. As false positives are currently
still expected, this will not block the overflow. The new warnings will
look like this:

  memcpy: detected field-spanning write (size N) of single field "var->dest" (size M)
  WARNING: CPU: n PID: pppp at source/file/path.c:nr function+0xXX/0xXX [module]

There may be false positives in the kernel where intentional
field-spanning writes are happening. These need to be addressed
similarly to how the compile-time cases were addressed: add a
struct_group(), split the memcpy(), or some other refactoring.

In order to make counting/investigating instances of added runtime checks
easier, each instance includes the destination variable name as a WARN
argument, prefixed with 'field "'. Therefore, on an x86_64 defconfig
build, it is trivial to inspect the build artifacts to find instances.
For example on an x86_64 defconfig build, there are 78 new run-time
memcpy() bounds checks added:

  $ for i in vmlinux $(find . -name '*.ko'); do \
      strings "$i" | grep '^field "'; done | wc -l
  78

Simple cases where a destination buffer is known to be a dynamic size
do not generate a WARN. For example:

struct normal_flex_array {
	void *a;
	int b;
	u32 c;
	size_t array_size;
	u8 flex_array[];
};

struct normal_flex_array *instance;
...
/* These will be ignored for run-time bounds checking. */
memcpy(instance, src, len);
memcpy(instance->flex_array, src, len);

However, one of the dynamic-sized destination cases is irritatingly
unable to be detected by the compiler: when using memcpy() to target
a composite struct member which contains a trailing flexible array
struct. For example:

struct wrapper {
	int foo;
	char bar;
	struct normal_flex_array embedded;
};

struct wrapper *instance;
...
/* This will incorrectly WARN when len > sizeof(instance->embedded) */
memcpy(&instance->embedded, src, len);

These cases end up appearing to the compiler to be sized as if the
flexible array had 0 elements. :( For more details see:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101832
https://godbolt.org/z/vW6x8vh4P

These "composite flexible array structure destination" cases will be
need to be flushed out and addressed on a case-by-case basis.

Regardless, for the general case of using memcpy() on flexible array
destinations, future APIs will be created to handle common cases. Those
can be used to migrate away from open-coded memcpy() so that proper
error handling (instead of trapping) can be used.

As mentioned, none of these bounds checks block any overflows
currently. For users that have tested their workloads, do not encounter
any warnings, and wish to make these checks stop any overflows, they
can use a big hammer and set the sysctl panic_on_warn=1.
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      54d9469b
    • Kees Cook's avatar
      fortify: Use SIZE_MAX instead of (size_t)-1 · 311fb40a
      Kees Cook authored 
      Clean up uses of "(size_t)-1" in favor of SIZE_MAX.

Cc: linux-hardening@vger.kernel.org
Suggested-by: default avatarNick Desaulniers <ndesaulniers@google.com>
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      311fb40a
    • Kees Cook's avatar
      fortify: Add KUnit test for FORTIFY_SOURCE internals · 875bfd52
      Kees Cook authored 
      Add lib/fortify_kunit.c KUnit test for checking the expected behavioral
characteristics of FORTIFY_SOURCE internals.

Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Tom Rix <trix@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: "Steven Rostedt (Google)" <rostedt@goodmis.org>
Cc: Yury Norov <yury.norov@gmail.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Sander Vanheule <sander@svanheule.net>
Cc: linux-hardening@vger.kernel.org
Cc: llvm@lists.linux.dev
Reviewed-by: default avatarDavid Gow <davidgow@google.com>
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      875bfd52
    • Kees Cook's avatar
      fortify: Fix __compiletime_strlen() under UBSAN_BOUNDS_LOCAL · d07c0acb
      Kees Cook authored 
      With CONFIG_FORTIFY=y and CONFIG_UBSAN_LOCAL_BOUNDS=y enabled, we observe
a runtime panic while running Android's Compatibility Test Suite's (CTS)
android.hardware.input.cts.tests. This is stemming from a strlen()
call in hidinput_allocate().

__compiletime_strlen() is implemented in terms of __builtin_object_size(),
then does an array access to check for NUL-termination. A quirk of
__builtin_object_size() is that for strings whose values are runtime
dependent, __builtin_object_size(str, 1 or 0) returns the maximum size
of possible values when those sizes are determinable at compile time.
Example:

  static const char *v = "FOO BAR";
  static const char *y = "FOO BA";
  unsigned long x (int z) {
      // Returns 8, which is:
      // max(__builtin_object_size(v, 1), __builtin_object_size(y, 1))
      return __builtin_object_size(z ? v : y, 1);
  }

So when FORTIFY_SOURCE is enabled, the current implementation of
__compiletime_strlen() will try to access beyond the end of y at runtime
using the size of v. Mixed with UBSAN_LOCAL_BOUNDS we get a fault.

hidinput_allocate() has a local C string whose value is control flow
dependent on a switch statement, so __builtin_object_size(str, 1)
evaluates to the maximum string length, making all other cases fault on
the last character check. hidinput_allocate() could be cleaned up to
avoid runtime calls to strlen() since the local variable can only have
literal values, so there's no benefit to trying to fortify the strlen
call site there.

Perform a __builtin_constant_p() check against index 0 earlier in the
macro to filter out the control-flow-dependant case. Add a KUnit test
for checking the expected behavioral characteristics of FORTIFY_SOURCE
internals.

Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Tom Rix <trix@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: "Steven Rostedt (Google)" <rostedt@goodmis.org>
Cc: David Gow <davidgow@google.com>
Cc: Yury Norov <yury.norov@gmail.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Sander Vanheule <sander@svanheule.net>
Cc: linux-hardening@vger.kernel.org
Cc: llvm@lists.linux.dev
Reviewed-by: default avatarNick Desaulniers <ndesaulniers@google.com>
Tested-by: Android Treehugger Robot
Link: https://android-review.googlesource.com/c/kernel/common/+/2206839Co-developed-by: default avatarNick Desaulniers <ndesaulniers@google.com>
Signed-off-by: default avatarNick Desaulniers <ndesaulniers@google.com>
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      d07c0acb
    • Kees Cook's avatar
      string: Introduce strtomem() and strtomem_pad() · dfbafa70
      Kees Cook authored 
      One of the "legitimate" uses of strncpy() is copying a NUL-terminated
string into a fixed-size non-NUL-terminated character array. To avoid
the weaknesses and ambiguity of intent when using strncpy(), provide
replacement functions that explicitly distinguish between trailing
padding and not, and require the destination buffer size be discoverable
by the compiler.

For example:

struct obj {
	int foo;
	char small[4] __nonstring;
	char big[8] __nonstring;
	int bar;
};

struct obj p;

/* This will truncate to 4 chars with no trailing NUL */
strncpy(p.small, "hello", sizeof(p.small));
/* p.small contains 'h', 'e', 'l', 'l' */

/* This will NUL pad to 8 chars. */
strncpy(p.big, "hello", sizeof(p.big));
/* p.big contains 'h', 'e', 'l', 'l', 'o', '\0', '\0', '\0' */

When the "__nonstring" attributes are missing, the intent of the
programmer becomes ambiguous for whether the lack of a trailing NUL
in the p.small copy is a bug. Additionally, it's not clear whether
the trailing padding in the p.big copy is _needed_. Both cases
become unambiguous with:

strtomem(p.small, "hello");
strtomem_pad(p.big, "hello", 0);

See also https://github.com/KSPP/linux/issues/90

Expand the memcpy KUnit tests to include these functions.

Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      dfbafa70
    • Kees Cook's avatar
      overflow: Split up kunit tests for smaller stack frames · 77974225
      Kees Cook authored 
      Under some pathological 32-bit configs, the shift overflow KUnit tests
create huge stack frames. Split up the function to avoid this,
separating by rough shift overflow cases.

Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Daniel Latypov <dlatypov@google.com>
Cc: Vitor Massaru Iha <vitor@massaru.org>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Reported-by: default avatarkernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/lkml/202208301850.iuv9VwA8-lkp@intel.comAcked-by: default avatarDaniel Latypov <dlatypov@google.com>
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      77974225
    • Kees Cook's avatar
      overflow: Allow mixed type arguments · d219d2a9
      Kees Cook authored 
      When the check_[op]_overflow() helpers were introduced, all arguments
were required to be the same type to make the fallback macros simpler.
However, now that the fallback macros have been removed[1], it is fine
to allow mixed types, which makes using the helpers much more useful,
as they can be used to test for type-based overflows (e.g. adding two
large ints but storing into a u8), as would be handy in the drm core[2].

Remove the restriction, and add additional self-tests that exercise
some of the mixed-type overflow cases, and double-check for accidental
macro side-effects.

[1] https://git.kernel.org/linus/4eb6bd55cfb22ffc20652732340c4962f3ac9a91
[2] https://lore.kernel.org/lkml/20220824084514.2261614-2-gwan-gyeong.mun@intel.com

Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: linux-hardening@vger.kernel.org
Reviewed-by: default avatarAndrzej Hajda <andrzej.hajda@intel.com>
Reviewed-by: default avatarGwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Tested-by: default avatarGwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      d219d2a9
  11. 31 Aug, 2022 2 commits
  13. 22 Aug, 2022 1 commit
  15. 21 Aug, 2022 16 commits
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7