mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Longjun Luo <luolongjuna@gmail.com>
To: jpoimboe@kernel.org, peterz@infradead.org
Cc: mbenes@suse.cz, pmladek@suse.com, joe.lawrence@redhat.com,
	song@kernel.org, puranjay@kernel.org, nathan@kernel.org,
	ndesaulniers@google.com, morbo@google.com,
	justinstitt@google.com, live-patching@vger.kernel.org,
	llvm@lists.linux.dev, linux-kernel@vger.kernel.org,
	Longjun Luo <luolongjuna@gmail.com>
Subject: [RFC PATCH v2 01/11] objtool/klp: Fix checksums and copying of wide strings
Date: Wed,  4 Nov 2026 21:31:47 +0800	[thread overview]
Message-ID: <20261104133158.3187525-2-luolongjuna@gmail.com> (raw)
In-Reply-To: <20261104133158.3187525-1-luolongjuna@gmail.com>

SHF_STRINGS sections can contain elements wider than one byte. Using
strlen() truncates their contents at an embedded zero byte, hiding
changes and copying incomplete strings into the patch.

Find the terminating zero element using sh_entsize. Use the resulting
length for both reference checksums and copying, and propagate errors
through checksum calculation. Keep single-byte string handling unchanged.

Add a test for two- and four-byte strings, including references from data.

Validated the available objtool tests with GCC 16.2.1 and Clang
22.1.8. No failures.

Fixes: 0d83da43b1e1 ("objtool/klp: Add --checksum option to generate per-function checksums")
Fixes: dd590d4d57eb ("objtool/klp: Introduce klp diff subcommand for diffing object files")
Fixes: 873a2208ea31 ("objtool/klp: Calculate object checksums")
Assisted-by: LLM
Signed-off-by: Longjun Luo <luolongjuna@gmail.com>
---
 tools/objtool/elf.c                           | 29 +++++++++
 tools/objtool/include/objtool/elf.h           |  1 +
 tools/objtool/klp-checksum.c                  | 44 ++++++++-----
 tools/objtool/klp-diff.c                      | 14 ++--
 .../tests/generic/fixtures/identity/common.h  | 15 +++++
 .../generic/fixtures/identity/wide_string.c   | 36 ++++++++++
 .../generic/test-checksum-wide-string.sh      | 39 +++++++++++
 tools/objtool/tests/identity-lib.sh           | 65 +++++++++++++++++++
 8 files changed, 223 insertions(+), 20 deletions(-)
 create mode 100644 tools/objtool/tests/generic/fixtures/identity/common.h
 create mode 100644 tools/objtool/tests/generic/fixtures/identity/wide_string.c
 create mode 100755 tools/objtool/tests/generic/test-checksum-wide-string.sh
 create mode 100644 tools/objtool/tests/identity-lib.sh

diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index a791f4ea6ec1..71f1a272e468 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -1382,6 +1382,35 @@ struct elf *elf_create_file(GElf_Ehdr *ehdr, const char *name)
 	return elf;
 }
 
