* [PATCH 0/3] objtool/klp: Reject changed functions without an ftrace call site
@ 2026-10-22 12:25 Puranjay Mohan
2026-10-22 12:25 ` [PATCH 1/3] objtool/klp: Build test fixtures with ftrace Puranjay Mohan
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Puranjay Mohan @ 2026-10-22 12:25 UTC (permalink / raw)
To: live-patching
Cc: Puranjay Mohan, Josh Poimboeuf, Peter Zijlstra, Song Liu,
Miroslav Benes, Joe Lawrence
A changed function without an ftrace call site makes klp-build produce a
patch module which fails to load. The kernel's 'inline' implies notrace,
so this happens whenever the compiler emits an inline function out of
line. Patch 2 makes klp diff reject such functions, patch 3 tests it.
Patch 1 builds the test fixtures with ftrace, which patch 2 needs.
Tested on x86_64 with GCC 11 and Clang 22: the objtool klp tests, and
klp-build with a vng boot, where a patch module built without the check
fails to load with "failed to find location for function".
Note for the arm64 klp-build series: arm64 has no __mcount_loc, so on
top of that series this would reject every changed function. It needs
mark_traceable_functions() to also handle __patchable_function_entries,
using pfe_offset. I can send that on top of the arm64 series.
Puranjay Mohan (3):
objtool/klp: Build test fixtures with ftrace
objtool/klp: Reject changed functions without an ftrace call site
objtool/klp: Add test for changed functions without an ftrace call
site
.../objtool/Documentation/klp-write-tests.txt | 9 ++-
tools/objtool/include/objtool/elf.h | 1 +
tools/objtool/klp-diff.c | 47 ++++++++++++
.../tests/generic/test-thinlto-ambiguity.sh | 3 +-
.../tests/generic/test-thinlto-local.sh | 5 +-
tools/objtool/tests/lib.sh | 42 ++++++++++-
.../tests/x86/fixtures/notrace_target.c | 71 ++++++++++++++++++
tools/objtool/tests/x86/test-checksum-alt.sh | 4 -
.../objtool/tests/x86/test-notrace-target.sh | 74 +++++++++++++++++++
.../tests/x86/test-special-sections.sh | 23 +++---
10 files changed, 257 insertions(+), 22 deletions(-)
create mode 100644 tools/objtool/tests/x86/fixtures/notrace_target.c
create mode 100755 tools/objtool/tests/x86/test-notrace-target.sh
base-commit: 6ecc854643e6fd6a2d07bc8d83015c7c34a951eb
--
2.53.0-Meta
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/3] objtool/klp: Build test fixtures with ftrace 2026-10-22 12:25 [PATCH 0/3] objtool/klp: Reject changed functions without an ftrace call site Puranjay Mohan @ 2026-10-22 12:25 ` Puranjay Mohan 2026-10-22 14:46 ` Joe Lawrence 2026-10-22 12:25 ` [PATCH 2/3] objtool/klp: Reject changed functions without an ftrace call site Puranjay Mohan 2026-10-22 12:25 ` [PATCH 3/3] objtool/klp: Add test for " Puranjay Mohan 2 siblings, 1 reply; 8+ messages in thread From: Puranjay Mohan @ 2026-10-22 12:25 UTC (permalink / raw) To: live-patching Cc: Puranjay Mohan, Josh Poimboeuf, Peter Zijlstra, Song Liu, Miroslav Benes, Joe Lawrence Livepatch depends on ftrace, so the kernel klp-build operates on always has __mcount_loc, but the test fixtures are built without it. On x86, build them with -pg -mfentry and generate __mcount_loc like the kernel does: with -mrecord-mcount -mnop-mcount if the compiler supports it (FTRACE_MCOUNT_USE_CC), otherwise with objtool --mcount --mnop (FTRACE_MCOUNT_USE_OBJTOOL). test-checksum-alt ran objtool --mcount only to get insn->alts, which "klp checksum" builds by itself, and that now warns about the existing __mcount_loc. Remove it. test-special-sections can now check the real __mcount_loc instead of a hand-written one. Signed-off-by: Puranjay Mohan <puranjay@kernel.org> --- .../objtool/Documentation/klp-write-tests.txt | 9 ++-- .../tests/generic/test-thinlto-ambiguity.sh | 3 +- .../tests/generic/test-thinlto-local.sh | 5 ++- tools/objtool/tests/lib.sh | 42 +++++++++++++++++-- tools/objtool/tests/x86/test-checksum-alt.sh | 4 -- .../tests/x86/test-special-sections.sh | 23 ++++++---- 6 files changed, 64 insertions(+), 22 deletions(-) diff --git a/tools/objtool/Documentation/klp-write-tests.txt b/tools/objtool/Documentation/klp-write-tests.txt index eb2cacaf57207..7cc2148083cc2 100644 --- a/tools/objtool/Documentation/klp-write-tests.txt +++ b/tools/objtool/Documentation/klp-write-tests.txt @@ -116,9 +116,12 @@ Traps that have bitten before section entsize or annotations". Their targets need real (global) symbols, or it reports "failed to convert reloc sym". * **Prefer letting objtool generate what objtool generates.** - ``.static_call_sites``, ``.mcount_loc``, ``.ibt_endbr_seal`` and ORC come - from its check pass. Call ``run_objtool_check --mcount`` and let it build - them; a hand-written copy tests your reading of the format, not the format. + ``.static_call_sites``, ``.ibt_endbr_seal`` and ORC come from its check + pass. Call ``run_objtool_check`` with the option for what you need and let + it build them; a hand-written copy tests your reading of the format, not the + format. +* **Fixtures are built with ftrace.** Objects not built with ``build_one`` + need ``$FTRACE_CFLAGS`` and ``record_mcount`` too. Write one by hand only when the test needs a shape objtool will not produce, and say so in the fixture. generic/fixtures/static_call.c does: objtool diff --git a/tools/objtool/tests/generic/test-thinlto-ambiguity.sh b/tools/objtool/tests/generic/test-thinlto-ambiguity.sh index 34f4f3a58e201..73740c804d54f 100755 --- a/tools/objtool/tests/generic/test-thinlto-ambiguity.sh +++ b/tools/objtool/tests/generic/test-thinlto-ambiguity.sh @@ -38,12 +38,13 @@ build_thinlto() # $1 output object, $2 extra flags { local t for t in "" -DTU_B -DTU_C; do - $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections \ + $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections $FTRACE_CFLAGS \ $2 $t -c "$FIXTURES_DIR/thinlto_ambiguity.c" \ -o "$workdir/tu$t.o" 2>/dev/null || return 1 done "$THIN_LD" -r "$workdir/tu.o" "$workdir/tu-DTU_B.o" "$workdir/tu-DTU_C.o" \ -o "$1" 2>/dev/null || return 1 + record_mcount "$1" } build_thinlto "$workdir/orig.o" "" || diff --git a/tools/objtool/tests/generic/test-thinlto-local.sh b/tools/objtool/tests/generic/test-thinlto-local.sh index a266263676ed5..090c6df75ad64 100755 --- a/tools/objtool/tests/generic/test-thinlto-local.sh +++ b/tools/objtool/tests/generic/test-thinlto-local.sh @@ -15,11 +15,12 @@ find_thinlto_toolchain || build_thinlto() # $1 output object, $2 extra flags { - $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections $2 \ + $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections $FTRACE_CFLAGS $2 \ -c "$FIXTURES_DIR/thinlto_local.c" -o "$workdir/tu_a.o" 2>/dev/null || return 1 - $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections $2 -DTU_B \ + $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections $FTRACE_CFLAGS $2 -DTU_B \ -c "$FIXTURES_DIR/thinlto_local.c" -o "$workdir/tu_b.o" 2>/dev/null || return 1 "$THIN_LD" -r "$workdir/tu_a.o" "$workdir/tu_b.o" -o "$1" 2>/dev/null || return 1 + record_mcount "$1" } build_thinlto "$workdir/orig.o" "" || diff --git a/tools/objtool/tests/lib.sh b/tools/objtool/tests/lib.sh index 4da168d0ddca3..d07eb07d15611 100644 --- a/tools/objtool/tests/lib.sh +++ b/tools/objtool/tests/lib.sh @@ -41,7 +41,7 @@ OBJTOOL="${OBJTOOL:-$TESTS_DIR/../objtool}" # klp_preflight() { - local tmp tool cc_version arch host cc_arch + local tmp tool cc_version arch host cc_arch mcount= bail() { echo "Bail out! $*" >&2; exit 1; } @@ -99,6 +99,16 @@ klp_preflight() *AArch64*) cc_arch=arm64 ;; *) cc_arch= ;; esac + + # Same test as FTRACE_MCOUNT_USE_CC. + if [ "$cc_arch" = x86 ]; then + if $CC -pg -mfentry -mrecord-mcount -mnop-mcount -c \ + -o "$tmp/probe.o" "$tmp/probe.c" 2>/dev/null; then + mcount=cc + else + mcount=objtool + fi + fi rm -rf "$tmp" # Normalize to the kernel's SRCARCH. @@ -119,8 +129,9 @@ klp_preflight() "the $arch tests would run against the wrong architecture" KLP_TEST_ARCH="$arch" + KLP_TEST_MCOUNT="$mcount" KLP_TEST_PREFLIGHT=done - export OBJTOOL CC KLP_TEST_ARCH KLP_TEST_PREFLIGHT + export OBJTOOL CC KLP_TEST_ARCH KLP_TEST_MCOUNT KLP_TEST_PREFLIGHT cc_version="$($CC --version 2>/dev/null | head -1)" cat <<EOF @@ -128,6 +139,7 @@ klp_preflight() # objtool $OBJTOOL (klp: yes) # compiler $cc_version # arch $KLP_TEST_ARCH$([ "$arch" = "$host" ] || echo " (host $host, cross)") +# mcount ${KLP_TEST_MCOUNT:-n/a} # tmpdir ${TMPDIR:-/tmp} (each test builds in a fresh directory here) EOF } @@ -153,6 +165,16 @@ EOF FIXTURE_CFLAGS="-O2 -ffunction-sections -fdata-sections -fno-common \ -fno-asynchronous-unwind-tables" +# Build with ftrace, which livepatch depends on. Objects not built with +# build_one() need FTRACE_CFLAGS and record_mcount() too. +FTRACE_CFLAGS= +if [ "$KLP_TEST_ARCH" = x86 ]; then + FTRACE_CFLAGS="-pg -mfentry" + [ "$KLP_TEST_MCOUNT" = cc ] && + FTRACE_CFLAGS="$FTRACE_CFLAGS -mrecord-mcount -mnop-mcount" +fi +FIXTURE_CFLAGS="$FIXTURE_CFLAGS $FTRACE_CFLAGS" + test_name="$(basename "$0" .sh)" workdir= @@ -343,6 +365,20 @@ build_one() $CC -c $FIXTURE_CFLAGS "$@" -o "$out" "$fixture" 2>"$workdir/cc.log" || fail "$(basename "$fixture") does not build: $(tail -1 "$workdir/cc.log")" + + record_mcount "$out" +} + +# record_mcount <object> +# +# FTRACE_MCOUNT_USE_OBJTOOL. CONFIG_KLP_BUILD always runs objtool with --link. +record_mcount() +{ + [ "$KLP_TEST_MCOUNT" = objtool ] || return 0 + + "$OBJTOOL" --mcount --mnop --link "$1" 2>"$workdir/objtool.log" || + fail "objtool --mcount failed on $(basename "$1"):" \ + "$(tail -1 "$workdir/objtool.log")" } # build_pair <fixture.c> [cflags...] @@ -365,7 +401,7 @@ build_pair() # Run objtool's ordinary check pass over the pair, as the kernel build does. # # Some of what klp diff consumes is produced by this pass rather than by the -# compiler: .static_call_sites, .mcount_loc, .ibt_endbr_seal, ORC. +# compiler: .static_call_sites, .ibt_endbr_seal, ORC. # # Only module objects see it before klp-build -- with CONFIG_KLP_BUILD the # per-object pass is deferred, so built-in objects reach klp diff exactly as diff --git a/tools/objtool/tests/x86/test-checksum-alt.sh b/tools/objtool/tests/x86/test-checksum-alt.sh index 74ebfca2b3ff3..eb9864c4bf825 100755 --- a/tools/objtool/tests/x86/test-checksum-alt.sh +++ b/tools/objtool/tests/x86/test-checksum-alt.sh @@ -16,9 +16,6 @@ # the feature bit, so the failure looks like a machine-specific bug rather than # a missing patch. # -# insn->alts exists only after objtool's check pass, so the pair goes through -# that first -- the compiler emits none of this structure itself. -# # Covers the same ground as corpus/x86_64/checksum-alt-group, # checksum-alt-no-group and checksum-alt-recursion-guard in Joe Lawrence's # klp-build unit test corpus. @@ -31,7 +28,6 @@ check() { build_pair checksum_alt.c "-D$1" assert_input_section .altinstructions - run_objtool_check --mcount run_checksum assert_checksum_differs target diff --git a/tools/objtool/tests/x86/test-special-sections.sh b/tools/objtool/tests/x86/test-special-sections.sh index 8dfaa4fc9a360..b05d43cdc8aa4 100755 --- a/tools/objtool/tests/x86/test-special-sections.sh +++ b/tools/objtool/tests/x86/test-special-sections.sh @@ -15,26 +15,31 @@ # section, entry size, relocations per entry -for spec in "__bug_table 12 1" "__ex_table 12 2" "__mcount_loc 8 1"; do +for spec in "__bug_table 12 1" "__ex_table 12 2"; do set -- $spec - sec=$1 # A fresh workdir per section: run_diff caches its checksums. setup build_pair special_sections.c \ -DSPECIAL_SEC="\"$1\"" -DSPECIAL_ENTSIZE="$2" -DSPECIAL_RELOCS="$3" - assert_input_section "$sec" + # __mcount_loc comes from FTRACE_CFLAGS, not from the fixture. + for sec in "$1" __mcount_loc; do + assert_input_section "$sec" + done run_diff - # Extracted, and pointing at the function that was patched. - assert_section "$sec" - assert_reloc_sym "$sec" target assert_patched target - - # Nothing belonging to the function that was not. assert_not_patched other - assert_no_reloc_sym "$sec" other + + for sec in "$1" __mcount_loc; do + # Extracted, and pointing at the function that was patched. + assert_section "$sec" + assert_reloc_sym "$sec" target + + # Nothing belonging to the function that was not. + assert_no_reloc_sym "$sec" other + done cleanup done -- 2.53.0-Meta ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] objtool/klp: Build test fixtures with ftrace 2026-10-22 12:25 ` [PATCH 1/3] objtool/klp: Build test fixtures with ftrace Puranjay Mohan @ 2026-10-22 14:46 ` Joe Lawrence 2026-10-22 16:43 ` Puranjay Mohan 0 siblings, 1 reply; 8+ messages in thread From: Joe Lawrence @ 2026-10-22 14:46 UTC (permalink / raw) To: Puranjay Mohan Cc: live-patching, Josh Poimboeuf, Peter Zijlstra, Song Liu, Miroslav Benes On Thu, Oct 22, 2026 at 05:25:36AM -0700, Puranjay Mohan wrote: > Livepatch depends on ftrace, so the kernel klp-build operates on always > has __mcount_loc, but the test fixtures are built without it. > > On x86, build them with -pg -mfentry and generate __mcount_loc like the > kernel does: with -mrecord-mcount -mnop-mcount if the compiler supports > it (FTRACE_MCOUNT_USE_CC), otherwise with objtool --mcount --mnop > (FTRACE_MCOUNT_USE_OBJTOOL). > > test-checksum-alt ran objtool --mcount only to get insn->alts, which > "klp checksum" builds by itself, and that now warns about the existing > __mcount_loc. Remove it. > > test-special-sections can now check the real __mcount_loc instead of a > hand-written one. > > Signed-off-by: Puranjay Mohan <puranjay@kernel.org> > --- > .../objtool/Documentation/klp-write-tests.txt | 9 ++-- > .../tests/generic/test-thinlto-ambiguity.sh | 3 +- > .../tests/generic/test-thinlto-local.sh | 5 ++- > tools/objtool/tests/lib.sh | 42 +++++++++++++++++-- > tools/objtool/tests/x86/test-checksum-alt.sh | 4 -- > .../tests/x86/test-special-sections.sh | 23 ++++++---- > 6 files changed, 64 insertions(+), 22 deletions(-) > > diff --git a/tools/objtool/Documentation/klp-write-tests.txt b/tools/objtool/Documentation/klp-write-tests.txt > index eb2cacaf57207..7cc2148083cc2 100644 > --- a/tools/objtool/Documentation/klp-write-tests.txt > +++ b/tools/objtool/Documentation/klp-write-tests.txt > @@ -116,9 +116,12 @@ Traps that have bitten before > section entsize or annotations". Their targets need real (global) symbols, > or it reports "failed to convert reloc sym". > * **Prefer letting objtool generate what objtool generates.** > - ``.static_call_sites``, ``.mcount_loc``, ``.ibt_endbr_seal`` and ORC come > - from its check pass. Call ``run_objtool_check --mcount`` and let it build > - them; a hand-written copy tests your reading of the format, not the format. > + ``.static_call_sites``, ``.ibt_endbr_seal`` and ORC come from its check > + pass. Call ``run_objtool_check`` with the option for what you need and let > + it build them; a hand-written copy tests your reading of the format, not the > + format. > +* **Fixtures are built with ftrace.** Objects not built with ``build_one`` > + need ``$FTRACE_CFLAGS`` and ``record_mcount`` too. > > Write one by hand only when the test needs a shape objtool will not produce, > and say so in the fixture. generic/fixtures/static_call.c does: objtool > diff --git a/tools/objtool/tests/generic/test-thinlto-ambiguity.sh b/tools/objtool/tests/generic/test-thinlto-ambiguity.sh > index 34f4f3a58e201..73740c804d54f 100755 > --- a/tools/objtool/tests/generic/test-thinlto-ambiguity.sh > +++ b/tools/objtool/tests/generic/test-thinlto-ambiguity.sh > @@ -38,12 +38,13 @@ build_thinlto() # $1 output object, $2 extra flags > { > local t > for t in "" -DTU_B -DTU_C; do > - $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections \ > + $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections $FTRACE_CFLAGS \ > $2 $t -c "$FIXTURES_DIR/thinlto_ambiguity.c" \ > -o "$workdir/tu$t.o" 2>/dev/null || return 1 > done > "$THIN_LD" -r "$workdir/tu.o" "$workdir/tu-DTU_B.o" "$workdir/tu-DTU_C.o" \ > -o "$1" 2>/dev/null || return 1 > + record_mcount "$1" > } > > build_thinlto "$workdir/orig.o" "" || > diff --git a/tools/objtool/tests/generic/test-thinlto-local.sh b/tools/objtool/tests/generic/test-thinlto-local.sh > index a266263676ed5..090c6df75ad64 100755 > --- a/tools/objtool/tests/generic/test-thinlto-local.sh > +++ b/tools/objtool/tests/generic/test-thinlto-local.sh > @@ -15,11 +15,12 @@ find_thinlto_toolchain || > > build_thinlto() # $1 output object, $2 extra flags > { > - $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections $2 \ > + $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections $FTRACE_CFLAGS $2 \ > -c "$FIXTURES_DIR/thinlto_local.c" -o "$workdir/tu_a.o" 2>/dev/null || return 1 > - $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections $2 -DTU_B \ > + $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections $FTRACE_CFLAGS $2 -DTU_B \ > -c "$FIXTURES_DIR/thinlto_local.c" -o "$workdir/tu_b.o" 2>/dev/null || return 1 > "$THIN_LD" -r "$workdir/tu_a.o" "$workdir/tu_b.o" -o "$1" 2>/dev/null || return 1 > + record_mcount "$1" > } > > build_thinlto "$workdir/orig.o" "" || > diff --git a/tools/objtool/tests/lib.sh b/tools/objtool/tests/lib.sh > index 4da168d0ddca3..d07eb07d15611 100644 > --- a/tools/objtool/tests/lib.sh > +++ b/tools/objtool/tests/lib.sh > @@ -41,7 +41,7 @@ OBJTOOL="${OBJTOOL:-$TESTS_DIR/../objtool}" > # > klp_preflight() > { > - local tmp tool cc_version arch host cc_arch > + local tmp tool cc_version arch host cc_arch mcount= > > bail() { echo "Bail out! $*" >&2; exit 1; } > > @@ -99,6 +99,16 @@ klp_preflight() > *AArch64*) cc_arch=arm64 ;; > *) cc_arch= ;; > esac > + > + # Same test as FTRACE_MCOUNT_USE_CC. > + if [ "$cc_arch" = x86 ]; then > + if $CC -pg -mfentry -mrecord-mcount -mnop-mcount -c \ > + -o "$tmp/probe.o" "$tmp/probe.c" 2>/dev/null; then > + mcount=cc > + else > + mcount=objtool > + fi > + fi > rm -rf "$tmp" > > # Normalize to the kernel's SRCARCH. > @@ -119,8 +129,9 @@ klp_preflight() > "the $arch tests would run against the wrong architecture" > > KLP_TEST_ARCH="$arch" > + KLP_TEST_MCOUNT="$mcount" > KLP_TEST_PREFLIGHT=done > - export OBJTOOL CC KLP_TEST_ARCH KLP_TEST_PREFLIGHT > + export OBJTOOL CC KLP_TEST_ARCH KLP_TEST_MCOUNT KLP_TEST_PREFLIGHT > > cc_version="$($CC --version 2>/dev/null | head -1)" > cat <<EOF > @@ -128,6 +139,7 @@ klp_preflight() > # objtool $OBJTOOL (klp: yes) > # compiler $cc_version > # arch $KLP_TEST_ARCH$([ "$arch" = "$host" ] || echo " (host $host, cross)") > +# mcount ${KLP_TEST_MCOUNT:-n/a} > # tmpdir ${TMPDIR:-/tmp} (each test builds in a fresh directory here) > EOF > } > @@ -153,6 +165,16 @@ EOF > FIXTURE_CFLAGS="-O2 -ffunction-sections -fdata-sections -fno-common \ > -fno-asynchronous-unwind-tables" > > +# Build with ftrace, which livepatch depends on. Objects not built with > +# build_one() need FTRACE_CFLAGS and record_mcount() too. > +FTRACE_CFLAGS= > +if [ "$KLP_TEST_ARCH" = x86 ]; then > + FTRACE_CFLAGS="-pg -mfentry" > + [ "$KLP_TEST_MCOUNT" = cc ] && > + FTRACE_CFLAGS="$FTRACE_CFLAGS -mrecord-mcount -mnop-mcount" > +fi > +FIXTURE_CFLAGS="$FIXTURE_CFLAGS $FTRACE_CFLAGS" > + > test_name="$(basename "$0" .sh)" > workdir= > > @@ -343,6 +365,20 @@ build_one() > > $CC -c $FIXTURE_CFLAGS "$@" -o "$out" "$fixture" 2>"$workdir/cc.log" || > fail "$(basename "$fixture") does not build: $(tail -1 "$workdir/cc.log")" > + > + record_mcount "$out" > +} > + > +# record_mcount <object> > +# > +# FTRACE_MCOUNT_USE_OBJTOOL. CONFIG_KLP_BUILD always runs objtool with --link. > +record_mcount() > +{ > + [ "$KLP_TEST_MCOUNT" = objtool ] || return 0 > + > + "$OBJTOOL" --mcount --mnop --link "$1" 2>"$workdir/objtool.log" || > + fail "objtool --mcount failed on $(basename "$1"):" \ > + "$(tail -1 "$workdir/objtool.log")" > } > > # build_pair <fixture.c> [cflags...] > @@ -365,7 +401,7 @@ build_pair() > # Run objtool's ordinary check pass over the pair, as the kernel build does. > # > # Some of what klp diff consumes is produced by this pass rather than by the > -# compiler: .static_call_sites, .mcount_loc, .ibt_endbr_seal, ORC. > +# compiler: .static_call_sites, .ibt_endbr_seal, ORC. > # > # Only module objects see it before klp-build -- with CONFIG_KLP_BUILD the > # per-object pass is deferred, so built-in objects reach klp diff exactly as > diff --git a/tools/objtool/tests/x86/test-checksum-alt.sh b/tools/objtool/tests/x86/test-checksum-alt.sh > index 74ebfca2b3ff3..eb9864c4bf825 100755 > --- a/tools/objtool/tests/x86/test-checksum-alt.sh > +++ b/tools/objtool/tests/x86/test-checksum-alt.sh > @@ -16,9 +16,6 @@ > # the feature bit, so the failure looks like a machine-specific bug rather than > # a missing patch. > # > -# insn->alts exists only after objtool's check pass, so the pair goes through > -# that first -- the compiler emits none of this structure itself. > -# > # Covers the same ground as corpus/x86_64/checksum-alt-group, > # checksum-alt-no-group and checksum-alt-recursion-guard in Joe Lawrence's > # klp-build unit test corpus. > @@ -31,7 +28,6 @@ check() > { > build_pair checksum_alt.c "-D$1" > assert_input_section .altinstructions > - run_objtool_check --mcount > run_checksum > > assert_checksum_differs target > diff --git a/tools/objtool/tests/x86/test-special-sections.sh b/tools/objtool/tests/x86/test-special-sections.sh > index 8dfaa4fc9a360..b05d43cdc8aa4 100755 > --- a/tools/objtool/tests/x86/test-special-sections.sh > +++ b/tools/objtool/tests/x86/test-special-sections.sh > @@ -15,26 +15,31 @@ > > > # section, entry size, relocations per entry > -for spec in "__bug_table 12 1" "__ex_table 12 2" "__mcount_loc 8 1"; do > +for spec in "__bug_table 12 1" "__ex_table 12 2"; do > set -- $spec > - sec=$1 > > # A fresh workdir per section: run_diff caches its checksums. > setup > build_pair special_sections.c \ > -DSPECIAL_SEC="\"$1\"" -DSPECIAL_ENTSIZE="$2" -DSPECIAL_RELOCS="$3" > > - assert_input_section "$sec" > + # __mcount_loc comes from FTRACE_CFLAGS, not from the fixture. > + for sec in "$1" __mcount_loc; do > + assert_input_section "$sec" > + done > run_diff > > - # Extracted, and pointing at the function that was patched. > - assert_section "$sec" > - assert_reloc_sym "$sec" target > assert_patched target > - > - # Nothing belonging to the function that was not. > assert_not_patched other > - assert_no_reloc_sym "$sec" other > + > + for sec in "$1" __mcount_loc; do > + # Extracted, and pointing at the function that was patched. > + assert_section "$sec" > + assert_reloc_sym "$sec" target > + > + # Nothing belonging to the function that was not. > + assert_no_reloc_sym "$sec" other > + done Nitpick: it looks a little odd that the __mcount_loc special case was pushed inside the greater section loop (bug-tables, ex-table) so it runs twice AFAICT. Would it be clearer to just pull this out as its own block? -- Joe ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] objtool/klp: Build test fixtures with ftrace 2026-10-22 14:46 ` Joe Lawrence @ 2026-10-22 16:43 ` Puranjay Mohan 0 siblings, 0 replies; 8+ messages in thread From: Puranjay Mohan @ 2026-10-22 16:43 UTC (permalink / raw) To: Joe Lawrence Cc: live-patching, Josh Poimboeuf, Peter Zijlstra, Song Liu, Miroslav Benes On Thu, Oct 22, 2026 at 3:46 PM Joe Lawrence <joe.lawrence@redhat.com> wrote: > > On Thu, Oct 22, 2026 at 05:25:36AM -0700, Puranjay Mohan wrote: > > Livepatch depends on ftrace, so the kernel klp-build operates on always > > has __mcount_loc, but the test fixtures are built without it. > > > > On x86, build them with -pg -mfentry and generate __mcount_loc like the > > kernel does: with -mrecord-mcount -mnop-mcount if the compiler supports > > it (FTRACE_MCOUNT_USE_CC), otherwise with objtool --mcount --mnop > > (FTRACE_MCOUNT_USE_OBJTOOL). > > > > test-checksum-alt ran objtool --mcount only to get insn->alts, which > > "klp checksum" builds by itself, and that now warns about the existing > > __mcount_loc. Remove it. > > > > test-special-sections can now check the real __mcount_loc instead of a > > hand-written one. > > > > Signed-off-by: Puranjay Mohan <puranjay@kernel.org> > > --- > > .../objtool/Documentation/klp-write-tests.txt | 9 ++-- > > .../tests/generic/test-thinlto-ambiguity.sh | 3 +- > > .../tests/generic/test-thinlto-local.sh | 5 ++- > > tools/objtool/tests/lib.sh | 42 +++++++++++++++++-- > > tools/objtool/tests/x86/test-checksum-alt.sh | 4 -- > > .../tests/x86/test-special-sections.sh | 23 ++++++---- > > 6 files changed, 64 insertions(+), 22 deletions(-) > > > > diff --git a/tools/objtool/Documentation/klp-write-tests.txt b/tools/objtool/Documentation/klp-write-tests.txt > > index eb2cacaf57207..7cc2148083cc2 100644 > > --- a/tools/objtool/Documentation/klp-write-tests.txt > > +++ b/tools/objtool/Documentation/klp-write-tests.txt > > @@ -116,9 +116,12 @@ Traps that have bitten before > > section entsize or annotations". Their targets need real (global) symbols, > > or it reports "failed to convert reloc sym". > > * **Prefer letting objtool generate what objtool generates.** > > - ``.static_call_sites``, ``.mcount_loc``, ``.ibt_endbr_seal`` and ORC come > > - from its check pass. Call ``run_objtool_check --mcount`` and let it build > > - them; a hand-written copy tests your reading of the format, not the format. > > + ``.static_call_sites``, ``.ibt_endbr_seal`` and ORC come from its check > > + pass. Call ``run_objtool_check`` with the option for what you need and let > > + it build them; a hand-written copy tests your reading of the format, not the > > + format. > > +* **Fixtures are built with ftrace.** Objects not built with ``build_one`` > > + need ``$FTRACE_CFLAGS`` and ``record_mcount`` too. > > > > Write one by hand only when the test needs a shape objtool will not produce, > > and say so in the fixture. generic/fixtures/static_call.c does: objtool > > diff --git a/tools/objtool/tests/generic/test-thinlto-ambiguity.sh b/tools/objtool/tests/generic/test-thinlto-ambiguity.sh > > index 34f4f3a58e201..73740c804d54f 100755 > > --- a/tools/objtool/tests/generic/test-thinlto-ambiguity.sh > > +++ b/tools/objtool/tests/generic/test-thinlto-ambiguity.sh > > @@ -38,12 +38,13 @@ build_thinlto() # $1 output object, $2 extra flags > > { > > local t > > for t in "" -DTU_B -DTU_C; do > > - $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections \ > > + $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections $FTRACE_CFLAGS \ > > $2 $t -c "$FIXTURES_DIR/thinlto_ambiguity.c" \ > > -o "$workdir/tu$t.o" 2>/dev/null || return 1 > > done > > "$THIN_LD" -r "$workdir/tu.o" "$workdir/tu-DTU_B.o" "$workdir/tu-DTU_C.o" \ > > -o "$1" 2>/dev/null || return 1 > > + record_mcount "$1" > > } > > > > build_thinlto "$workdir/orig.o" "" || > > diff --git a/tools/objtool/tests/generic/test-thinlto-local.sh b/tools/objtool/tests/generic/test-thinlto-local.sh > > index a266263676ed5..090c6df75ad64 100755 > > --- a/tools/objtool/tests/generic/test-thinlto-local.sh > > +++ b/tools/objtool/tests/generic/test-thinlto-local.sh > > @@ -15,11 +15,12 @@ find_thinlto_toolchain || > > > > build_thinlto() # $1 output object, $2 extra flags > > { > > - $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections $2 \ > > + $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections $FTRACE_CFLAGS $2 \ > > -c "$FIXTURES_DIR/thinlto_local.c" -o "$workdir/tu_a.o" 2>/dev/null || return 1 > > - $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections $2 -DTU_B \ > > + $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections $FTRACE_CFLAGS $2 -DTU_B \ > > -c "$FIXTURES_DIR/thinlto_local.c" -o "$workdir/tu_b.o" 2>/dev/null || return 1 > > "$THIN_LD" -r "$workdir/tu_a.o" "$workdir/tu_b.o" -o "$1" 2>/dev/null || return 1 > > + record_mcount "$1" > > } > > > > build_thinlto "$workdir/orig.o" "" || > > diff --git a/tools/objtool/tests/lib.sh b/tools/objtool/tests/lib.sh > > index 4da168d0ddca3..d07eb07d15611 100644 > > --- a/tools/objtool/tests/lib.sh > > +++ b/tools/objtool/tests/lib.sh > > @@ -41,7 +41,7 @@ OBJTOOL="${OBJTOOL:-$TESTS_DIR/../objtool}" > > # > > klp_preflight() > > { > > - local tmp tool cc_version arch host cc_arch > > + local tmp tool cc_version arch host cc_arch mcount= > > > > bail() { echo "Bail out! $*" >&2; exit 1; } > > > > @@ -99,6 +99,16 @@ klp_preflight() > > *AArch64*) cc_arch=arm64 ;; > > *) cc_arch= ;; > > esac > > + > > + # Same test as FTRACE_MCOUNT_USE_CC. > > + if [ "$cc_arch" = x86 ]; then > > + if $CC -pg -mfentry -mrecord-mcount -mnop-mcount -c \ > > + -o "$tmp/probe.o" "$tmp/probe.c" 2>/dev/null; then > > + mcount=cc > > + else > > + mcount=objtool > > + fi > > + fi > > rm -rf "$tmp" > > > > # Normalize to the kernel's SRCARCH. > > @@ -119,8 +129,9 @@ klp_preflight() > > "the $arch tests would run against the wrong architecture" > > > > KLP_TEST_ARCH="$arch" > > + KLP_TEST_MCOUNT="$mcount" > > KLP_TEST_PREFLIGHT=done > > - export OBJTOOL CC KLP_TEST_ARCH KLP_TEST_PREFLIGHT > > + export OBJTOOL CC KLP_TEST_ARCH KLP_TEST_MCOUNT KLP_TEST_PREFLIGHT > > > > cc_version="$($CC --version 2>/dev/null | head -1)" > > cat <<EOF > > @@ -128,6 +139,7 @@ klp_preflight() > > # objtool $OBJTOOL (klp: yes) > > # compiler $cc_version > > # arch $KLP_TEST_ARCH$([ "$arch" = "$host" ] || echo " (host $host, cross)") > > +# mcount ${KLP_TEST_MCOUNT:-n/a} > > # tmpdir ${TMPDIR:-/tmp} (each test builds in a fresh directory here) > > EOF > > } > > @@ -153,6 +165,16 @@ EOF > > FIXTURE_CFLAGS="-O2 -ffunction-sections -fdata-sections -fno-common \ > > -fno-asynchronous-unwind-tables" > > > > +# Build with ftrace, which livepatch depends on. Objects not built with > > +# build_one() need FTRACE_CFLAGS and record_mcount() too. > > +FTRACE_CFLAGS= > > +if [ "$KLP_TEST_ARCH" = x86 ]; then > > + FTRACE_CFLAGS="-pg -mfentry" > > + [ "$KLP_TEST_MCOUNT" = cc ] && > > + FTRACE_CFLAGS="$FTRACE_CFLAGS -mrecord-mcount -mnop-mcount" > > +fi > > +FIXTURE_CFLAGS="$FIXTURE_CFLAGS $FTRACE_CFLAGS" > > + > > test_name="$(basename "$0" .sh)" > > workdir= > > > > @@ -343,6 +365,20 @@ build_one() > > > > $CC -c $FIXTURE_CFLAGS "$@" -o "$out" "$fixture" 2>"$workdir/cc.log" || > > fail "$(basename "$fixture") does not build: $(tail -1 "$workdir/cc.log")" > > + > > + record_mcount "$out" > > +} > > + > > +# record_mcount <object> > > +# > > +# FTRACE_MCOUNT_USE_OBJTOOL. CONFIG_KLP_BUILD always runs objtool with --link. > > +record_mcount() > > +{ > > + [ "$KLP_TEST_MCOUNT" = objtool ] || return 0 > > + > > + "$OBJTOOL" --mcount --mnop --link "$1" 2>"$workdir/objtool.log" || > > + fail "objtool --mcount failed on $(basename "$1"):" \ > > + "$(tail -1 "$workdir/objtool.log")" > > } > > > > # build_pair <fixture.c> [cflags...] > > @@ -365,7 +401,7 @@ build_pair() > > # Run objtool's ordinary check pass over the pair, as the kernel build does. > > # > > # Some of what klp diff consumes is produced by this pass rather than by the > > -# compiler: .static_call_sites, .mcount_loc, .ibt_endbr_seal, ORC. > > +# compiler: .static_call_sites, .ibt_endbr_seal, ORC. > > # > > # Only module objects see it before klp-build -- with CONFIG_KLP_BUILD the > > # per-object pass is deferred, so built-in objects reach klp diff exactly as > > diff --git a/tools/objtool/tests/x86/test-checksum-alt.sh b/tools/objtool/tests/x86/test-checksum-alt.sh > > index 74ebfca2b3ff3..eb9864c4bf825 100755 > > --- a/tools/objtool/tests/x86/test-checksum-alt.sh > > +++ b/tools/objtool/tests/x86/test-checksum-alt.sh > > @@ -16,9 +16,6 @@ > > # the feature bit, so the failure looks like a machine-specific bug rather than > > # a missing patch. > > # > > -# insn->alts exists only after objtool's check pass, so the pair goes through > > -# that first -- the compiler emits none of this structure itself. > > -# > > # Covers the same ground as corpus/x86_64/checksum-alt-group, > > # checksum-alt-no-group and checksum-alt-recursion-guard in Joe Lawrence's > > # klp-build unit test corpus. > > @@ -31,7 +28,6 @@ check() > > { > > build_pair checksum_alt.c "-D$1" > > assert_input_section .altinstructions > > - run_objtool_check --mcount > > run_checksum > > > > assert_checksum_differs target > > diff --git a/tools/objtool/tests/x86/test-special-sections.sh b/tools/objtool/tests/x86/test-special-sections.sh > > index 8dfaa4fc9a360..b05d43cdc8aa4 100755 > > --- a/tools/objtool/tests/x86/test-special-sections.sh > > +++ b/tools/objtool/tests/x86/test-special-sections.sh > > @@ -15,26 +15,31 @@ > > > > > > # section, entry size, relocations per entry > > -for spec in "__bug_table 12 1" "__ex_table 12 2" "__mcount_loc 8 1"; do > > +for spec in "__bug_table 12 1" "__ex_table 12 2"; do > > set -- $spec > > - sec=$1 > > > > # A fresh workdir per section: run_diff caches its checksums. > > setup > > build_pair special_sections.c \ > > -DSPECIAL_SEC="\"$1\"" -DSPECIAL_ENTSIZE="$2" -DSPECIAL_RELOCS="$3" > > > > - assert_input_section "$sec" > > + # __mcount_loc comes from FTRACE_CFLAGS, not from the fixture. > > + for sec in "$1" __mcount_loc; do > > + assert_input_section "$sec" > > + done > > run_diff > > > > - # Extracted, and pointing at the function that was patched. > > - assert_section "$sec" > > - assert_reloc_sym "$sec" target > > assert_patched target > > - > > - # Nothing belonging to the function that was not. > > assert_not_patched other > > - assert_no_reloc_sym "$sec" other > > + > > + for sec in "$1" __mcount_loc; do > > + # Extracted, and pointing at the function that was patched. > > + assert_section "$sec" > > + assert_reloc_sym "$sec" target > > + > > + # Nothing belonging to the function that was not. > > + assert_no_reloc_sym "$sec" other > > + done > > Nitpick: it looks a little odd that the __mcount_loc special case was > pushed inside the greater section loop (bug-tables, ex-table) so it runs > twice AFAICT. Would it be clearer to just pull this out as its own > block? Thanks for the review on these patches. I will put the __mcount_loc in own block and send next version. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] objtool/klp: Reject changed functions without an ftrace call site 2026-10-22 12:25 [PATCH 0/3] objtool/klp: Reject changed functions without an ftrace call site Puranjay Mohan 2026-10-22 12:25 ` [PATCH 1/3] objtool/klp: Build test fixtures with ftrace Puranjay Mohan @ 2026-10-22 12:25 ` Puranjay Mohan 2026-10-22 15:55 ` Joe Lawrence 2026-10-22 12:25 ` [PATCH 3/3] objtool/klp: Add test for " Puranjay Mohan 2 siblings, 1 reply; 8+ messages in thread From: Puranjay Mohan @ 2026-10-22 12:25 UTC (permalink / raw) To: live-patching Cc: Puranjay Mohan, Josh Poimboeuf, Peter Zijlstra, Song Liu, Miroslav Benes, Joe Lawrence Livepatch redirects a function through its ftrace call site. If a changed function doesn't have one, klp-build succeeds but the patch module fails to load: livepatch: failed to find location for function 'nsproxy_free' livepatch: failed to patch object 'vmlinux' livepatch: failed to enable patch 'livepatch_nsproxy' This is easy to hit because the kernel's 'inline' implies notrace, so any inline function which the compiler emits out of line can't be patched. Make klp diff fail for any changed function which has no __mcount_loc entry in the original object. Signed-off-by: Puranjay Mohan <puranjay@kernel.org> --- tools/objtool/include/objtool/elf.h | 1 + tools/objtool/klp-diff.c | 47 +++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h index a82517a76a0f6..f7e6a652ee788 100644 --- a/tools/objtool/include/objtool/elf.h +++ b/tools/objtool/include/objtool/elf.h @@ -98,6 +98,7 @@ struct symbol { u8 klp : 1; u8 dont_correlate : 1; u8 fake : 1; + u8 traceable : 1; struct list_head pv_target; struct reloc *relocs; struct section *group_sec; diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c index 34da55e456097..f8659950ec9a9 100644 --- a/tools/objtool/klp-diff.c +++ b/tools/objtool/klp-diff.c @@ -1098,6 +1098,50 @@ static int mark_changed_functions(struct elfs *e) return !changed ? 1 : 0; } +/* + * Like ftrace_location(), accept an ftrace call site anywhere in the function. + */ +static void mark_traceable_functions(struct elf *elf) +{ + struct section *sec; + struct reloc *reloc; + + sec = find_section_by_name(elf, "__mcount_loc"); + if (!sec || !sec->rsec) + return; + + for_each_reloc(sec->rsec, reloc) { + struct symbol *func; + + func = find_func_containing(reloc->sym->sec, + reloc->sym->offset + reloc_addend(reloc)); + if (func) + func->traceable = 1; + } +} + +/* + * Livepatch redirects functions through their ftrace call sites, so a function + * without one can't be patched. + */ +static int validate_traceable_functions(struct elfs *e) +{ + struct symbol *patched_sym; + bool error = false; + + mark_traceable_functions(e->orig); + + for_each_sym(e->patched, patched_sym) { + if (!patched_sym->changed || patched_sym->twin->traceable) + continue; + + ERROR("%s: no ftrace call site, can't patch", patched_sym->twin->name); + error = true; + } + + return error ? -1 : 0; +} + static int clone_included_functions(struct elfs *e) { struct symbol *patched_sym; @@ -2260,6 +2304,9 @@ int cmd_klp_diff(int argc, const char **argv) if (ret > 0) return 0; + if (validate_traceable_functions(&e)) + return -1; + e.out = elf_create_file(&e.orig->ehdr, argv[2]); if (!e.out) return -1; -- 2.53.0-Meta ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] objtool/klp: Reject changed functions without an ftrace call site 2026-10-22 12:25 ` [PATCH 2/3] objtool/klp: Reject changed functions without an ftrace call site Puranjay Mohan @ 2026-10-22 15:55 ` Joe Lawrence 0 siblings, 0 replies; 8+ messages in thread From: Joe Lawrence @ 2026-10-22 15:55 UTC (permalink / raw) To: Puranjay Mohan Cc: live-patching, Josh Poimboeuf, Peter Zijlstra, Song Liu, Miroslav Benes On Thu, Oct 22, 2026 at 05:25:37AM -0700, Puranjay Mohan wrote: > Livepatch redirects a function through its ftrace call site. If a > changed function doesn't have one, klp-build succeeds but the patch > module fails to load: > > livepatch: failed to find location for function 'nsproxy_free' > livepatch: failed to patch object 'vmlinux' > livepatch: failed to enable patch 'livepatch_nsproxy' > > This is easy to hit because the kernel's 'inline' implies notrace, so > any inline function which the compiler emits out of line can't be > patched. > > Make klp diff fail for any changed function which has no __mcount_loc > entry in the original object. > > Signed-off-by: Puranjay Mohan <puranjay@kernel.org> > --- > tools/objtool/include/objtool/elf.h | 1 + > tools/objtool/klp-diff.c | 47 +++++++++++++++++++++++++++++ > 2 files changed, 48 insertions(+) > > diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h > index a82517a76a0f6..f7e6a652ee788 100644 > --- a/tools/objtool/include/objtool/elf.h > +++ b/tools/objtool/include/objtool/elf.h > @@ -98,6 +98,7 @@ struct symbol { > u8 klp : 1; > u8 dont_correlate : 1; > u8 fake : 1; > + u8 traceable : 1; > struct list_head pv_target; > struct reloc *relocs; > struct section *group_sec; > diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c > index 34da55e456097..f8659950ec9a9 100644 > --- a/tools/objtool/klp-diff.c > +++ b/tools/objtool/klp-diff.c > @@ -1098,6 +1098,50 @@ static int mark_changed_functions(struct elfs *e) > return !changed ? 1 : 0; > } > > +/* > + * Like ftrace_location(), accept an ftrace call site anywhere in the function. > + */ > +static void mark_traceable_functions(struct elf *elf) > +{ > + struct section *sec; > + struct reloc *reloc; > + > + sec = find_section_by_name(elf, "__mcount_loc"); > + if (!sec || !sec->rsec) > + return; > + > + for_each_reloc(sec->rsec, reloc) { > + struct symbol *func; > + > + func = find_func_containing(reloc->sym->sec, > + reloc->sym->offset + reloc_addend(reloc)); > + if (func) > + func->traceable = 1; > + } > +} > + > +/* > + * Livepatch redirects functions through their ftrace call sites, so a function > + * without one can't be patched. > + */ > +static int validate_traceable_functions(struct elfs *e) > +{ > + struct symbol *patched_sym; > + bool error = false; > + > + mark_traceable_functions(e->orig); > + > + for_each_sym(e->patched, patched_sym) { > + if (!patched_sym->changed || patched_sym->twin->traceable) > + continue; > + > + ERROR("%s: no ftrace call site, can't patch", patched_sym->twin->name); > + error = true; > + } > + > + return error ? -1 : 0; > +} > + > static int clone_included_functions(struct elfs *e) > { > struct symbol *patched_sym; > @@ -2260,6 +2304,9 @@ int cmd_klp_diff(int argc, const char **argv) > if (ret > 0) > return 0; > > + if (validate_traceable_functions(&e)) > + return -1; > + > e.out = elf_create_file(&e.orig->ehdr, argv[2]); > if (!e.out) > return -1; > -- > 2.53.0-Meta > > Acked-by: Joe Lawrence <joe.lawrence@redhat.com> -- Joe ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] objtool/klp: Add test for changed functions without an ftrace call site 2026-10-22 12:25 [PATCH 0/3] objtool/klp: Reject changed functions without an ftrace call site Puranjay Mohan 2026-10-22 12:25 ` [PATCH 1/3] objtool/klp: Build test fixtures with ftrace Puranjay Mohan 2026-10-22 12:25 ` [PATCH 2/3] objtool/klp: Reject changed functions without an ftrace call site Puranjay Mohan @ 2026-10-22 12:25 ` Puranjay Mohan 2026-10-22 16:02 ` Joe Lawrence 2 siblings, 1 reply; 8+ messages in thread From: Puranjay Mohan @ 2026-10-22 12:25 UTC (permalink / raw) To: live-patching Cc: Puranjay Mohan, Josh Poimboeuf, Peter Zijlstra, Song Liu, Miroslav Benes, Joe Lawrence Test that klp diff rejects a changed function without an ftrace call site, both a notrace one and an inline one emitted out of line, and reports each of them. Also test that a fully inlined function and an ordinary function can still be patched. Signed-off-by: Puranjay Mohan <puranjay@kernel.org> --- .../tests/x86/fixtures/notrace_target.c | 71 ++++++++++++++++++ .../objtool/tests/x86/test-notrace-target.sh | 74 +++++++++++++++++++ 2 files changed, 145 insertions(+) create mode 100644 tools/objtool/tests/x86/fixtures/notrace_target.c create mode 100755 tools/objtool/tests/x86/test-notrace-target.sh diff --git a/tools/objtool/tests/x86/fixtures/notrace_target.c b/tools/objtool/tests/x86/fixtures/notrace_target.c new file mode 100644 index 0000000000000..64da58c872b8a --- /dev/null +++ b/tools/objtool/tests/x86/fixtures/notrace_target.c @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: GPL-2.0 +/* PATCH_<name> changes <name> in the patched build. */ +#define notrace __attribute__((__no_instrument_function__)) + +static const char __modinfo[] + __attribute__((section(".modinfo"), used, aligned(1))) = "\0name=vmlinux"; + +#ifndef PATCHED +#undef PATCH_OUTLINED +#undef PATCH_NOTRACE +#undef PATCH_INLINED +#undef PATCH_TRACED +#endif + +#ifdef PATCH_OUTLINED +#define OUTLINED_K 2 +#else +#define OUTLINED_K 1 +#endif + +#ifdef PATCH_NOTRACE +#define NOTRACE_K 2 +#else +#define NOTRACE_K 1 +#endif + +#ifdef PATCH_INLINED +#define INLINED_K 2 +#else +#define INLINED_K 1 +#endif + +#ifdef PATCH_TRACED +#define TRACED_K 2 +#else +#define TRACED_K 1 +#endif + +int sink; + +/* Address taken, so emitted out of line. */ +static inline notrace int outlined(int x) +{ + int i; + + for (i = 0; i < x; i++) + sink += i * OUTLINED_K; + return sink; +} + +int (*outlined_ptr)(int) = outlined; + +notrace int notrace_fn(int x) +{ + return x * 3 + NOTRACE_K; +} + +static inline int inlined(int x) +{ + return x + INLINED_K; +} + +int inlined_user(int x) +{ + return inlined(x) * 5; +} + +int traced(int x) +{ + return x * 7 + TRACED_K; +} diff --git a/tools/objtool/tests/x86/test-notrace-target.sh b/tools/objtool/tests/x86/test-notrace-target.sh new file mode 100755 index 0000000000000..09414ae65fc2c --- /dev/null +++ b/tools/objtool/tests/x86/test-notrace-target.sh @@ -0,0 +1,74 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# +# A changed function without an ftrace call site can't be livepatched: +# klp_enable_patch() fails with "failed to find location for function". +# klp diff must reject it instead. + +. "$(dirname "$0")/../lib.sh" + +# mcount_entry <function>: orig.o has an __mcount_loc entry for it. +mcount_entry() +{ + in_relocs "$orig_obj" | awk '/rela__mcount_loc/,/^$/' | + awk -v f="$1" '$5 == f || $5 == ".text." f' | grep -q . +} + +# check <expected exit status> <-DPATCH_... flags> +check() +{ + local expect="$1"; shift + + setup + build_pair notrace_target.c "$@" + + assert_input_symbol outlined + assert_input_symbol notrace_fn + has_input_symbol "$orig_obj" inlined && + fail "inlined() was emitted out of line; the fixture tests nothing" + mcount_entry traced || + fail "no __mcount_loc entry for traced(); the fixture tests nothing" + mcount_entry inlined_user || + fail "no __mcount_loc entry for inlined_user(); the fixture tests nothing" + mcount_entry outlined && + fail "outlined() has a __mcount_loc entry; it should be notrace" + mcount_entry notrace_fn && + fail "notrace_fn() has a __mcount_loc entry; it should be notrace" + + run_diff "$expect" +} + +rejected() +{ + local n + + n="$(diff_log | grep -c "error: objtool: $1: no ftrace call site, can't patch")" + [ "$n" = 1 ] || + fail "expected '$1' rejected once, found $n: $(diff_log | tail -2)" +} + +check 255 -DPATCH_OUTLINED +rejected outlined +cleanup + +check 255 -DPATCH_NOTRACE +rejected notrace_fn +cleanup + +check 255 -DPATCH_OUTLINED -DPATCH_NOTRACE +rejected outlined +rejected notrace_fn +cleanup + +# Fully inlined: the traceable caller gets patched. +check 0 -DPATCH_INLINED +assert_patched inlined_user +assert_not_patched traced +cleanup + +check 0 -DPATCH_TRACED +assert_patched traced +assert_not_patched inlined_user +cleanup + +pass "changed function without an ftrace call site rejected" -- 2.53.0-Meta ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] objtool/klp: Add test for changed functions without an ftrace call site 2026-10-22 12:25 ` [PATCH 3/3] objtool/klp: Add test for " Puranjay Mohan @ 2026-10-22 16:02 ` Joe Lawrence 0 siblings, 0 replies; 8+ messages in thread From: Joe Lawrence @ 2026-10-22 16:02 UTC (permalink / raw) To: Puranjay Mohan Cc: live-patching, Josh Poimboeuf, Peter Zijlstra, Song Liu, Miroslav Benes On Thu, Oct 22, 2026 at 05:25:38AM -0700, Puranjay Mohan wrote: > Test that klp diff rejects a changed function without an ftrace call > site, both a notrace one and an inline one emitted out of line, and > reports each of them. Also test that a fully inlined function and an > ordinary function can still be patched. > > Signed-off-by: Puranjay Mohan <puranjay@kernel.org> > --- > .../tests/x86/fixtures/notrace_target.c | 71 ++++++++++++++++++ > .../objtool/tests/x86/test-notrace-target.sh | 74 +++++++++++++++++++ > 2 files changed, 145 insertions(+) > create mode 100644 tools/objtool/tests/x86/fixtures/notrace_target.c > create mode 100755 tools/objtool/tests/x86/test-notrace-target.sh > > diff --git a/tools/objtool/tests/x86/fixtures/notrace_target.c b/tools/objtool/tests/x86/fixtures/notrace_target.c > new file mode 100644 > index 0000000000000..64da58c872b8a > --- /dev/null > +++ b/tools/objtool/tests/x86/fixtures/notrace_target.c > @@ -0,0 +1,71 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* PATCH_<name> changes <name> in the patched build. */ > +#define notrace __attribute__((__no_instrument_function__)) > + > +static const char __modinfo[] > + __attribute__((section(".modinfo"), used, aligned(1))) = "\0name=vmlinux"; > + > +#ifndef PATCHED > +#undef PATCH_OUTLINED > +#undef PATCH_NOTRACE > +#undef PATCH_INLINED > +#undef PATCH_TRACED > +#endif > + > +#ifdef PATCH_OUTLINED > +#define OUTLINED_K 2 > +#else > +#define OUTLINED_K 1 > +#endif > + > +#ifdef PATCH_NOTRACE > +#define NOTRACE_K 2 > +#else > +#define NOTRACE_K 1 > +#endif > + > +#ifdef PATCH_INLINED > +#define INLINED_K 2 > +#else > +#define INLINED_K 1 > +#endif > + > +#ifdef PATCH_TRACED > +#define TRACED_K 2 > +#else > +#define TRACED_K 1 > +#endif > + > +int sink; > + > +/* Address taken, so emitted out of line. */ > +static inline notrace int outlined(int x) > +{ > + int i; > + > + for (i = 0; i < x; i++) > + sink += i * OUTLINED_K; > + return sink; > +} > + > +int (*outlined_ptr)(int) = outlined; > + > +notrace int notrace_fn(int x) > +{ > + return x * 3 + NOTRACE_K; > +} > + > +static inline int inlined(int x) > +{ > + return x + INLINED_K; > +} > + > +int inlined_user(int x) > +{ > + return inlined(x) * 5; > +} > + > +int traced(int x) > +{ > + return x * 7 + TRACED_K; > +} > diff --git a/tools/objtool/tests/x86/test-notrace-target.sh b/tools/objtool/tests/x86/test-notrace-target.sh > new file mode 100755 > index 0000000000000..09414ae65fc2c > --- /dev/null > +++ b/tools/objtool/tests/x86/test-notrace-target.sh > @@ -0,0 +1,74 @@ > +#!/bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# > +# A changed function without an ftrace call site can't be livepatched: > +# klp_enable_patch() fails with "failed to find location for function". > +# klp diff must reject it instead. > + > +. "$(dirname "$0")/../lib.sh" > + > +# mcount_entry <function>: orig.o has an __mcount_loc entry for it. > +mcount_entry() > +{ > + in_relocs "$orig_obj" | awk '/rela__mcount_loc/,/^$/' | > + awk -v f="$1" '$5 == f || $5 == ".text." f' | grep -q . > +} > + > +# check <expected exit status> <-DPATCH_... flags> > +check() > +{ > + local expect="$1"; shift > + > + setup > + build_pair notrace_target.c "$@" > + > + assert_input_symbol outlined > + assert_input_symbol notrace_fn > + has_input_symbol "$orig_obj" inlined && > + fail "inlined() was emitted out of line; the fixture tests nothing" > + mcount_entry traced || > + fail "no __mcount_loc entry for traced(); the fixture tests nothing" > + mcount_entry inlined_user || > + fail "no __mcount_loc entry for inlined_user(); the fixture tests nothing" > + mcount_entry outlined && > + fail "outlined() has a __mcount_loc entry; it should be notrace" > + mcount_entry notrace_fn && > + fail "notrace_fn() has a __mcount_loc entry; it should be notrace" > + > + run_diff "$expect" > +} > + > +rejected() > +{ > + local n > + > + n="$(diff_log | grep -c "error: objtool: $1: no ftrace call site, can't patch")" > + [ "$n" = 1 ] || > + fail "expected '$1' rejected once, found $n: $(diff_log | tail -2)" > +} > + > +check 255 -DPATCH_OUTLINED > +rejected outlined > +cleanup > + > +check 255 -DPATCH_NOTRACE > +rejected notrace_fn > +cleanup > + > +check 255 -DPATCH_OUTLINED -DPATCH_NOTRACE > +rejected outlined > +rejected notrace_fn > +cleanup > + > +# Fully inlined: the traceable caller gets patched. > +check 0 -DPATCH_INLINED > +assert_patched inlined_user > +assert_not_patched traced > +cleanup > + > +check 0 -DPATCH_TRACED > +assert_patched traced > +assert_not_patched inlined_user > +cleanup > + > +pass "changed function without an ftrace call site rejected" > -- > 2.53.0-Meta > > Thanks for finding and fixing this bug! By coincidence, I was debugging this very problem yesterday (wondering why I couldn't load a patch that modified an un-patchable function). Acked-by: Joe Lawrence <joe.lawrence@redhat.com> -- Joe ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-10-22 16:43 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-22 12:25 [PATCH 0/3] objtool/klp: Reject changed functions without an ftrace call site Puranjay Mohan 2026-10-22 12:25 ` [PATCH 1/3] objtool/klp: Build test fixtures with ftrace Puranjay Mohan 2026-10-22 14:46 ` Joe Lawrence 2026-10-22 16:43 ` Puranjay Mohan 2026-10-22 12:25 ` [PATCH 2/3] objtool/klp: Reject changed functions without an ftrace call site Puranjay Mohan 2026-10-22 15:55 ` Joe Lawrence 2026-10-22 12:25 ` [PATCH 3/3] objtool/klp: Add test for " Puranjay Mohan 2026-10-22 16:02 ` Joe Lawrence
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox