All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joe Lawrence <joe.lawrence@redhat.com>
To: Josh Poimboeuf <jpoimboe@kernel.org>
Cc: x86@kernel.org, linux-kernel@vger.kernel.org,
	live-patching@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>, Song Liu <song@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	Mark Rutland <mark.rutland@arm.com>,
	Miroslav Benes <mbenes@suse.cz>, Petr Mladek <pmladek@suse.com>
Subject: Re: [PATCH v3 14/21] objtool: Prevent kCFI hashes from being decoded as instructions
Date: Tue, 30 Jun 2026 14:40:42 -0400	[thread overview]
Message-ID: <akQNqlfFC0T5pcMa@redhat.com> (raw)
In-Reply-To: <b1d50c9fc9e6b9bca43833cc4ccbd88a31fed84b.1778642120.git.jpoimboe@kernel.org>

On Tue, May 12, 2026 at 08:33:48PM -0700, Josh Poimboeuf wrote:
> On arm64 with CONFIG_CFI=y, Clang places a 4-byte kCFI type hash
> immediately before each address-taken function entry.  Since these
> hashes are in the text section, objtool tries to decode them, leading to
> unpredictable results (e.g., "unannotated intra-function call").
> 
> arm64 uses mapping symbols to annotate where code ends and data begins
> (and vice versa).  Use those to just mark such "instructions" as NOP so
> objtool will ignore them.
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

Hi Josh,

While continuing down the klp-build unit test path, I found a bug in
kCFI special-section handling.  I don't think it's directly related to
this patch, though it was the motivation to try testing kCFI + klp-build
together.

This looks like the same Clang/klp-diff issue as commit f7ceffd21a8a
("objtool/klp: Fix kCFI prefix finding/cloning").  That commit fixed
prefix finding in .text, while this one is in .kcfi_traps and causes
create_fake_symbols() to skip the entire section, so
clone_special_sections() extracts nothing. (klp-build still exits
SUCCESS, but the livepatch .ko has no __kcfi_traps.)

That means da4326573ae8 ("objtool/klp: Fix kCFI trap handling"), which
added .kcfi_traps to the special-section list, would be incomplete for
the fake-symbol path.

Bug report as follows:


Kernel Config
=============

Setup LLVM and CFI, plus livepatching requirements on top of the default
x86 config:

  $ make clean
  $ make LLVM=1 defconfig

  # For livepatching
  $ ./scripts/config --file .config \
         --set-val CONFIG_FTRACE y \
         --set-val CONFIG_KALLSYMS_ALL y \
         --set-val CONFIG_FUNCTION_TRACER y \
         --set-val CONFIG_DYNAMIC_FTRACE y \
         --set-val CONFIG_DYNAMIC_DEBUG y \
         --set-val CONFIG_LIVEPATCH y

  # For CFI
  $ ./scripts/config --file .config \
         --set-val CONFIG_CFI y

  $ make LLVM=1 olddefconfigremotes/origin/objtool/urgent
  $ make LLVM=1 -j$(nproc)


Livepatch
=========

diff --git a/fs/proc/base.c b/fs/proc/base.c
index d9acfa89c894..6944d3f53847 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -809,6 +809,8 @@ static int proc_single_show(struct seq_file *m, void *v)
 	if (!task)
 		return -ESRCH;

+	pr_debug("test: proc_single_show\n");
+
 	ret = PROC_I(inode)->op.proc_show(m, ns, pid, task);

 	put_task_struct(task);


klp-build
=========

The klp-build looks good and exits with SUCCESS:

  $ LLVM=1 scripts/livepatch/klp-build -T klp-cfi-traps.patch 2>&1 | tee out
  Validating patch(es)
  Building original kernel
  Copying original object files
  Fixing patch(es)
  Building patched kernel
  Copying patched object files
  Generating original checksums
  Generating patched checksums
  Diffing objects
  vmlinux.o: changed function: proc_single_show
  Building patch module: livepatch-klp-cfi-traps.ko
  SUCCESS

The patched vmlinux.o contains a per-function .kcfi_traps section
associated with .text.proc_single_show, but klp-build does not copy the
traps section into the livepatch module:

  $ llvm-readelf --wide --sections klp-tmp/3-checksum-patched/vmlinux.o

  Section Headers:
  [Nr]     Name                               Type      Address           Off      Size    ES  Flg  Lk      Inf    Al
  [74992]  .text.proc_single_show             PROGBITS  0000000000000000  164d470  0000f5  00  AX   0       0      16
  [74993]  .kcfi_traps                        PROGBITS  0000000000000000  164d565  000004  00  AL   74992   0      1
  [74994]  __patchable_function_entries       PROGBITS  0000000000000000  164d570  000008  00  WAL  74992   0      8
  [74995]  .rela.text.proc_single_show        RELA      0000000000000000  164d578  000120  18  I    299696  74992  8
  [74996]  .rela.kcfi_traps                   RELA      0000000000000000  164d698  000018  18  I    299696  74993  8
  [74997]  .rela__patchable_function_entries  RELA      0000000000000000  164d6b0  000018  18  I    299696  74994  8

  $ llvm-readelf --wide --relocs klp-tmp/3-checksum-patched/vmlinux.o | \
      awk '$0 ~ "at offset 0x164d698" {p=1; print; next} /^Relocation section/ {p=0} p'
  Relocation section '.rela.kcfi_traps' at offset 0x164d698 contains 1 entries:
      Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
  0000000000000000  00016d6700000002 R_X86_64_PC32          0000000000000000 .text.proc_single_show + 6b

  $ llvm-readelf --wide --sections livepatch-klp-cfi-traps.ko  | grep .kcfi_traps
  (none)