+/* Wide strings, such as the u16 PSR setup table, end at an all-zero element. */
+ssize_t elf_string_len(struct section *sec, unsigned long offset)
+{
+	size_t entsize = sec->sh.sh_entsize, size, len;
+	const char *str;
+
+	/*
+	 * FIXME: GCC's range_string() uses a one-past-end string pointer.
+	 * strlen() reads past the section; preserve the string base and addend.
+	 */
+	if (entsize == 1)
+		return strlen(sec->data->d_buf + offset);
+
+	if (!entsize || offset >= sec_size(sec)) {
+		ERROR("%s+%#lx: invalid string offset or character size", sec->name, offset);
+		return -1;
+	}
+
+	str = sec->data->d_buf + offset;
+	size = sec_size(sec) - offset;
+	for (len = 0; size - len >= entsize; len += entsize) {
+		if (!memchr_inv(str + len, 0, entsize))
+			return len;
+	}
+
+	ERROR("%s+%#lx: unterminated string", sec->name, offset);
+	return -1;
+}
+
 unsigned int elf_add_string(struct elf *elf, struct section *strtab, const char *str)
 {
 	unsigned int offset;
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index a82517a76a0f..5d7ddadf9471 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -179,6 +179,7 @@ struct symbol *elf_create_section_symbol(struct elf *elf, struct section *sec);
 void *elf_add_data(struct elf *elf, struct section *sec, const void *data,
 		   size_t size);
 
+ssize_t elf_string_len(struct section *sec, unsigned long offset);
 unsigned int elf_add_string(struct elf *elf, struct section *strtab, const char *str);
 
 struct reloc *elf_create_reloc(struct elf *elf, struct section *sec,
diff --git a/tools/objtool/klp-checksum.c b/tools/objtool/klp-checksum.c
index ebe25f9c5260..efaf083c03a1 100644
--- a/tools/objtool/klp-checksum.c
+++ b/tools/objtool/klp-checksum.c
@@ -67,8 +67,8 @@ static bool is_anonymous_const_data(struct symbol *sym)
 	       (is_sec_sym(sym) || strstarts(sym->name, ".LC"));
 }
 
-static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
-				 struct instruction *insn)
+static int checksum_update_insn(struct objtool_file *file, struct symbol *func,
+				struct instruction *insn)
 {
 	struct reloc *reloc = insn_reloc(file, insn);
 	struct alternative *alt;
@@ -77,7 +77,7 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
 	static bool in_alt;
 
 	if (insn->fake)
-		return;
+		return 0;
 
 	if (!reloc) {
 		struct symbol *call_dest = insn_call_dest(insn);
@@ -135,10 +135,13 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
 	offset = arch_insn_adjusted_addend(insn, reloc);
 
 	if (is_string_sec(sym->sec)) {
-		char *str;
+		ssize_t size = elf_string_len(sym->sec, sym->offset + offset);
+		const void *data;
 
-		str = sym->sec->data->d_buf + sym->offset + offset;
-		__checksum_update_insn(func, insn, str, strlen(str));
+		if (size < 0)
+			return -1;
+		data = sym->sec->data->d_buf + sym->offset + offset;
+		__checksum_update_insn(func, insn, data, size);
 		goto alts;
 	}
 
@@ -180,19 +183,22 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
 			__checksum_update_insn(func, insn, &alt_group->feature,sizeof(alt_group->feature));
 
 			for (alt_insn = alt->insn; alt_insn; alt_insn = next_insn_same_sec(file, alt_insn)) {
-				checksum_update_insn(file, func, alt_insn);
+				if (checksum_update_insn(file, func, alt_insn))
+					return -1;
 				if (!alt_group->last_insn || alt_insn == alt_group->last_insn)
 					break;
 			}
 		} else {
-			checksum_update_insn(file, func, alt->insn);
+			if (checksum_update_insn(file, func, alt->insn))
+				return -1;
 		}
 
 		in_alt = false;
 	}
+	return 0;
 }
 
