1. 30 Sep, 2022 2 commits
    • Kees Cook's avatar
      hardening: Remove Clang's enable flag for -ftrivial-auto-var-init=zero · 607e57c6
      Kees Cook authored
      Now that Clang's -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang
      option is no longer required, remove it from the command line. Clang 16
      and later will warn when it is used, which will cause Kconfig to think
      it can't use -ftrivial-auto-var-init=zero at all. Check for whether it
      is required and only use it when so.
      
      Cc: Nathan Chancellor <nathan@kernel.org>
      Cc: Masahiro Yamada <masahiroy@kernel.org>
      Cc: Nick Desaulniers <ndesaulniers@google.com>
      Cc: linux-kbuild@vger.kernel.org
      Cc: llvm@lists.linux.dev
      Cc: stable@vger.kernel.org
      Fixes: f02003c8 ("hardening: Avoid harmless Clang option under CONFIG_INIT_STACK_ALL_ZERO")
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      607e57c6
    • Bart Van Assche's avatar
      sparc: Unbreak the build · 17006e86
      Bart Van Assche authored
      Fix the following build errors:
      
      arch/sparc/mm/srmmu.c: In function ‘smp_flush_page_for_dma’:
      arch/sparc/mm/srmmu.c:1639:13: error: cast between incompatible function types from ‘void (*)(long unsigned int)’ to ‘void (*)(long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int)’ [-Werror=cast-function-type]
       1639 |         xc1((smpfunc_t) local_ops->page_for_dma, page);
            |             ^
      arch/sparc/mm/srmmu.c: In function ‘smp_flush_cache_mm’:
      arch/sparc/mm/srmmu.c:1662:29: error: cast between incompatible function types from ‘void (*)(struct mm_struct *)’ to ‘void (*)(long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int)’ [-Werror=cast-function-type]
       1662 |                         xc1((smpfunc_t) local_ops->cache_mm, (unsigned long) mm);
            |
      [ ... ]
      
      Compile-tested only.
      
      Fixes: 552a23a0 ("Makefile: Enable -Wcast-function-type")
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarBart Van Assche <bvanassche@acm.org>
      Tested-by: default avatarAndreas Larsson <andreas@gaisler.com>
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      Link: https://lore.kernel.org/r/20220830205854.1918026-1-bvanassche@acm.org
      17006e86
  2. 26 Sep, 2022 5 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
  4. 14 Sep, 2022 1 commit
  5. 13 Sep, 2022 1 commit
  6. 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
  7. 31 Aug, 2022 2 commits
  8. 22 Aug, 2022 1 commit
  9. 21 Aug, 2022 13 commits
    • Linus Torvalds's avatar
      Merge tag 'irq-urgent-2022-08-21' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip · 4daa6a81
      Linus Torvalds authored
      Pull irq fixes from Ingo Molnar:
       "Misc irqchip fixes: LoongArch driver fixes and a Hyper-V IOMMU fix"
      
      * tag 'irq-urgent-2022-08-21' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip:
        irqchip/loongson-liointc: Fix an error handling path in liointc_init()
        irqchip/loongarch: Fix irq_domain_alloc_fwnode() abuse
        irqchip/loongson-pch-pic: Move find_pch_pic() into CONFIG_ACPI
        irqchip/loongson-eiointc: Fix a build warning
        irqchip/loongson-eiointc: Fix irq affinity setting
        iommu/hyper-v: Use helper instead of directly accessing affinity
      4daa6a81
    • Linus Torvalds's avatar
      Merge tag 'perf-urgent-2022-08-21' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip · 4f61f842
      Linus Torvalds authored
      Pull x86 kprobes fix from Ingo Molnar:
       "Fix a kprobes bug in JNG/JNLE emulation when a kprobe is installed at
        such instructions, possibly resulting in incorrect execution (the
        wrong branch taken)"
      
      * tag 'perf-urgent-2022-08-21' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip:
        x86/kprobes: Fix JNG/JNLE emulation
      4f61f842
    • Linus Torvalds's avatar
      Merge tag 'trace-v6.0-rc1-2' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace · 7fb312d2
      Linus Torvalds authored
      Pull tracing fixes from Steven Rostedt:
       "Various fixes for tracing:
      
         - Fix a return value of traceprobe_parse_event_name()
      
         - Fix NULL pointer dereference from failed ftrace enabling
      
         - Fix NULL pointer dereference when asking for registers from eprobes
      
         - Make eprobes consistent with kprobes/uprobes, filters and
           histograms"
      
      * tag 'trace-v6.0-rc1-2' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace:
        tracing: Have filter accept "common_cpu" to be consistent
        tracing/probes: Have kprobes and uprobes use $COMM too
        tracing/eprobes: Have event probes be consistent with kprobes and uprobes
        tracing/eprobes: Fix reading of string fields
        tracing/eprobes: Do not hardcode $comm as a string
        tracing/eprobes: Do not allow eprobes to use $stack, or % for regs
        ftrace: Fix NULL pointer dereference in is_ftrace_trampoline when ftrace is dead
        tracing/perf: Fix double put of trace event when init fails
        tracing: React to error return from traceprobe_parse_event_name()
      7fb312d2
    • Steven Rostedt (Google)'s avatar
      tracing: Have filter accept "common_cpu" to be consistent · b2380577
      Steven Rostedt (Google) authored
      Make filtering consistent with histograms. As "cpu" can be a field of an
      event, allow for "common_cpu" to keep it from being confused with the
      "cpu" field of the event.
      
      Link: https://lkml.kernel.org/r/20220820134401.513062765@goodmis.org
      Link: https://lore.kernel.org/all/20220820220920.e42fa32b70505b1904f0a0ad@kernel.org/
      
      Cc: stable@vger.kernel.org
      Cc: Ingo Molnar <mingo@kernel.org>
      Cc: Andrew Morton <akpm@linux-foundation.org>
      Cc: Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
      Cc: Tom Zanussi <zanussi@kernel.org>
      Fixes: 1e3bac71 ("tracing/histogram: Rename "cpu" to "common_cpu"")
      Suggested-by: default avatarMasami Hiramatsu (Google) <mhiramat@kernel.org>
      Acked-by: default avatarMasami Hiramatsu (Google) <mhiramat@kernel.org>
      Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
      b2380577
    • Steven Rostedt (Google)'s avatar
      tracing/probes: Have kprobes and uprobes use $COMM too · ab838444
      Steven Rostedt (Google) authored
      Both $comm and $COMM can be used to get current->comm in eprobes and the
      filtering and histogram logic. Make kprobes and uprobes consistent in this
      regard and allow both $comm and $COMM as well. Currently kprobes and
      uprobes only handle $comm, which is inconsistent with the other utilities,
      and can be confusing to users.
      
      Link: https://lkml.kernel.org/r/20220820134401.317014913@goodmis.org
      Link: https://lore.kernel.org/all/20220820220442.776e1ddaf8836e82edb34d01@kernel.org/
      
      Cc: stable@vger.kernel.org
      Cc: Ingo Molnar <mingo@kernel.org>
      Cc: Andrew Morton <akpm@linux-foundation.org>
      Cc: Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
      Cc: Tom Zanussi <zanussi@kernel.org>
      Fixes: 53305928 ("tracing: probeevent: Introduce new argument fetching code")
      Suggested-by: default avatarMasami Hiramatsu (Google) <mhiramat@kernel.org>
      Acked-by: default avatarMasami Hiramatsu (Google) <mhiramat@kernel.org>
      Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
      ab838444
    • Steven Rostedt (Google)'s avatar
      tracing/eprobes: Have event probes be consistent with kprobes and uprobes · 6a832ec3
      Steven Rostedt (Google) authored
      Currently, if a symbol "@" is attempted to be used with an event probe
      (eprobes), it will cause a NULL pointer dereference crash.
      
      Both kprobes and uprobes can reference data other than the main registers.
      Such as immediate address, symbols and the current task name. Have eprobes
      do the same thing.
      
      For "comm", if "comm" is used and the event being attached to does not
      have the "comm" field, then make it the "$comm" that kprobes has. This is
      consistent to the way histograms and filters work.
      
      Link: https://lkml.kernel.org/r/20220820134401.136924220@goodmis.org
      
      Cc: stable@vger.kernel.org
      Cc: Ingo Molnar <mingo@kernel.org>
      Cc: Andrew Morton <akpm@linux-foundation.org>
      Cc: Masami Hiramatsu <mhiramat@kernel.org>
      Cc: Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
      Cc: Tom Zanussi <zanussi@kernel.org>
      Fixes: 7491e2c4 ("tracing: Add a probe that attaches to trace events")
      Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
      6a832ec3
    • Steven Rostedt (Google)'s avatar
      tracing/eprobes: Fix reading of string fields · f04dec93
      Steven Rostedt (Google) authored
      Currently when an event probe (eprobe) hooks to a string field, it does
      not display it as a string, but instead as a number. This makes the field
      rather useless. Handle the different kinds of strings, dynamic, static,
      relational/dynamic etc.
      
      Now when a string field is used, the ":string" type can be used to display
      it:
      
        echo "e:sw sched/sched_switch comm=$next_comm:string" > dynamic_events
      
      Link: https://lkml.kernel.org/r/20220820134400.959640191@goodmis.org
      
      Cc: stable@vger.kernel.org
      Cc: Ingo Molnar <mingo@kernel.org>
      Cc: Andrew Morton <akpm@linux-foundation.org>
      Cc: Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
      Cc: Tom Zanussi <zanussi@kernel.org>
      Fixes: 7491e2c4 ("tracing: Add a probe that attaches to trace events")
      Acked-by: default avatarMasami Hiramatsu (Google) <mhiramat@kernel.org>
      Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
      f04dec93
    • Steven Rostedt (Google)'s avatar
      tracing/eprobes: Do not hardcode $comm as a string · 02333de9
      Steven Rostedt (Google) authored
      The variable $comm is hard coded as a string, which is true for both
      kprobes and uprobes, but for event probes (eprobes) it is a field name. In
      most cases the "comm" field would be a string, but there's no guarantee of
      that fact.
      
      Do not assume that comm is a string. Not to mention, it currently forces
      comm fields to fault, as string processing for event probes is currently
      broken.
      
      Link: https://lkml.kernel.org/r/20220820134400.756152112@goodmis.org
      
      Cc: stable@vger.kernel.org
      Cc: Ingo Molnar <mingo@kernel.org>
      Cc: Andrew Morton <akpm@linux-foundation.org>
      Cc: Masami Hiramatsu <mhiramat@kernel.org>
      Cc: Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
      Cc: Tom Zanussi <zanussi@kernel.org>
      Fixes: 7491e2c4 ("tracing: Add a probe that attaches to trace events")
      Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
      02333de9
    • Steven Rostedt (Google)'s avatar
      tracing/eprobes: Do not allow eprobes to use $stack, or % for regs · 2673c60e
      Steven Rostedt (Google) authored
      While playing with event probes (eprobes), I tried to see what would
      happen if I attempted to retrieve the instruction pointer (%rip) knowing
      that event probes do not use pt_regs. The result was:
      
       BUG: kernel NULL pointer dereference, address: 0000000000000024
       #PF: supervisor read access in kernel mode
       #PF: error_code(0x0000) - not-present page
       PGD 0 P4D 0
       Oops: 0000 [#1] PREEMPT SMP PTI
       CPU: 1 PID: 1847 Comm: trace-cmd Not tainted 5.19.0-rc5-test+ #309
       Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01
      v03.03 07/14/2016
       RIP: 0010:get_event_field.isra.0+0x0/0x50
       Code: ff 48 c7 c7 c0 8f 74 a1 e8 3d 8b f5 ff e8 88 09 f6 ff 4c 89 e7 e8
      50 6a 13 00 48 89 ef 5b 5d 41 5c 41 5d e9 42 6a 13 00 66 90 <48> 63 47 24
      8b 57 2c 48 01 c6 8b 47 28 83 f8 02 74 0e 83 f8 04 74
       RSP: 0018:ffff916c394bbaf0 EFLAGS: 00010086
       RAX: ffff916c854041d8 RBX: ffff916c8d9fbf50 RCX: ffff916c255d2000
       RDX: 0000000000000000 RSI: ffff916c255d2008 RDI: 0000000000000000
       RBP: 0000000000000000 R08: ffff916c3a2a0c08 R09: ffff916c394bbda8
       R10: 0000000000000000 R11: 0000000000000000 R12: ffff916c854041d8
       R13: ffff916c854041b0 R14: 0000000000000000 R15: 0000000000000000
       FS:  0000000000000000(0000) GS:ffff916c9ea40000(0000)
      knlGS:0000000000000000
       CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
       CR2: 0000000000000024 CR3: 000000011b60a002 CR4: 00000000001706e0
       Call Trace:
        <TASK>
        get_eprobe_size+0xb4/0x640
        ? __mod_node_page_state+0x72/0xc0
        __eprobe_trace_func+0x59/0x1a0
        ? __mod_lruvec_page_state+0xaa/0x1b0
        ? page_remove_file_rmap+0x14/0x230
        ? page_remove_rmap+0xda/0x170
        event_triggers_call+0x52/0xe0
        trace_event_buffer_commit+0x18f/0x240
        trace_event_raw_event_sched_wakeup_template+0x7a/0xb0
        try_to_wake_up+0x260/0x4c0
        __wake_up_common+0x80/0x180
        __wake_up_common_lock+0x7c/0xc0
        do_notify_parent+0x1c9/0x2a0
        exit_notify+0x1a9/0x220
        do_exit+0x2ba/0x450
        do_group_exit+0x2d/0x90
        __x64_sys_exit_group+0x14/0x20
        do_syscall_64+0x3b/0x90
        entry_SYSCALL_64_after_hwframe+0x46/0xb0
      
      Obviously this is not the desired result.
      
      Move the testing for TPARG_FL_TPOINT which is only used for event probes
      to the top of the "$" variable check, as all the other variables are not
      used for event probes. Also add a check in the register parsing "%" to
      fail if an event probe is used.
      
      Link: https://lkml.kernel.org/r/20220820134400.564426983@goodmis.org
      
      Cc: stable@vger.kernel.org
      Cc: Ingo Molnar <mingo@kernel.org>
      Cc: Andrew Morton <akpm@linux-foundation.org>
      Cc: Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
      Cc: Tom Zanussi <zanussi@kernel.org>
      Fixes: 7491e2c4 ("tracing: Add a probe that attaches to trace events")
      Acked-by: default avatarMasami Hiramatsu (Google) <mhiramat@kernel.org>
      Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
      2673c60e
    • Yang Jihong's avatar
      ftrace: Fix NULL pointer dereference in is_ftrace_trampoline when ftrace is dead · c3b0f72e
      Yang Jihong authored
      ftrace_startup does not remove ops from ftrace_ops_list when
      ftrace_startup_enable fails:
      
      register_ftrace_function
        ftrace_startup
          __register_ftrace_function
            ...
            add_ftrace_ops(&ftrace_ops_list, ops)
            ...
          ...
          ftrace_startup_enable // if ftrace failed to modify, ftrace_disabled is set to 1
          ...
        return 0 // ops is in the ftrace_ops_list.
      
      When ftrace_disabled = 1, unregister_ftrace_function simply returns without doing anything:
      unregister_ftrace_function
        ftrace_shutdown
          if (unlikely(ftrace_disabled))
                  return -ENODEV;  // return here, __unregister_ftrace_function is not executed,
                                   // as a result, ops is still in the ftrace_ops_list
          __unregister_ftrace_function
          ...
      
      If ops is dynamically allocated, it will be free later, in this case,
      is_ftrace_trampoline accesses NULL pointer:
      
      is_ftrace_trampoline
        ftrace_ops_trampoline
          do_for_each_ftrace_op(op, ftrace_ops_list) // OOPS! op may be NULL!
      
      Syzkaller reports as follows:
      [ 1203.506103] BUG: kernel NULL pointer dereference, address: 000000000000010b
      [ 1203.508039] #PF: supervisor read access in kernel mode
      [ 1203.508798] #PF: error_code(0x0000) - not-present page
      [ 1203.509558] PGD 800000011660b067 P4D 800000011660b067 PUD 130fb8067 PMD 0
      [ 1203.510560] Oops: 0000 [#1] SMP KASAN PTI
      [ 1203.511189] CPU: 6 PID: 29532 Comm: syz-executor.2 Tainted: G    B   W         5.10.0 #8
      [ 1203.512324] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
      [ 1203.513895] RIP: 0010:is_ftrace_trampoline+0x26/0xb0
      [ 1203.514644] Code: ff eb d3 90 41 55 41 54 49 89 fc 55 53 e8 f2 00 fd ff 48 8b 1d 3b 35 5d 03 e8 e6 00 fd ff 48 8d bb 90 00 00 00 e8 2a 81 26 00 <48> 8b ab 90 00 00 00 48 85 ed 74 1d e8 c9 00 fd ff 48 8d bb 98 00
      [ 1203.518838] RSP: 0018:ffffc900012cf960 EFLAGS: 00010246
      [ 1203.520092] RAX: 0000000000000000 RBX: 000000000000007b RCX: ffffffff8a331866
      [ 1203.521469] RDX: 0000000000000000 RSI: 0000000000000008 RDI: 000000000000010b
      [ 1203.522583] RBP: 0000000000000000 R08: 0000000000000000 R09: ffffffff8df18b07
      [ 1203.523550] R10: fffffbfff1be3160 R11: 0000000000000001 R12: 0000000000478399
      [ 1203.524596] R13: 0000000000000000 R14: ffff888145088000 R15: 0000000000000008
      [ 1203.525634] FS:  00007f429f5f4700(0000) GS:ffff8881daf00000(0000) knlGS:0000000000000000
      [ 1203.526801] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
      [ 1203.527626] CR2: 000000000000010b CR3: 0000000170e1e001 CR4: 00000000003706e0
      [ 1203.528611] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
      [ 1203.529605] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
      
      Therefore, when ftrace_startup_enable fails, we need to rollback registration
      process and remove ops from ftrace_ops_list.
      
      Link: https://lkml.kernel.org/r/20220818032659.56209-1-yangjihong1@huawei.comSuggested-by: default avatarSteven Rostedt <rostedt@goodmis.org>
      Signed-off-by: default avatarYang Jihong <yangjihong1@huawei.com>
      Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
      c3b0f72e
    • Steven Rostedt (Google)'s avatar
      tracing/perf: Fix double put of trace event when init fails · 7249921d
      Steven Rostedt (Google) authored
      If in perf_trace_event_init(), the perf_trace_event_open() fails, then it
      will call perf_trace_event_unreg() which will not only unregister the perf
      trace event, but will also call the put() function of the tp_event.
      
      The problem here is that the trace_event_try_get_ref() is called by the
      caller of perf_trace_event_init() and if perf_trace_event_init() returns a
      failure, it will then call trace_event_put(). But since the
      perf_trace_event_unreg() already called the trace_event_put() function, it
      triggers a WARN_ON().
      
       WARNING: CPU: 1 PID: 30309 at kernel/trace/trace_dynevent.c:46 trace_event_dyn_put_ref+0x15/0x20
      
      If perf_trace_event_reg() does not call the trace_event_try_get_ref() then
      the perf_trace_event_unreg() should not be calling trace_event_put(). This
      breaks symmetry and causes bugs like these.
      
      Pull out the trace_event_put() from perf_trace_event_unreg() and call it
      in the locations that perf_trace_event_unreg() is called. This not only
      fixes this bug, but also brings back the proper symmetry of the reg/unreg
      vs get/put logic.
      
      Link: https://lore.kernel.org/all/cover.1660347763.git.kjlx@templeofstupid.com/
      Link: https://lkml.kernel.org/r/20220816192817.43d5e17f@gandalf.local.home
      
      Cc: stable@vger.kernel.org
      Fixes: 1d18538e ("tracing: Have dynamic events have a ref counter")
      Reported-by: default avatarKrister Johansen <kjlx@templeofstupid.com>
      Reviewed-by: default avatarKrister Johansen <kjlx@templeofstupid.com>
      Tested-by: default avatarKrister Johansen <kjlx@templeofstupid.com>
      Acked-by: default avatarJiri Olsa <jolsa@kernel.org>
      Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
      7249921d
    • Lukas Bulwahn's avatar
      tracing: React to error return from traceprobe_parse_event_name() · d8a64313
      Lukas Bulwahn authored
      The function traceprobe_parse_event_name() may set the first two function
      arguments to a non-null value and still return -EINVAL to indicate an
      unsuccessful completion of the function. Hence, it is not sufficient to
      just check the result of the two function arguments for being not null,
      but the return value also needs to be checked.
      
      Commit 95c104c3 ("tracing: Auto generate event name when creating a
      group of events") changed the error-return-value checking of the second
      traceprobe_parse_event_name() invocation in __trace_eprobe_create() and
      removed checking the return value to jump to the error handling case.
      
      Reinstate using the return value in the error-return-value checking.
      
      Link: https://lkml.kernel.org/r/20220811071734.20700-1-lukas.bulwahn@gmail.com
      
      Fixes: 95c104c3 ("tracing: Auto generate event name when creating a group of events")
      Acked-by: default avatarLinyu Yuan <quic_linyyuan@quicinc.com>
      Signed-off-by: default avatarLukas Bulwahn <lukas.bulwahn@gmail.com>
      Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
      d8a64313
    • Linus Torvalds's avatar
      Merge tag 'i2c-for-6.0-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux · e3f259d3
      Linus Torvalds authored
      Pull i2c fixes from Wolfram Sang:
       "A revert to fix a regression introduced this merge window and a fix
        for proper error handling in the remove path of the iMX driver"
      
      * tag 'i2c-for-6.0-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux:
        i2c: imx: Make sure to unregister adapter on remove()
        Revert "i2c: scmi: Replace open coded device_get_match_data()"
      e3f259d3