The root cause is that create_fake_symbols() skips the entire special
section if any symbol exists at offset 0.  But Clang places a .Ltmp*
label at the start of .kcfi_traps, so no per-entry fake symbols are
created and clone_special_sections() extracts nothing.

  static int create_fake_symbols(struct elf *elf)
  {
  ...
  	/*
  	 * 2) Make symbols for sh_entsize, and simple arrays of pointers:
  	 */
  entsize:
  	for_each_sec(elf, sec) {
  		unsigned int entry_size;
  		unsigned long offset;

  		if (!is_special_section(sec) || find_symbol_by_offset(sec, 0))
  			continue;

  $ llvm-readelf --wide --symbols klp-tmp/3-checksum-patched/vmlinux.o | \
          awk '$7 == 74993'
   93266: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT   74993 .Ltmp78
   93544: 0000000000000000     0 SECTION LOCAL  DEFAULT   74993 .kcfi_traps


Possible fix: ignore .L* assembler-local labels at section offset 0
using the existing is_local_label() helper.

-->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index b9624bd9439b..4ba400926647 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -1860,8 +1860,17 @@ static int create_fake_symbols(struct elf *elf)
 	for_each_sec(elf, sec) {
 		unsigned int entry_size;
 		unsigned long offset;
+		struct symbol *sym_at_0;

-		if (!is_special_section(sec) || find_symbol_by_offset(sec, 0))
+		if (!is_special_section(sec))
+			continue;
+
+		/*
+		 * Clang may place assembler-local .L* labels at offset 0;
+		 * they must not prevent per-entry fake symbol creation.
+		 */
+		sym_at_0 = find_symbol_by_offset(sec, 0);
+		if (sym_at_0 && !is_local_label(sym_at_0))
 			continue;

 		if (!sec->rsec) {

-->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--

With that allowance, the section propagates through to the livepatch.ko:

  $ llvm-readelf --wide --sections livepatch-klp-cfi-traps.ko | grep kcfi_traps
    [ 5] __kcfi_traps      PROGBITS        0000000000000000 0000b8 000004 00  AL  0   0  1
    [27] .rela__kcfi_traps RELA            0000000000000000 001410 000018 18   I 51   5  8

  $ llvm-readelf --wide --relocs livepatch-klp-cfi-traps.ko | \
      awk '$0 ~ ".rela__kcfi_traps" {p=1; print; next} /^Relocation section/ {p=0} p'
  Relocation section '.rela__kcfi_traps' at offset 0x1410 contains 1 entries:
      Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
  0000000000000000  0000000b00000002 R_X86_64_PC32          0000000000000010 proc_single_show + 5b


Additionally, how about a follow-on patch that detects and warns when
special sections should have been included, but are missing for whatever
reason?  Something like:

-->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 4ba400926647..2e265d38259e 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -2029,12 +2029,33 @@ static int validate_special_section_klp_reloc(struct elfs *e, struct symbol *sym
 	return ret;
 }

+/* True if any relocation in sec references a changed (included) function. */
+static bool special_section_refs_included_func(struct elf *elf, struct section *sec)
+{
+	struct reloc *reloc;
+
+	if (!sec->rsec)
+		return false;
+
+	for_each_reloc(sec->rsec, reloc) {
+		if (convert_reloc_sym(elf, reloc))
+			continue;
+
+		if (reloc->sym->included && is_func_sym(reloc->sym))
+			return true;
+	}
+
+	return false;
+}
+
 static int clone_special_section(struct elfs *e, struct section *patched_sec)
 {
 	bool is_pfe = !strcmp(patremotes/origin/objtool/urgentched_sec->name, "__patchable_function_entries");
 	struct section *out_sec = NULL;
 	struct reloc *patched_reloc;
 	struct symbol *patched_sym;
+	unsigned int cloned = 0;
+	unsigned int skipped = 0;

 	/*
 	 * Extract all special section symbols (and their dependencies) which
@@ -2053,13 +2074,17 @@ static int clone_special_section(struct elfs *e, struct section *patched_sec)
 		ret = validate_special_section_klp_reloc(e, patched_sym);
 		if (ret < 0)
 			return -1;
-		if (ret > 0)
+		if (ret > 0) {
+			skipped++;
 			continue;
+		}

 		out_sym = clone_symbol(e, patched_sym, true);
 		if (!out_sym)
 			return -1;

+		cloned++;
+
 		if (!is_pfe || (out_sec && out_sec->sh.sh_link))
 			continue;

@@ -2075,6 +2100,19 @@ static int clone_special_section(struct elfs *e, struct section *patched_sec)
 		out_sec->sh.sh_link = patched_reloc->sym->clone->sec->idx;
 	}

+	/*
+	 * Detect extraction failures: the patched object references
+	 * changed functions from this section, but nothing was cloned and
+	 * nothing was intentionally skipped (e.g. disabled tracepoints).
+	 */
+	if (special_section_refs_included_func(e->patched, patched_sec) &&
+	    cloned == 0 && skipped == 0) {
+		out_sec = find_section_by_name(e->out, patched_sec->name);
+		if (!out_sec || !sec_size(out_sec))
+			WARN("%s: %s missing from output despite references to changed functions",
+			     objname, patched_sec->name);
+	}
+
 	return 0;
 }

-->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--
H
k
wHappy to spin the .L* fix out as a separate patch post based on your
tklp-build-arm64 or other branch (Fixes: da4326573ae8), with the
warn-on-empty-extraction as an optional follow-up if you'd like that
too.

--
Joe


  parent reply	other threads:[~2026-06-30 18:40 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-13  3:33 [PATCH v3 00/21] objtool/arm64: Port klp-build to arm64 Josh Poimboeuf
2026-05-13  3:33 ` Josh Poimboeuf
2026-05-13  3:33 ` [PATCH v3 01/21] klp-build: Reject patches to init/*.c Josh Poimboeuf
2026-05-13  3:33   ` Josh Poimboeuf
2026-05-15 21:16   ` Song Liu
2026-07-01 11:38   ` Miroslav Benes
2026-05-13  3:33 ` [PATCH v3 02/21] arm64: Annotate intra-function calls Josh Poimboeuf
2026-05-13  3:33   ` Josh Poimboeuf
2026-05-13  3:33     ` Josh Poimboeuf
2026-07-01 11:42     ` Miroslav Benes
2026-05-13  3:33 ` [PATCH v3 03/21] arm64: Fix EFI linking with -fdata-sections Josh Poimboeuf
2026-05-13  3:33   ` Josh Poimboeuf
2026-07-01 11:54   ` Miroslav Benes
2026-05-13  3:33 ` [PATCH v3 04/21] arm64: Rename TRAMP_VALIAS -> TRAMP_VALIAS_ASM in asm-offsets Josh Poimboeuf
2026-05-13  3:33   ` Josh Poimboeuf
2026-05-13  3:34     ` Josh Poimboeuf
2026-05-15 21:18     ` Song Liu
2026-07-01 11:54     ` Miroslav Benes
2026-05-13  3:33 ` [PATCH v3 05/21] arm64: vdso: Discard .discard.* sections Josh Poimboeuf
2026-05-13  3:34   ` Josh Poimboeuf
2026-05-13  3:33 ` [PATCH v3 06/21] arm64: Annotate special section entries Josh Poimboeuf
2026-05-13  3:34   ` Josh Poimboeuf
2026-05-13  3:33 ` [PATCH v3 07/21] crypto: arm64: Move data to .rodata Josh Poimboeuf
2026-05-13  3:34   ` Josh Poimboeuf
2026-05-13  3:33 ` [PATCH v3 08/21] objtool: Allow setting --mnop without --mcount Josh Poimboeuf
2026-05-13  3:34   ` Josh Poimboeuf
2026-05-13  3:33 ` [PATCH v3 09/21] kbuild: Only run objtool if there is at least one command Josh Poimboeuf
2026-05-13  3:34   ` Josh Poimboeuf
2026-05-14 22:25   ` sashiko-bot
2026-05-13  3:33 ` [PATCH v3 10/21] objtool: Ignore jumps to the end of the function for checksum runs Josh Poimboeuf
2026-05-13  3:33   ` Josh Poimboeuf
2026-05-13  3:34     ` Josh Poimboeuf
2026-05-13  7:36     ` Peter Zijlstra
2026-05-14 22:30     ` sashiko-bot
2026-05-13  3:33 ` [PATCH v3 11/21] objtool: Allow empty alternatives Josh Poimboeuf
2026-05-13  3:33   ` Josh Poimboeuf
2026-05-13  3:34     ` Josh Poimboeuf
2026-05-13  7:37     ` Peter Zijlstra
2026-05-13  3:33 ` [PATCH v3 12/21] objtool: Refactor elf_add_data() to use a growable data buffer Josh Poimboeuf
2026-05-13  3:34   ` Josh Poimboeuf
2026-05-14 23:13   ` sashiko-bot
2026-05-13  3:33 ` [PATCH v3 13/21] objtool: Reuse string references Josh Poimboeuf
2026-05-13  3:34   ` Josh Poimboeuf
2026-05-13  3:33 ` [PATCH v3 14/21] objtool: Prevent kCFI hashes from being decoded as instructions Josh Poimboeuf
2026-05-13  3:34   ` Josh Poimboeuf
2026-05-15  0:16   ` sashiko-bot
2026-06-30 18:40   ` Joe Lawrence [this message]
2026-05-13  3:33 ` [PATCH v3 15/21] objtool/klp: Add arm64 support for prefix/PFE detection Josh Poimboeuf
2026-05-13  3:34   ` Josh Poimboeuf
2026-05-13  3:33 ` [PATCH v3 16/21] objtool/klp: Filter arm64 mapping symbols in find_symbol_by_offset() Josh Poimboeuf
2026-05-13  3:34   ` Josh Poimboeuf
2026-05-15 21:20   ` Song Liu
2026-05-13  3:33 ` [PATCH v3 17/21] objtool/klp: Don't correlate arm64 mapping symbols Josh Poimboeuf
2026-05-13  3:34   ` Josh Poimboeuf
2026-05-15  1:19   ` sashiko-bot
2026-05-13  3:33 ` [PATCH v3 18/21] objtool/klp: Clone inline alternative replacements Josh Poimboeuf
2026-05-13  3:34   ` Josh Poimboeuf
2026-05-13  3:33 ` [PATCH v3 19/21] objtool/klp: Introduce objtool for arm64 Josh Poimboeuf
2026-05-13  3:34   ` Josh Poimboeuf
2026-05-15  2:08   ` sashiko-bot
2026-05-13  3:33 ` [PATCH v3 20/21] klp-build: Support cross-compilation Josh Poimboeuf
2026-05-13  3:34   ` Josh Poimboeuf
2026-05-13  3:33 ` [PATCH v3 21/21] klp-build: Add arm64 syscall patching macro Josh Poimboeuf
2026-05-13  3:34   ` Josh Poimboeuf
2026-05-15  2:44   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=akQNqlfFC0T5pcMa@redhat.com \
    --to=joe.lawrence@redhat.com \
    --cc=catalin.marinas@arm.com \
    --cc=jpoimboe@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mbenes@suse.cz \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=song@kernel.org \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.