-static void checksum_update_object(struct objtool_file *file, struct symbol *sym)
+static int checksum_update_object(struct objtool_file *file, struct symbol *sym)
 {
 	struct reloc *reloc;
 
@@ -211,11 +217,13 @@ static void checksum_update_object(struct objtool_file *file, struct symbol *sym
 		offset = reloc_addend(reloc);
 
 		if (is_string_sec(target->sec)) {
-			char *str;
+			ssize_t size = elf_string_len(target->sec, target->offset + offset);
+			const void *data;
 
-			str = target->sec->data->d_buf + target->offset + offset;
-			__checksum_update_object(sym, sym_offset,
-						 "reloc string", str, strlen(str));
+			if (size < 0)
+				return -1;
+			data = target->sec->data->d_buf + target->offset + offset;
+			__checksum_update_object(sym, sym_offset, "reloc string", data, size);
 			continue;
 		}
 
@@ -233,6 +241,7 @@ static void checksum_update_object(struct objtool_file *file, struct symbol *sym
 		__checksum_update_object(sym, sym_offset, "reloc addend",
 					 &offset, sizeof(offset));
 	}
+	return 0;
 }
 
 int calculate_checksums(struct objtool_file *file)
@@ -256,13 +265,16 @@ int calculate_checksums(struct objtool_file *file)
 
 		if (is_func_sym(sym)) {
 			checksum_init(sym);
-			func_for_each_insn(file, sym, insn)
-				checksum_update_insn(file, sym, insn);
+			func_for_each_insn(file, sym, insn) {
+				if (checksum_update_insn(file, sym, insn))
+					return -1;
+			}
 			checksum_finish(sym);
 
 		} else if (is_object_sym(sym)) {
 			checksum_init(sym);
-			checksum_update_object(file, sym);
+			if (checksum_update_object(file, sym))
+				return -1;
 			checksum_finish(sym);
 		}
 
diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 34da55e45609..6321f691adbb 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -1548,12 +1548,18 @@ static int clone_reloc(struct elfs *e, struct reloc *patched_reloc,
 	 * version of the string section.  Now copy the string itself.
 	 */
 	if (is_string_sec(patched_sym->sec)) {
-		const char *str = patched_sym->sec->data->d_buf + addend;
+		ssize_t len = elf_string_len(patched_sym->sec, addend);
+		const char *str;
 
-		__dbg_clone("\"%s\"", escape_str(str));
+		if (len < 0)
+			return -1;
+		str = patched_sym->sec->data->d_buf + addend;
+		if (patched_sym->sec->sh.sh_entsize == 1)
+			__dbg_clone("\"%s\"", escape_str(str));
 
-		addend = elf_add_string(e->out, out_sym->sec, str);
-		if (addend == -1)
+		addend = ALIGN(sec_size(out_sym->sec), out_sym->sec->sh.sh_addralign);
+		if (!elf_add_data(e->out, out_sym->sec, str,
+				  len + patched_sym->sec->sh.sh_entsize))
 			return -1;
 	}
 
diff --git a/tools/objtool/tests/generic/fixtures/identity/common.h b/tools/objtool/tests/generic/fixtures/identity/common.h
new file mode 100644
index 000000000000..d0f56751fcd9
--- /dev/null
+++ b/tools/objtool/tests/generic/fixtures/identity/common.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */
+#ifndef KLP_TEST_IDENTITY_H
+#define KLP_TEST_IDENTITY_H
+
+#ifdef __clang__
+/* Use noinline on Clang to keep calls visible in these tests. */
+#define NOIPA __attribute__((noinline))
+#else
+#define NOIPA __attribute__((noipa))
+#endif
+#define MODINFO static const char modinfo[] \
+	__attribute__((used, section(".modinfo"))) = "name=vmlinux"
+
+#endif
diff --git a/tools/objtool/tests/generic/fixtures/identity/wide_string.c b/tools/objtool/tests/generic/fixtures/identity/wide_string.c
new file mode 100644
index 000000000000..4b25781e1827
--- /dev/null
+++ b/tools/objtool/tests/generic/fixtures/identity/wide_string.c
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */
+#include "common.h"
+MODINFO;
+
+#ifdef WIDE32
+#ifdef PATCHED
+#define TEXT U"\x014a\x0113\x00dc\x00a5\x006e\x0038"
+#else
+#define TEXT U"\x014a\x0113\x00dc\x00a5\x006e\x0037"
+#endif
+#else
+#ifdef PATCHED
+#define TEXT u"\x014a\x0113\x00dc\x00a5\x006e\x0038"
+#else
+#define TEXT u"\x014a\x0113\x00dc\x00a5\x006e\x0037"
+#endif
+#endif
+
+#ifdef OBJECT
+struct descriptor {
+	const void *text;
+};
+
+const struct descriptor descriptor = { TEXT };
+
+NOIPA const struct descriptor *target(void)
+{
+	return &descriptor;
+}
+#else
+NOIPA const void *target(void)
+{
+	return TEXT;
+}
+#endif
diff --git a/tools/objtool/tests/generic/test-checksum-wide-string.sh b/tools/objtool/tests/generic/test-checksum-wide-string.sh
new file mode 100755
index 000000000000..426adc42337a
--- /dev/null
+++ b/tools/objtool/tests/generic/test-checksum-wide-string.sh
@@ -0,0 +1,39 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com>
+# Wide strings contain zero bytes before their terminating zero element.
+. "$(dirname "$0")/../identity-lib.sh"
+
+wide_section()
+{
+	in_sections "$1" | sed 's/^ *\[[ 0-9]*\] *//' |
+		awk -v width="$width" '$7 ~ /S/ && $6+0 == width { print $1; exit }'
+}
+
+setup
+for width in 2 4; do
+	flags=()
+	[ "$width" = 4 ] && flags=(-DWIDE32)
+	build_pair identity/wide_string.c "${flags[@]}"
+	input="$(wide_section patched.o)"
+	[ -n "$input" ] || fail "missing $width-byte string section"
+
+	assert_checksum_differs target
+	run_diff
+	assert_patched target
+	output="$(wide_section out.o)"
+	[ -n "$output" ] || fail "wide string was not copied"
+	$OBJCOPY -O binary --only-section="$input" "$workdir/patched.o" "$workdir/expected.bin" ||
+		fail "cannot extract input string"
+	$OBJCOPY -O binary --only-section="$output" "$workdir/out.o" "$workdir/actual.bin" ||
+		fail "cannot extract output string"
+	cmp -s "$workdir/expected.bin" "$workdir/actual.bin" || fail "wide string was truncated"
+	partial_link "$workdir/linked.o" "$workdir/out.o" || fail "wide-string output does not link"
+
+	build_pair identity/wide_string.c "${flags[@]}" -DOBJECT
+	assert_checksum_differs descriptor
+	run_diff 255
+	assert_diff_log 'changed data: descriptor'
+done
+
+pass "wide-string checksums and copies cover every element"
diff --git a/tools/objtool/tests/identity-lib.sh b/tools/objtool/tests/identity-lib.sh
new file mode 100644
index 000000000000..fc7014a88bc8
--- /dev/null
+++ b/tools/objtool/tests/identity-lib.sh
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com>
+# Shared build helpers for the multi-TU identity fixtures.
+. "$(dirname "$0")/../lib.sh"
+
+FIXTURE_CFLAGS="$FIXTURE_CFLAGS -fno-pie -fno-pic"
+
+# build_identity_pair <none|thin|full> <extra-cflags> <fixture.c...>
+build_identity_pair()
+{
+	local mode="$1" cflags="$2" side file i
+	local CC="$CC"
+	local flags=() parts=() patched=()
+	shift 2
+	if [ "$mode" != none ]; then
+		clang_only "LTO fixtures require clang"
+		find_thinlto_toolchain || probe_skip "no matching clang/lld pair"
+		flags=("-flto=$mode")
+		CC="$THIN_CC"
+	fi
+	orig_obj=orig.o
+	patched_obj=patched.o
+	for side in orig patched; do
+		patched=()
+		[ "$side" = patched ] && patched=(-DPATCHED)
+		parts=()
+		i=0
+		for file in "$@"; do
+			build_one "$file" "$side-$i.o" "${flags[@]}" "${patched[@]}" $cflags
+			parts+=("$workdir/$side-$i.o")
+			i=$((i + 1))
+		done
+		if [ "$mode" = none ]; then
+			partial_link "$workdir/$side.o" "${parts[@]}" || fail "partial link failed"
+		else
+			"$THIN_LD" -r "${parts[@]}" -o "$workdir/$side.o" || fail "LTO link failed"
+		fi
+	done
+}
+
+# identity_relocs <object> <relocation-section>
+identity_relocs()
+{
+	# readelf prints the section name in single quotes.
+	in_relocs "$1" | awk -v sec="'$2'" '
+		/Relocation section/ { active = $3 == sec }
+		active && $1 ~ /^[[:xdigit:]]+$/ { print }'
+}
+
+# The fixture's function references one static in .bss, .data or .rodata.
+identity_object_for()
+{
+	identity_relocs "$1" ".rela.text.$2" | awk '
+		$5 ~ /^\.(bss|data|rodata)\./ {
+			sub(/^\.(bss|data|rodata)\./, "", $5); print $5; exit
+		}'
+}
+
+# After post-link, check the reference to the original symbol, not just its presence.
+assert_original_reference()
+{
+	identity_relocs out.o ".klp.rela.orig..text.$1" |
+		awk -v n=".klp.sym.orig.$2," 'index($5, n) == 1' | grep -q . ||
+		fail "expected '$1' to reference original '$2'"
+}
-- 
2.55.0


  reply	other threads:[~2026-11-04 13:32 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-04 13:31 [RFC PATCH v2 00/11] objtool/klp: DWARF-based cross-build symbol correlation Longjun Luo
2026-11-04 13:31 ` Longjun Luo [this message]
2026-11-04 13:31 ` [RFC PATCH v2 02/11] objtool/klp: Hash compound literals by content Longjun Luo
2026-11-04 13:31 ` [RFC PATCH v2 03/11] objtool/klp: Preserve identifier digits when stripping suffixes Longjun Luo
2026-11-04 13:31 ` [RFC PATCH v2 04/11] objtool/klp: Read symbol identities from DWARF Longjun Luo
2026-11-04 13:31 ` [RFC PATCH v2 05/11] objtool/klp: Support reduced and split DWARF Longjun Luo
2026-11-04 13:31 ` [RFC PATCH v2 06/11] klp-build: Require CONFIG_DEBUG_INFO Longjun Luo
2026-11-04 13:31 ` [RFC PATCH v2 07/11] objtool/klp: Correlate symbols using DWARF identities Longjun Luo
2026-11-04 13:31 ` [RFC PATCH v2 08/11] objtool/klp: Include symbol identities in reference checksums Longjun Luo
2026-11-04 13:31 ` [RFC PATCH v2 09/11] objtool/klp: Extend twins through unchanged function references Longjun Luo
2026-11-04 13:31 ` [RFC PATCH v2 10/11] objtool/klp: Check unmatched data references Longjun Luo
2026-11-04 13:31 ` [RFC PATCH v2 11/11] objtool/klp: Test symbol identity and state preservation Longjun Luo

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=20261104133158.3187525-2-luolongjuna@gmail.com \
    --to=luolongjuna@gmail.com \
    --cc=joe.lawrence@redhat.com \
    --cc=jpoimboe@kernel.org \
    --cc=justinstitt@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=mbenes@suse.cz \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=puranjay@kernel.org \
    --cc=song@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®