Skip to contentSkip to conversation

objtool: CONFIG_OBJTOOL_WERROR fixes and cleanups

PATCH Applied Josh Poimboeuf 9 participants 75 messages Mar 24, 2025 – Mar 28, 2025 2 messages not in the archive

Subsystems (17)
Media (V4L/DVB) x86 Sound Kbuild +13 more subsystems
lore
Share & tools
lore

Josh Poimboeuf

author maintainer
#0 lore raw

cover objtool: CONFIG_OBJTOOL_WERROR fixes and cleanups

Since CONFIG_OBJTOOL_WERROR got merged into -tip, a lot of warnings have been reported. Fix those along with several other improvements.

Cc Raju Rangoju <[email protected]>
CC Mark Brown <[email protected]>
Cc Christoph Hellwig <[email protected]>
Cc Sagi Grimberg <[email protected]>
Cc Chaitanya Kulkarni <[email protected]>
Cc Mauro Carvalho Chehab <[email protected]>
Cc Dmitry Torokhov <[email protected]>
Cc Srinivas Kandagatla <[email protected]>
Cc Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Kees Cook <[email protected]>

Josh Poimboeuf (22):
objtool: Fix detection of consecutive jump tables
objtool: Warn when disabling unreachable warnings
objtool: Ignore entire functions rather than instructions
objtool: Fix X86_FEATURE_SMAP alternative handling
objtool: Fix CONFIG_OBJTOOL_WERROR for vmlinux.o
objtool: Fix init_module() handling
objtool: Silence more KCOV warnings
objtool: Properly disable uaccess validation
objtool: Improve error handling
objtool: Reduce CONFIG_OBJTOOL_WERROR verbosity
objtool: Fix up some outdated references to ENTRY/ENDPROC
objtool: Remove --no-unreachable for noinstr-only vmlinux.o runs
objtool: Remove redundant opts.noinstr dependency
spi: amd: Fix out-of-bounds stack access in amd_set_spi_freq()
nvmet: Fix out-of-bounds stack access in nvmet_ctrl_state_show()
media: dib8000: Prevent divide-by-zero in dib8000_set_dds()
panic: Disable SMAP in __stack_chk_fail()
Input: cyapa - remove undefined behavior in cyapa_update_fw_store()
ASoC: codecs: wcd934x: Remove undefined behavior in

    wcd934x_slim_irq_handler()

regulator: rk808: Remove undefined behavior in rk806_set_mode_dcdc()
pwm: mediatek: Prevent divide-by-zero in pwm_mediatek_config()
lkdtm: Obfuscate do_nothing() pointer

 arch/x86/include/asm/arch_hweight.h     |   8 +-
 arch/x86/include/asm/smap.h             |  23 +-
 arch/x86/include/asm/xen/hypercall.h    |   6 +-
 drivers/input/mouse/cyapa.c             |   4 +-
 drivers/media/dvb-frontends/dib8000.c   |   5 +-
 drivers/misc/lkdtm/perms.c              |  14 +-
 drivers/nvme/target/debugfs.c           |   2 +-
 drivers/pwm/pwm-mediatek.c              |   8 +-
 drivers/regulator/rk808-regulator.c     |   4 +-
 drivers/spi/spi-amd.c                   |   2 +-
 include/linux/linkage.h                 |   4 -
 include/linux/objtool.h                 |   2 +-
 kernel/panic.c                          |   6 +
 scripts/Makefile.lib                    |   2 +-
 scripts/Makefile.vmlinux_o              |  15 +-
 sound/soc/codecs/wcd934x.c              |   2 +-
 tools/objtool/Documentation/objtool.txt |  10 +-
 tools/objtool/arch/x86/special.c        |  38 +-
 tools/objtool/builtin-check.c           | 146 ++++---
 tools/objtool/check.c                   | 525 ++++++++++++------------
 tools/objtool/elf.c                     |  28 +-
 tools/objtool/include/objtool/builtin.h |   6 +-
 tools/objtool/include/objtool/check.h   |   3 +-
 tools/objtool/include/objtool/elf.h     |  28 +-
 tools/objtool/include/objtool/objtool.h |   2 +-
 tools/objtool/include/objtool/special.h |   4 +-
 tools/objtool/include/objtool/warn.h    |  13 +-
 tools/objtool/objtool.c                 |  11 +-
 tools/objtool/special.c                 |  12 +-
 29 files changed, 477 insertions(+), 456 deletions(-)
Signature · 1 line
2.48.1
author maintainer
#1 lore raw

PATCH 1/22 objtool: Fix detection of consecutive jump tables

The jump table detection code assumes jump tables are in the same order as their corresponding indirect branches. That's apparently not always true with Clang 20.

Fix that by changing how multiple jump tables are detected. In the first detection pass, mark the beginning of each jump table so the second pass can tell where one ends and the next one begins.

Fixes the following warnings:

vmlinux.o: warning: objtool: SiS_GetCRT2Ptr+0x1ad: stack state mismatch: cfa1=4+8 cfa2=5+16 sound/core/seq/snd-seq.o: warning: objtool: cc_ev_to_ump_midi2+0x589: return with modified stack frame

---
 tools/objtool/check.c               | 26 ++++++++------------------
 tools/objtool/elf.c                 |  6 +++---
 tools/objtool/include/objtool/elf.h | 27 ++++++++++++++++++++++++++-
 3 files changed, 37 insertions(+), 22 deletions(-)
tools/objtool/check.c +8 -18
index 48d7bc5b4736..33beefd459a6 100644
@@ -1949,8 +1949,7 @@ __weak unsigned long arch_jump_table_sym_offset(struct reloc *reloc, struct relo
 	return reloc->sym->offset + reloc_addend(reloc);
 }
 
-static int add_jump_table(struct objtool_file *file, struct instruction *insn,
-			  struct reloc *next_table)
+static int add_jump_table(struct objtool_file *file, struct instruction *insn)
 {
 	unsigned long table_size = insn_jump_table_size(insn);
 	struct symbol *pfunc = insn_func(insn)->pfunc;
@@ -1970,7 +1969,7 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
 		/* Check for the end of the table: */
 		if (table_size && reloc_offset(reloc) - reloc_offset(table) >= table_size)
 			break;
-		if (reloc != table && reloc == next_table)
+		if (reloc != table && is_jump_table(reloc))
 			break;
 
 		/* Make sure the table entries are consecutive: */
@@ -2061,8 +2060,10 @@ static void find_jump_table(struct objtool_file *file, struct symbol *func,
 		if (!dest_insn || !insn_func(dest_insn) || insn_func(dest_insn)->pfunc != func)
 			continue;
 
+		set_jump_table(table_reloc);
 		orig_insn->_jump_table = table_reloc;
 		orig_insn->_jump_table_size = table_size;
+
 		break;
 	}
 }
@@ -2104,31 +2105,20 @@ static void mark_func_jump_tables(struct objtool_file *file,
 static int add_func_jump_tables(struct objtool_file *file,
 				  struct symbol *func)
 {
-	struct instruction *insn, *insn_t1 = NULL, *insn_t2;
-	int ret = 0;
+	struct instruction *insn;
+	int ret;
 
 	func_for_each_insn(file, func, insn) {
 		if (!insn_jump_table(insn))
 			continue;
 
-		if (!insn_t1) {
-			insn_t1 = insn;
-			continue;
-		}
 
-		insn_t2 = insn;
-
-		ret = add_jump_table(file, insn_t1, insn_jump_table(insn_t2));
+		ret = add_jump_table(file, insn);
 		if (ret)
 			return ret;
-
-		insn_t1 = insn_t2;
 	}
 
-	if (insn_t1)
-		ret = add_jump_table(file, insn_t1, NULL);
-
-	return ret;
+	return 0;
 }
 
 /*
tools/objtool/elf.c +3 -3
index be4f4b62730c..0f38167bd840 100644
@@ -583,7 +583,7 @@ static int elf_update_sym_relocs(struct elf *elf, struct symbol *sym)
 {
 	struct reloc *reloc;
 
-	for (reloc = sym->relocs; reloc; reloc = reloc->sym_next_reloc)
+	for (reloc = sym->relocs; reloc; reloc = sym_next_reloc(reloc))
 		set_reloc_sym(elf, reloc, reloc->sym->idx);
 
 	return 0;
@@ -880,7 +880,7 @@ static struct reloc *elf_init_reloc(struct elf *elf, struct section *rsec,
 	set_reloc_addend(elf, reloc, addend);
 
 	elf_hash_add(reloc, &reloc->hash, reloc_hash(reloc));
-	reloc->sym_next_reloc = sym->relocs;
+	set_sym_next_reloc(reloc, sym->relocs);
 	sym->relocs = reloc;
 
 	return reloc;
@@ -979,7 +979,7 @@ static int read_relocs(struct elf *elf)
 			}
 
 			elf_hash_add(reloc, &reloc->hash, reloc_hash(reloc));
-			reloc->sym_next_reloc = sym->relocs;
+			set_sym_next_reloc(reloc, sym->relocs);
 			sym->relocs = reloc;
 
 			nr_reloc++;
tools/objtool/include/objtool/elf.h +26 -1
index 223ac1c24b90..4edc957a6f6b 100644
@@ -77,7 +77,7 @@ struct reloc {
 	struct elf_hash_node hash;
 	struct section *sec;
 	struct symbol *sym;
-	struct reloc *sym_next_reloc;
+	unsigned long _sym_next_reloc;
 };
 
 struct elf {
@@ -297,6 +297,31 @@ static inline void set_reloc_type(struct elf *elf, struct reloc *reloc, unsigned
 	mark_sec_changed(elf, reloc->sec, true);
 }
 
+#define RELOC_JUMP_TABLE_BIT 1UL
+
+/* Does reloc mark the beginning of a jump table? */
+static inline bool is_jump_table(struct reloc *reloc)
+{
+	return reloc->_sym_next_reloc & RELOC_JUMP_TABLE_BIT;
+}
+
+static inline void set_jump_table(struct reloc *reloc)
+{
+	reloc->_sym_next_reloc |= RELOC_JUMP_TABLE_BIT;
+}
+
+static inline struct reloc *sym_next_reloc(struct reloc *reloc)
+{
+	return (struct reloc *)(reloc->_sym_next_reloc & ~RELOC_JUMP_TABLE_BIT);
+}
+
+static inline void set_sym_next_reloc(struct reloc *reloc, struct reloc *next)
+{
+	unsigned long bit = reloc->_sym_next_reloc & RELOC_JUMP_TABLE_BIT;
+
+	reloc->_sym_next_reloc = (unsigned long)next | bit;
+}
+
 #define for_each_sec(file, sec)						\
 	list_for_each_entry(sec, &file->elf->sections, list)
 
Signature · 1 line
2.48.1
merge bot Applied
#2 lore raw

[tip: objtool/urgent] objtool: Fix detection of consecutive jump tables on Clang 20

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     ef753d66051ca03bee1982ce047f9eaf90f81ab4
Gitweb:        https://git.kernel.org/tip/ef753d66051ca03bee1982ce047f9eaf90f81ab4
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:55:51 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:25 +01:00

objtool: Fix detection of consecutive jump tables on Clang 20

The jump table detection code assumes jump tables are in the same order as their corresponding indirect branches. That's apparently not always true with Clang 20.

Fix that by changing how multiple jump tables are detected. In the first detection pass, mark the beginning of each jump table so the second pass can tell where one ends and the next one begins.

Fixes the following warnings:

vmlinux.o: warning: objtool: SiS_GetCRT2Ptr+0x1ad: stack state mismatch: cfa1=4+8 cfa2=5+16 sound/core/seq/snd-seq.o: warning: objtool: cc_ev_to_ump_midi2+0x589: return with modified stack frame

---
 tools/objtool/check.c               | 26 ++++++++------------------
 tools/objtool/elf.c                 |  6 +++---
 tools/objtool/include/objtool/elf.h | 27 ++++++++++++++++++++++++++-
 3 files changed, 37 insertions(+), 22 deletions(-)
tools/objtool/check.c +8 -18
index ca3435a..dae17ed 100644
@@ -1941,8 +1941,7 @@ __weak unsigned long arch_jump_table_sym_offset(struct reloc *reloc, struct relo
 	return reloc->sym->offset + reloc_addend(reloc);
 }
 
-static int add_jump_table(struct objtool_file *file, struct instruction *insn,
-			  struct reloc *next_table)
+static int add_jump_table(struct objtool_file *file, struct instruction *insn)
 {
 	unsigned long table_size = insn_jump_table_size(insn);
 	struct symbol *pfunc = insn_func(insn)->pfunc;
@@ -1962,7 +1961,7 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
 		/* Check for the end of the table: */
 		if (table_size && reloc_offset(reloc) - reloc_offset(table) >= table_size)
 			break;
-		if (reloc != table && reloc == next_table)
+		if (reloc != table && is_jump_table(reloc))
 			break;
 
 		/* Make sure the table entries are consecutive: */
@@ -2053,8 +2052,10 @@ static void find_jump_table(struct objtool_file *file, struct symbol *func,
 		if (!dest_insn || !insn_func(dest_insn) || insn_func(dest_insn)->pfunc != func)
 			continue;
 
+		set_jump_table(table_reloc);
 		orig_insn->_jump_table = table_reloc;
 		orig_insn->_jump_table_size = table_size;
+
 		break;
 	}
 }
@@ -2096,31 +2097,20 @@ static void mark_func_jump_tables(struct objtool_file *file,
 static int add_func_jump_tables(struct objtool_file *file,
 				  struct symbol *func)
 {
-	struct instruction *insn, *insn_t1 = NULL, *insn_t2;
-	int ret = 0;
+	struct instruction *insn;
+	int ret;
 
 	func_for_each_insn(file, func, insn) {
 		if (!insn_jump_table(insn))
 			continue;
 
-		if (!insn_t1) {
-			insn_t1 = insn;
-			continue;
-		}
-
-		insn_t2 = insn;
 
-		ret = add_jump_table(file, insn_t1, insn_jump_table(insn_t2));
+		ret = add_jump_table(file, insn);
 		if (ret)
 			return ret;
-
-		insn_t1 = insn_t2;
 	}
 
-	if (insn_t1)
-		ret = add_jump_table(file, insn_t1, NULL);
-
-	return ret;
+	return 0;
 }
 
 /*
tools/objtool/elf.c +3 -3
index be4f4b6..0f38167 100644
@@ -583,7 +583,7 @@ static int elf_update_sym_relocs(struct elf *elf, struct symbol *sym)
 {
 	struct reloc *reloc;
 
-	for (reloc = sym->relocs; reloc; reloc = reloc->sym_next_reloc)
+	for (reloc = sym->relocs; reloc; reloc = sym_next_reloc(reloc))
 		set_reloc_sym(elf, reloc, reloc->sym->idx);
 
 	return 0;
@@ -880,7 +880,7 @@ static struct reloc *elf_init_reloc(struct elf *elf, struct section *rsec,
 	set_reloc_addend(elf, reloc, addend);
 
 	elf_hash_add(reloc, &reloc->hash, reloc_hash(reloc));
-	reloc->sym_next_reloc = sym->relocs;
+	set_sym_next_reloc(reloc, sym->relocs);
 	sym->relocs = reloc;
 
 	return reloc;
@@ -979,7 +979,7 @@ static int read_relocs(struct elf *elf)
 			}
 
 			elf_hash_add(reloc, &reloc->hash, reloc_hash(reloc));
-			reloc->sym_next_reloc = sym->relocs;
+			set_sym_next_reloc(reloc, sym->relocs);
 			sym->relocs = reloc;
 
 			nr_reloc++;
tools/objtool/include/objtool/elf.h +26 -1
index 223ac1c..4edc957 100644
@@ -77,7 +77,7 @@ struct reloc {
 	struct elf_hash_node hash;
 	struct section *sec;
 	struct symbol *sym;
-	struct reloc *sym_next_reloc;
+	unsigned long _sym_next_reloc;
 };
 
 struct elf {
@@ -297,6 +297,31 @@ static inline void set_reloc_type(struct elf *elf, struct reloc *reloc, unsigned
 	mark_sec_changed(elf, reloc->sec, true);
 }
 
+#define RELOC_JUMP_TABLE_BIT 1UL
+
+/* Does reloc mark the beginning of a jump table? */
+static inline bool is_jump_table(struct reloc *reloc)
+{
+	return reloc->_sym_next_reloc & RELOC_JUMP_TABLE_BIT;
+}
+
+static inline void set_jump_table(struct reloc *reloc)
+{
+	reloc->_sym_next_reloc |= RELOC_JUMP_TABLE_BIT;
+}
+
+static inline struct reloc *sym_next_reloc(struct reloc *reloc)
+{
+	return (struct reloc *)(reloc->_sym_next_reloc & ~RELOC_JUMP_TABLE_BIT);
+}
+
+static inline void set_sym_next_reloc(struct reloc *reloc, struct reloc *next)
+{
+	unsigned long bit = reloc->_sym_next_reloc & RELOC_JUMP_TABLE_BIT;
+
+	reloc->_sym_next_reloc = (unsigned long)next | bit;
+}
+
 #define for_each_sec(file, sec)						\
 	list_for_each_entry(sec, &file->elf->sections, list)
 
author maintainer
#3 lore raw

PATCH 2/22 objtool: Warn when disabling unreachable warnings

In reply to Josh Poimboeuf

Print a warning when disabling the unreachable warnings (due to a GCC bug). This will help determine if recent GCCs still have the issue and alert us if any other issues might be silently lurking behind the unreachable disablement.

---
 tools/objtool/arch/x86/special.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
tools/objtool/arch/x86/special.c +4 -1
index 9c1c9df09aaa..5f46d4e7f7f8 100644
@@ -3,6 +3,7 @@
 
 #include <objtool/special.h>
 #include <objtool/builtin.h>
+#include <objtool/warn.h>
 
 #define X86_FEATURE_POPCNT (4 * 32 + 23)
 #define X86_FEATURE_SMAP   (9 * 32 + 20)
@@ -156,8 +157,10 @@ struct reloc *arch_find_switch_table(struct objtool_file *file,
 	 * indicates a rare GCC quirk/bug which can leave dead
 	 * code behind.
 	 */
-	if (reloc_type(text_reloc) == R_X86_64_PC32)
+	if (reloc_type(text_reloc) == R_X86_64_PC32) {
+		WARN_INSN(insn, "ignoring unreachables due to jump table quirk");
 		file->ignore_unreachables = true;
+	}
 
 	*table_size = 0;
 	return rodata_reloc;
Signature · 1 line
2.48.1
merge bot Applied
#4 lore raw

[tip: objtool/urgent] objtool: Warn when disabling unreachable warnings

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     eeff7ac61526a4a9c3916cf46885622078ad886b
Gitweb:        https://git.kernel.org/tip/eeff7ac61526a4a9c3916cf46885622078ad886b
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:55:52 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:25 +01:00

objtool: Warn when disabling unreachable warnings

Print a warning when disabling the unreachable warnings (due to a GCC bug). This will help determine if recent GCCs still have the issue and alert us if any other issues might be silently lurking behind the unreachable disablement.

---
 tools/objtool/arch/x86/special.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
tools/objtool/arch/x86/special.c +4 -1
index 9c1c9df..5f46d4e 100644
@@ -3,6 +3,7 @@
 
 #include <objtool/special.h>
 #include <objtool/builtin.h>
+#include <objtool/warn.h>
 
 #define X86_FEATURE_POPCNT (4 * 32 + 23)
 #define X86_FEATURE_SMAP   (9 * 32 + 20)
@@ -156,8 +157,10 @@ struct reloc *arch_find_switch_table(struct objtool_file *file,
 	 * indicates a rare GCC quirk/bug which can leave dead
 	 * code behind.
 	 */
-	if (reloc_type(text_reloc) == R_X86_64_PC32)
+	if (reloc_type(text_reloc) == R_X86_64_PC32) {
+		WARN_INSN(insn, "ignoring unreachables due to jump table quirk");
 		file->ignore_unreachables = true;
+	}
 
 	*table_size = 0;
 	return rodata_reloc;
author maintainer
#5 lore raw

PATCH 3/22 objtool: Ignore entire functions rather than instructions

In reply to Josh Poimboeuf

STACK_FRAME_NON_STANDARD applies to functions. Use a function-specific ignore attribute in preparation for getting rid of insn->ignore.

---
 tools/objtool/check.c               | 35 +++++++++++++++--------------
 tools/objtool/include/objtool/elf.h |  1 +
 2 files changed, 19 insertions(+), 17 deletions(-)
tools/objtool/check.c +18 -17
index 33beefd459a6..8e2f3405d78c 100644
@@ -981,7 +981,6 @@ static int create_direct_call_sections(struct objtool_file *file)
  */
 static void add_ignores(struct objtool_file *file)
 {
-	struct instruction *insn;
 	struct section *rsec;
 	struct symbol *func;
 	struct reloc *reloc;
@@ -1008,8 +1007,7 @@ static void add_ignores(struct objtool_file *file)
 			continue;
 		}
 
-		func_for_each_insn(file, func, insn)
-			insn->ignore = true;
+		func->ignore = true;
 	}
 }
 
@@ -1620,6 +1618,7 @@ static int add_call_destinations(struct objtool_file *file)
 	struct reloc *reloc;
 
 	for_each_insn(file, insn) {
+		struct symbol *func = insn_func(insn);
 		if (insn->type != INSN_CALL)
 			continue;
 
@@ -1630,7 +1629,7 @@ static int add_call_destinations(struct objtool_file *file)
 
 			add_call_dest(file, insn, dest, false);
 
-			if (insn->ignore)
+			if (func && func->ignore)
 				continue;
 
 			if (!insn_call_dest(insn)) {
@@ -1638,7 +1637,7 @@ static int add_call_destinations(struct objtool_file *file)
 				return -1;
 			}
 
-			if (insn_func(insn) && insn_call_dest(insn)->type != STT_FUNC) {
+			if (func && insn_call_dest(insn)->type != STT_FUNC) {
 				WARN_INSN(insn, "unsupported call to non-function");
 				return -1;
 			}
@@ -3478,6 +3477,9 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 	u8 visited;
 	int ret;
 
+	if (func && func->ignore)
+		return 0;
+
 	sec = insn->sec;
 
 	while (1) {
@@ -3723,7 +3725,7 @@ static int validate_unwind_hint(struct objtool_file *file,
 				  struct instruction *insn,
 				  struct insn_state *state)
 {
-	if (insn->hint && !insn->visited && !insn->ignore) {
+	if (insn->hint && !insn->visited) {
 		int ret = validate_branch(file, insn_func(insn), insn, *state);
 		if (ret)
 			BT_INSN(insn, "<=== (hint)");
@@ -3937,10 +3939,11 @@ static bool is_ubsan_insn(struct instruction *insn)
 
 static bool ignore_unreachable_insn(struct objtool_file *file, struct instruction *insn)
 {
-	int i;
+	struct symbol *func = insn_func(insn);
 	struct instruction *prev_insn;
+	int i;
 
-	if (insn->ignore || insn->type == INSN_NOP || insn->type == INSN_TRAP)
+	if (insn->ignore || insn->type == INSN_NOP || insn->type == INSN_TRAP || (func && func->ignore))
 		return true;
 
 	/*
@@ -3959,7 +3962,7 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
 	 * In this case we'll find a piece of code (whole function) that is not
 	 * covered by a !section symbol. Ignore them.
 	 */
-	if (opts.link && !insn_func(insn)) {
+	if (opts.link && !func) {
 		int size = find_symbol_hole_containing(insn->sec, insn->offset);
 		unsigned long end = insn->offset + size;
 
@@ -3985,19 +3988,17 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
 			 */
 			if (insn->jump_dest && insn_func(insn->jump_dest) &&
 			    strstr(insn_func(insn->jump_dest)->name, ".cold")) {
-				struct instruction *dest = insn->jump_dest;
-				func_for_each_insn(file, insn_func(dest), dest)
-					dest->ignore = true;
+				insn_func(insn->jump_dest)->ignore = true;
 			}
 		}
 
 		return false;
 	}
 
-	if (!insn_func(insn))
+	if (!func)
 		return false;
 
-	if (insn_func(insn)->static_call_tramp)
+	if (func->static_call_tramp)
 		return true;
 
 	/*
@@ -4028,7 +4029,7 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
 
 		if (insn->type == INSN_JUMP_UNCONDITIONAL) {
 			if (insn->jump_dest &&
-			    insn_func(insn->jump_dest) == insn_func(insn)) {
+			    insn_func(insn->jump_dest) == func) {
 				insn = insn->jump_dest;
 				continue;
 			}
@@ -4036,7 +4037,7 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
 			break;
 		}
 
-		if (insn->offset + insn->len >= insn_func(insn)->offset + insn_func(insn)->len)
+		if (insn->offset + insn->len >= func->offset + func->len)
 			break;
 
 		insn = next_insn_same_sec(file, insn);
@@ -4128,7 +4129,7 @@ static int validate_symbol(struct objtool_file *file, struct section *sec,
 		return 0;
 
 	insn = find_insn(file, sec, sym->offset);
-	if (!insn || insn->ignore || insn->visited)
+	if (!insn || insn->visited)
 		return 0;
 
 	state->uaccess = sym->uaccess_safe;
tools/objtool/include/objtool/elf.h +1 -0
index 4edc957a6f6b..eba04392c6fd 100644
@@ -68,6 +68,7 @@ struct symbol {
 	u8 embedded_insn     : 1;
 	u8 local_label       : 1;
 	u8 frame_pointer     : 1;
+	u8 ignore	     : 1;
 	u8 warnings	     : 2;
 	struct list_head pv_target;
 	struct reloc *relocs;
Signature · 1 line
2.48.1
merge bot Applied
#6 lore raw

[tip: objtool/urgent] objtool: Ignore entire functions rather than instructions

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     c84301d706c5456b1460439b2987a0f0b6362a82
Gitweb:        https://git.kernel.org/tip/c84301d706c5456b1460439b2987a0f0b6362a82
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:55:53 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:25 +01:00

objtool: Ignore entire functions rather than instructions

STACK_FRAME_NON_STANDARD applies to functions. Use a function-specific ignore attribute in preparation for getting rid of insn->ignore.

---
 tools/objtool/check.c               | 35 ++++++++++++++--------------
 tools/objtool/include/objtool/elf.h |  1 +-
 2 files changed, 19 insertions(+), 17 deletions(-)
tools/objtool/check.c +18 -17
index dae17ed..f465020 100644
@@ -981,7 +981,6 @@ static int create_direct_call_sections(struct objtool_file *file)
  */
 static void add_ignores(struct objtool_file *file)
 {
-	struct instruction *insn;
 	struct section *rsec;
 	struct symbol *func;
 	struct reloc *reloc;
@@ -1008,8 +1007,7 @@ static void add_ignores(struct objtool_file *file)
 			continue;
 		}
 
-		func_for_each_insn(file, func, insn)
-			insn->ignore = true;
+		func->ignore = true;
 	}
 }
 
@@ -1612,6 +1610,7 @@ static int add_call_destinations(struct objtool_file *file)
 	struct reloc *reloc;
 
 	for_each_insn(file, insn) {
+		struct symbol *func = insn_func(insn);
 		if (insn->type != INSN_CALL)
 			continue;
 
@@ -1622,7 +1621,7 @@ static int add_call_destinations(struct objtool_file *file)
 
 			add_call_dest(file, insn, dest, false);
 
-			if (insn->ignore)
+			if (func && func->ignore)
 				continue;
 
 			if (!insn_call_dest(insn)) {
@@ -1630,7 +1629,7 @@ static int add_call_destinations(struct objtool_file *file)
 				return -1;
 			}
 
-			if (insn_func(insn) && insn_call_dest(insn)->type != STT_FUNC) {
+			if (func && insn_call_dest(insn)->type != STT_FUNC) {
 				WARN_INSN(insn, "unsupported call to non-function");
 				return -1;
 			}
@@ -3470,6 +3469,9 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 	u8 visited;
 	int ret;
 
+	if (func && func->ignore)
+		return 0;
+
 	sec = insn->sec;
 
 	while (1) {
@@ -3715,7 +3717,7 @@ static int validate_unwind_hint(struct objtool_file *file,
 				  struct instruction *insn,
 				  struct insn_state *state)
 {
-	if (insn->hint && !insn->visited && !insn->ignore) {
+	if (insn->hint && !insn->visited) {
 		int ret = validate_branch(file, insn_func(insn), insn, *state);
 		if (ret)
 			BT_INSN(insn, "<=== (hint)");
@@ -3929,10 +3931,11 @@ static bool is_ubsan_insn(struct instruction *insn)
 
 static bool ignore_unreachable_insn(struct objtool_file *file, struct instruction *insn)
 {
-	int i;
+	struct symbol *func = insn_func(insn);
 	struct instruction *prev_insn;
+	int i;
 
-	if (insn->ignore || insn->type == INSN_NOP || insn->type == INSN_TRAP)
+	if (insn->ignore || insn->type == INSN_NOP || insn->type == INSN_TRAP || (func && func->ignore))
 		return true;
 
 	/*
@@ -3951,7 +3954,7 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
 	 * In this case we'll find a piece of code (whole function) that is not
 	 * covered by a !section symbol. Ignore them.
 	 */
-	if (opts.link && !insn_func(insn)) {
+	if (opts.link && !func) {
 		int size = find_symbol_hole_containing(insn->sec, insn->offset);
 		unsigned long end = insn->offset + size;
 
@@ -3977,19 +3980,17 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
 			 */
 			if (insn->jump_dest && insn_func(insn->jump_dest) &&
 			    strstr(insn_func(insn->jump_dest)->name, ".cold")) {
-				struct instruction *dest = insn->jump_dest;
-				func_for_each_insn(file, insn_func(dest), dest)
-					dest->ignore = true;
+				insn_func(insn->jump_dest)->ignore = true;
 			}
 		}
 
 		return false;
 	}
 
-	if (!insn_func(insn))
+	if (!func)
 		return false;
 
-	if (insn_func(insn)->static_call_tramp)
+	if (func->static_call_tramp)
 		return true;
 
 	/*
@@ -4020,7 +4021,7 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
 
 		if (insn->type == INSN_JUMP_UNCONDITIONAL) {
 			if (insn->jump_dest &&
-			    insn_func(insn->jump_dest) == insn_func(insn)) {
+			    insn_func(insn->jump_dest) == func) {
 				insn = insn->jump_dest;
 				continue;
 			}
@@ -4028,7 +4029,7 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
 			break;
 		}
 
-		if (insn->offset + insn->len >= insn_func(insn)->offset + insn_func(insn)->len)
+		if (insn->offset + insn->len >= func->offset + func->len)
 			break;
 
 		insn = next_insn_same_sec(file, insn);
@@ -4120,7 +4121,7 @@ static int validate_symbol(struct objtool_file *file, struct section *sec,
 		return 0;
 
 	insn = find_insn(file, sec, sym->offset);
-	if (!insn || insn->ignore || insn->visited)
+	if (!insn || insn->visited)
 		return 0;
 
 	state->uaccess = sym->uaccess_safe;
tools/objtool/include/objtool/elf.h +1 -0
index 4edc957..eba0439 100644
@@ -68,6 +68,7 @@ struct symbol {
 	u8 embedded_insn     : 1;
 	u8 local_label       : 1;
 	u8 frame_pointer     : 1;
+	u8 ignore	     : 1;
 	u8 warnings	     : 2;
 	struct list_head pv_target;
 	struct reloc *relocs;
author maintainer
#7 lore raw

PATCH 4/22 objtool: Fix X86_FEATURE_SMAP alternative handling

In reply to Josh Poimboeuf

For X86_FEATURE_SMAP alternatives which replace NOP with STAC or CLAC, uaccess validation skips the NOP branch to avoid following impossible code paths, e.g. where a STAC would be patched but a CLAC wouldn't.

However, it's not safe to assume an X86_FEATURE_SMAP alternative is patching STAC/CLAC. There can be other alternatives, like static_cpu_has(), where both branches need to be validated.

Fix that by repurposing ANNOTATE_IGNORE_ALTERNATIVE for skipping either original instructions or new ones. This is a more generic approach which enables the removal of the feature checking hacks and the insn->ignore bit.

Fixes the following warnings:

arch/x86/mm/fault.o: warning: objtool: do_user_addr_fault+0x8ec: __stack_chk_fail() missing __noreturn in .c/.h or NORETURN() in noreturns.h arch/x86/mm/fault.o: warning: objtool: do_user_addr_fault+0x8f1: unreachable instruction

---
 arch/x86/include/asm/arch_hweight.h     |  8 +++--
 arch/x86/include/asm/smap.h             | 23 ++++++++++-----
 arch/x86/include/asm/xen/hypercall.h    |  6 ++--
 tools/objtool/arch/x86/special.c        | 33 +--------------------
 tools/objtool/check.c                   | 39 +++++++------------------
 tools/objtool/include/objtool/check.h   |  3 +-
 tools/objtool/include/objtool/special.h |  4 +--
 tools/objtool/special.c                 | 12 ++------
 8 files changed, 39 insertions(+), 89 deletions(-)
arch/x86/include/asm/arch_hweight.h +6 -2
index ba88edd0d58b..a0eaa8fda55f 100644
@@ -16,7 +16,9 @@ static __always_inline unsigned int __arch_hweight32(unsigned int w)
 {
 	unsigned int res;
 
-	asm (ALTERNATIVE("call __sw_hweight32", "popcntl %1, %0", X86_FEATURE_POPCNT)
+	asm (ALTERNATIVE(ANNOTATE_IGNORE_ALTERNATIVE
+			 "call __sw_hweight32",
+			 "popcntl %1, %0", X86_FEATURE_POPCNT)
 			 : "="REG_OUT (res)
 			 : REG_IN (w));
 
@@ -44,7 +46,9 @@ static __always_inline unsigned long __arch_hweight64(__u64 w)
 {
 	unsigned long res;
 
-	asm (ALTERNATIVE("call __sw_hweight64", "popcntq %1, %0", X86_FEATURE_POPCNT)
+	asm (ALTERNATIVE(ANNOTATE_IGNORE_ALTERNATIVE
+			 "call __sw_hweight64",
+			 "popcntq %1, %0", X86_FEATURE_POPCNT)
 			 : "="REG_OUT (res)
 			 : REG_IN (w));
 
arch/x86/include/asm/smap.h +15 -8
index 2de1e5a75c57..a984cdd6de30 100644
@@ -16,23 +16,23 @@
 #ifdef __ASSEMBLY__
 
 #define ASM_CLAC \
-	ALTERNATIVE "", "clac", X86_FEATURE_SMAP
+	ALTERNATIVE __stringify(ANNOTATE_IGNORE_ALTERNATIVE), "clac", X86_FEATURE_SMAP
 
 #define ASM_STAC \
-	ALTERNATIVE "", "stac", X86_FEATURE_SMAP
+	ALTERNATIVE __stringify(ANNOTATE_IGNORE_ALTERNATIVE), "stac", X86_FEATURE_SMAP
 
 #else /* __ASSEMBLY__ */
 
 static __always_inline void clac(void)
 {
 	/* Note: a barrier is implicit in alternative() */
-	alternative("", "clac", X86_FEATURE_SMAP);
+	alternative(ANNOTATE_IGNORE_ALTERNATIVE "", "clac", X86_FEATURE_SMAP);
 }
 
 static __always_inline void stac(void)
 {
 	/* Note: a barrier is implicit in alternative() */
-	alternative("", "stac", X86_FEATURE_SMAP);
+	alternative(ANNOTATE_IGNORE_ALTERNATIVE "", "stac", X86_FEATURE_SMAP);
 }
 
 static __always_inline unsigned long smap_save(void)
@@ -40,7 +40,8 @@ static __always_inline unsigned long smap_save(void)
 	unsigned long flags;
 
 	asm volatile ("# smap_save\n\t"
-		      ALTERNATIVE("", "pushf; pop %0; " "clac" "\n\t",
+		      ALTERNATIVE(ANNOTATE_IGNORE_ALTERNATIVE
+				  "", "pushf; pop %0; clac",
 				  X86_FEATURE_SMAP)
 		      : "=rm" (flags) : : "memory", "cc");
 
@@ -50,16 +51,22 @@ static __always_inline unsigned long smap_save(void)
 static __always_inline void smap_restore(unsigned long flags)
 {
 	asm volatile ("# smap_restore\n\t"
-		      ALTERNATIVE("", "push %0; popf\n\t",
+		      ALTERNATIVE(ANNOTATE_IGNORE_ALTERNATIVE
+				  "", "push %0; popf",
 				  X86_FEATURE_SMAP)
 		      : : "g" (flags) : "memory", "cc");
 }
 
 /* These macros can be used in asm() statements */
 #define ASM_CLAC \
-	ALTERNATIVE("", "clac", X86_FEATURE_SMAP)
+	ALTERNATIVE(ANNOTATE_IGNORE_ALTERNATIVE "", "clac", X86_FEATURE_SMAP)
 #define ASM_STAC \
-	ALTERNATIVE("", "stac", X86_FEATURE_SMAP)
+	ALTERNATIVE(ANNOTATE_IGNORE_ALTERNATIVE "", "stac", X86_FEATURE_SMAP)
+
+#define ASM_CLAC_UNSAFE \
+	ALTERNATIVE("", ANNOTATE_IGNORE_ALTERNATIVE "clac", X86_FEATURE_SMAP)
+#define ASM_STAC_UNSAFE \
+	ALTERNATIVE("", ANNOTATE_IGNORE_ALTERNATIVE "stac", X86_FEATURE_SMAP)
 
 #endif /* __ASSEMBLY__ */
 
arch/x86/include/asm/xen/hypercall.h +2 -4
index 97771b9d33af..59a62c3780a2 100644
@@ -231,14 +231,12 @@ static __always_inline void __xen_stac(void)
 	 * Suppress objtool seeing the STAC/CLAC and getting confused about it
 	 * calling random code with AC=1.
 	 */
-	asm volatile(ANNOTATE_IGNORE_ALTERNATIVE
-		     ASM_STAC ::: "memory", "flags");
+	asm volatile(ASM_STAC_UNSAFE ::: "memory", "flags");
 }
 
 static __always_inline void __xen_clac(void)
 {
-	asm volatile(ANNOTATE_IGNORE_ALTERNATIVE
-		     ASM_CLAC ::: "memory", "flags");
+	asm volatile(ASM_CLAC_UNSAFE ::: "memory", "flags");
 }
 
 static inline long
tools/objtool/arch/x86/special.c +1 -32
index 5f46d4e7f7f8..403e587676f1 100644
@@ -5,10 +5,7 @@
 #include <objtool/builtin.h>
 #include <objtool/warn.h>
 
-#define X86_FEATURE_POPCNT (4 * 32 + 23)
-#define X86_FEATURE_SMAP   (9 * 32 + 20)
-
-void arch_handle_alternative(unsigned short feature, struct special_alt *alt)
+void arch_handle_alternative(struct special_alt *alt)
 {
 	static struct special_alt *group, *prev;
 
@@ -32,34 +29,6 @@ void arch_handle_alternative(unsigned short feature, struct special_alt *alt)
 	} else group = alt;
 
 	prev = alt;
-
-	switch (feature) {
-	case X86_FEATURE_SMAP:
-		/*
-		 * If UACCESS validation is enabled; force that alternative;
-		 * otherwise force it the other way.
-		 *
-		 * What we want to avoid is having both the original and the
-		 * alternative code flow at the same time, in that case we can
-		 * find paths that see the STAC but take the NOP instead of
-		 * CLAC and the other way around.
-		 */
-		if (opts.uaccess)
-			alt->skip_orig = true;
-		else
-			alt->skip_alt = true;
-		break;
-	case X86_FEATURE_POPCNT:
-		/*
-		 * It has been requested that we don't validate the !POPCNT
-		 * feature path which is a "very very small percentage of
-		 * machines".
-		 */
-		alt->skip_orig = true;
-		break;
-	default:
-		break;
-	}
 }
 
 bool arch_support_alt_relocation(struct special_alt *special_alt,
tools/objtool/check.c +10 -29
index 8e2f3405d78c..bac88ca82dec 100644
@@ -25,7 +25,6 @@
 struct alternative {
 	struct alternative *next;
 	struct instruction *insn;
-	bool skip_orig;
 };
 
 static unsigned long nr_cfi, nr_cfi_reused, nr_cfi_cache;
@@ -1704,6 +1703,7 @@ static int handle_group_alt(struct objtool_file *file,
 		orig_alt_group->first_insn = orig_insn;
 		orig_alt_group->last_insn = last_orig_insn;
 		orig_alt_group->nop = NULL;
+		orig_alt_group->ignore = orig_insn->ignore_alts;
 	} else {
 		if (orig_alt_group->last_insn->offset + orig_alt_group->last_insn->len -
 		    orig_alt_group->first_insn->offset != special_alt->orig_len) {
@@ -1743,7 +1743,6 @@ static int handle_group_alt(struct objtool_file *file,
 		nop->type = INSN_NOP;
 		nop->sym = orig_insn->sym;
 		nop->alt_group = new_alt_group;
-		nop->ignore = orig_insn->ignore_alts;
 	}
 
 	if (!special_alt->new_len) {
@@ -1760,7 +1759,6 @@ static int handle_group_alt(struct objtool_file *file,
 
 		last_new_insn = insn;
 
-		insn->ignore = orig_insn->ignore_alts;
 		insn->sym = orig_insn->sym;
 		insn->alt_group = new_alt_group;
 
@@ -1807,6 +1805,7 @@ static int handle_group_alt(struct objtool_file *file,
 	new_alt_group->first_insn = *new_insn;
 	new_alt_group->last_insn = last_new_insn;
 	new_alt_group->nop = nop;
+	new_alt_group->ignore = (*new_insn)->ignore_alts;
 	new_alt_group->cfi = orig_alt_group->cfi;
 	return 0;
 }
@@ -1924,8 +1923,6 @@ static int add_special_section_alts(struct objtool_file *file)
 		}
 
 		alt->insn = new_insn;
-		alt->skip_orig = special_alt->skip_orig;
-		orig_insn->ignore_alts |= special_alt->skip_alt;
 		alt->next = orig_insn->alts;
 		orig_insn->alts = alt;
 
@@ -2303,6 +2300,8 @@ static int read_annotate(struct objtool_file *file,
 static int __annotate_early(struct objtool_file *file, int type, struct instruction *insn)
 {
 	switch (type) {
+
+	/* Must be before add_special_section_alts() */
 	case ANNOTYPE_IGNORE_ALTS:
 		insn->ignore_alts = true;
 		break;
@@ -3496,11 +3495,6 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 			return 1;
 		}
 
-		if (func && insn->ignore) {
-			WARN_INSN(insn, "BUG: why am I validating an ignored function?");
-			return 1;
-		}
-
 		visited = VISITED_BRANCH << state.uaccess;
 		if (insn->visited & VISITED_BRANCH_MASK) {
 			if (!insn->hint && !insn_cfi_match(insn, &state.cfi))
@@ -3572,24 +3566,19 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 		if (propagate_alt_cfi(file, insn))
 			return 1;
 
-		if (!insn->ignore_alts && insn->alts) {
-			bool skip_orig = false;
-
+		if (insn->alts) {
 			for (alt = insn->alts; alt; alt = alt->next) {
-				if (alt->skip_orig)
-					skip_orig = true;
-
 				ret = validate_branch(file, func, alt->insn, state);
 				if (ret) {
 					BT_INSN(insn, "(alt)");
 					return ret;
 				}
 			}
-
-			if (skip_orig)
-				return 0;
 		}
 
+		if (insn->alt_group && insn->alt_group->ignore)
+			return 0;
+
 		if (handle_insn_ops(insn, next_insn, &state))
 			return 1;
 
@@ -3776,23 +3765,15 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn)
 
 		insn->visited |= VISITED_UNRET;
 
-		if (!insn->ignore_alts && insn->alts) {
+		if (insn->alts) {
 			struct alternative *alt;
-			bool skip_orig = false;
-
 			for (alt = insn->alts; alt; alt = alt->next) {
-				if (alt->skip_orig)
-					skip_orig = true;
-
 				ret = validate_unret(file, alt->insn);
 				if (ret) {
 					BT_INSN(insn, "(alt)");
 					return ret;
 				}
 			}
-
-			if (skip_orig)
-				return 0;
 		}
 
 		switch (insn->type) {
@@ -3943,7 +3924,7 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
 	struct instruction *prev_insn;
 	int i;
 
-	if (insn->ignore || insn->type == INSN_NOP || insn->type == INSN_TRAP || (func && func->ignore))
+	if (insn->type == INSN_NOP || insn->type == INSN_TRAP || (func && func->ignore))
 		return true;
 
 	/*
tools/objtool/include/objtool/check.h +2 -1
index e1cd13cd28a3..00fb745e7233 100644
@@ -34,6 +34,8 @@ struct alt_group {
 	 * This is shared with the other alt_groups in the same alternative.
 	 */
 	struct cfi_state **cfi;
+
+	bool ignore;
 };
 
 #define INSN_CHUNK_BITS		8
@@ -54,7 +56,6 @@ struct instruction {
 
 	u32 idx			: INSN_CHUNK_BITS,
 	    dead_end		: 1,
-	    ignore		: 1,
 	    ignore_alts		: 1,
 	    hint		: 1,
 	    save		: 1,
tools/objtool/include/objtool/special.h +1 -3
index e049679bb17b..72d09c0adf1a 100644
@@ -16,8 +16,6 @@ struct special_alt {
 	struct list_head list;
 
 	bool group;
-	bool skip_orig;
-	bool skip_alt;
 	bool jump_or_nop;
 	u8 key_addend;
 
@@ -32,7 +30,7 @@ struct special_alt {
 
 int special_get_alts(struct elf *elf, struct list_head *alts);
 
-void arch_handle_alternative(unsigned short feature, struct special_alt *alt);
+void arch_handle_alternative(struct special_alt *alt);
 
 bool arch_support_alt_relocation(struct special_alt *special_alt,
 				 struct instruction *insn,
tools/objtool/special.c +2 -10
index 097a69db82a0..6cd7b1be5331 100644
@@ -54,7 +54,7 @@ static const struct special_entry entries[] = {
 	{},
 };
 
-void __weak arch_handle_alternative(unsigned short feature, struct special_alt *alt)
+void __weak arch_handle_alternative(struct special_alt *alt)
 {
 }
 
@@ -92,15 +92,7 @@ static int get_alt_entry(struct elf *elf, const struct special_entry *entry,
 
 	reloc_to_sec_off(orig_reloc, &alt->orig_sec, &alt->orig_off);
 
-	if (entry->feature) {
-		unsigned short feature;
-
-		feature = bswap_if_needed(elf,
-					  *(unsigned short *)(sec->data->d_buf +
-							      offset +
-							      entry->feature));
-		arch_handle_alternative(feature, alt);
-	}
+	arch_handle_alternative(alt);
 
 	if (!entry->group || alt->new_len) {
 		new_reloc = find_reloc_by_dest(elf, sec, offset + entry->new);
Signature · 1 line
2.48.1
merge bot Applied
#8 lore raw

[tip: objtool/urgent] objtool: Fix X86_FEATURE_SMAP alternative handling

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     1154bbd326de4453858cf78cf29420888b3ffd52
Gitweb:        https://git.kernel.org/tip/1154bbd326de4453858cf78cf29420888b3ffd52
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:55:54 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:26 +01:00

objtool: Fix X86_FEATURE_SMAP alternative handling

For X86_FEATURE_SMAP alternatives which replace NOP with STAC or CLAC, uaccess validation skips the NOP branch to avoid following impossible code paths, e.g. where a STAC would be patched but a CLAC wouldn't.

However, it's not safe to assume an X86_FEATURE_SMAP alternative is patching STAC/CLAC. There can be other alternatives, like static_cpu_has(), where both branches need to be validated.

Fix that by repurposing ANNOTATE_IGNORE_ALTERNATIVE for skipping either original instructions or new ones. This is a more generic approach which enables the removal of the feature checking hacks and the insn->ignore bit.

Fixes the following warnings:

arch/x86/mm/fault.o: warning: objtool: do_user_addr_fault+0x8ec: __stack_chk_fail() missing __noreturn in .c/.h or NORETURN() in noreturns.h arch/x86/mm/fault.o: warning: objtool: do_user_addr_fault+0x8f1: unreachable instruction

[ mingo: Fix up conflicts with recent x86 changes. ]

---
 arch/x86/include/asm/arch_hweight.h     |  6 ++--
 arch/x86/include/asm/smap.h             | 23 +++++++++-----
 arch/x86/include/asm/xen/hypercall.h    |  6 +---
 tools/objtool/arch/x86/special.c        | 33 +--------------------
 tools/objtool/check.c                   | 39 ++++++------------------
 tools/objtool/include/objtool/check.h   |  3 +-
 tools/objtool/include/objtool/special.h |  4 +--
 tools/objtool/special.c                 | 12 +------
 8 files changed, 37 insertions(+), 89 deletions(-)
arch/x86/include/asm/arch_hweight.h +4 -2
index b5982b9..cbc6157 100644
@@ -16,7 +16,8 @@ static __always_inline unsigned int __arch_hweight32(unsigned int w)
 {
 	unsigned int res;
 
-	asm_inline (ALTERNATIVE("call __sw_hweight32",
+	asm_inline (ALTERNATIVE(ANNOTATE_IGNORE_ALTERNATIVE
+				"call __sw_hweight32",
 				"popcntl %[val], %[cnt]", X86_FEATURE_POPCNT)
 			 : [cnt] "=" REG_OUT (res), ASM_CALL_CONSTRAINT
 			 : [val] REG_IN (w));
@@ -45,7 +46,8 @@ static __always_inline unsigned long __arch_hweight64(__u64 w)
 {
 	unsigned long res;
 
-	asm_inline (ALTERNATIVE("call __sw_hweight64",
+	asm_inline (ALTERNATIVE(ANNOTATE_IGNORE_ALTERNATIVE
+				"call __sw_hweight64",
 				"popcntq %[val], %[cnt]", X86_FEATURE_POPCNT)
 			 : [cnt] "=" REG_OUT (res), ASM_CALL_CONSTRAINT
 			 : [val] REG_IN (w));
arch/x86/include/asm/smap.h +15 -8
index daea94c..55a5e65 100644
@@ -16,23 +16,23 @@
 #ifdef __ASSEMBLER__
 
 #define ASM_CLAC \
-	ALTERNATIVE "", "clac", X86_FEATURE_SMAP
+	ALTERNATIVE __stringify(ANNOTATE_IGNORE_ALTERNATIVE), "clac", X86_FEATURE_SMAP
 
 #define ASM_STAC \
-	ALTERNATIVE "", "stac", X86_FEATURE_SMAP
+	ALTERNATIVE __stringify(ANNOTATE_IGNORE_ALTERNATIVE), "stac", X86_FEATURE_SMAP
 
 #else /* __ASSEMBLER__ */
 
 static __always_inline void clac(void)
 {
 	/* Note: a barrier is implicit in alternative() */
-	alternative("", "clac", X86_FEATURE_SMAP);
+	alternative(ANNOTATE_IGNORE_ALTERNATIVE "", "clac", X86_FEATURE_SMAP);
 }
 
 static __always_inline void stac(void)
 {
 	/* Note: a barrier is implicit in alternative() */
-	alternative("", "stac", X86_FEATURE_SMAP);
+	alternative(ANNOTATE_IGNORE_ALTERNATIVE "", "stac", X86_FEATURE_SMAP);
 }
 
 static __always_inline unsigned long smap_save(void)
@@ -40,7 +40,8 @@ static __always_inline unsigned long smap_save(void)
 	unsigned long flags;
 
 	asm volatile ("# smap_save\n\t"
-		      ALTERNATIVE("", "pushf; pop %0; " "clac" "\n\t",
+		      ALTERNATIVE(ANNOTATE_IGNORE_ALTERNATIVE
+				  "", "pushf; pop %0; clac",
 				  X86_FEATURE_SMAP)
 		      : "=rm" (flags) : : "memory", "cc");
 
@@ -50,16 +51,22 @@ static __always_inline unsigned long smap_save(void)
 static __always_inline void smap_restore(unsigned long flags)
 {
 	asm volatile ("# smap_restore\n\t"
-		      ALTERNATIVE("", "push %0; popf\n\t",
+		      ALTERNATIVE(ANNOTATE_IGNORE_ALTERNATIVE
+				  "", "push %0; popf",
 				  X86_FEATURE_SMAP)
 		      : : "g" (flags) : "memory", "cc");
 }
 
 /* These macros can be used in asm() statements */
 #define ASM_CLAC \
-	ALTERNATIVE("", "clac", X86_FEATURE_SMAP)
+	ALTERNATIVE(ANNOTATE_IGNORE_ALTERNATIVE "", "clac", X86_FEATURE_SMAP)
 #define ASM_STAC \
-	ALTERNATIVE("", "stac", X86_FEATURE_SMAP)
+	ALTERNATIVE(ANNOTATE_IGNORE_ALTERNATIVE "", "stac", X86_FEATURE_SMAP)
+
+#define ASM_CLAC_UNSAFE \
+	ALTERNATIVE("", ANNOTATE_IGNORE_ALTERNATIVE "clac", X86_FEATURE_SMAP)
+#define ASM_STAC_UNSAFE \
+	ALTERNATIVE("", ANNOTATE_IGNORE_ALTERNATIVE "stac", X86_FEATURE_SMAP)
 
 #endif /* __ASSEMBLER__ */
 
arch/x86/include/asm/xen/hypercall.h +2 -4
index 97771b9..59a62c3 100644
@@ -231,14 +231,12 @@ static __always_inline void __xen_stac(void)
 	 * Suppress objtool seeing the STAC/CLAC and getting confused about it
 	 * calling random code with AC=1.
 	 */
-	asm volatile(ANNOTATE_IGNORE_ALTERNATIVE
-		     ASM_STAC ::: "memory", "flags");
+	asm volatile(ASM_STAC_UNSAFE ::: "memory", "flags");
 }
 
 static __always_inline void __xen_clac(void)
 {
-	asm volatile(ANNOTATE_IGNORE_ALTERNATIVE
-		     ASM_CLAC ::: "memory", "flags");
+	asm volatile(ASM_CLAC_UNSAFE ::: "memory", "flags");
 }
 
 static inline long
tools/objtool/arch/x86/special.c +1 -32
index 5f46d4e..403e587 100644
@@ -5,10 +5,7 @@
 #include <objtool/builtin.h>
 #include <objtool/warn.h>
 
-#define X86_FEATURE_POPCNT (4 * 32 + 23)
-#define X86_FEATURE_SMAP   (9 * 32 + 20)
-
-void arch_handle_alternative(unsigned short feature, struct special_alt *alt)
+void arch_handle_alternative(struct special_alt *alt)
 {
 	static struct special_alt *group, *prev;
 
@@ -32,34 +29,6 @@ void arch_handle_alternative(unsigned short feature, struct special_alt *alt)
 	} else group = alt;
 
 	prev = alt;
-
-	switch (feature) {
-	case X86_FEATURE_SMAP:
-		/*
-		 * If UACCESS validation is enabled; force that alternative;
-		 * otherwise force it the other way.
-		 *
-		 * What we want to avoid is having both the original and the
-		 * alternative code flow at the same time, in that case we can
-		 * find paths that see the STAC but take the NOP instead of
-		 * CLAC and the other way around.
-		 */
-		if (opts.uaccess)
-			alt->skip_orig = true;
-		else
-			alt->skip_alt = true;
-		break;
-	case X86_FEATURE_POPCNT:
-		/*
-		 * It has been requested that we don't validate the !POPCNT
-		 * feature path which is a "very very small percentage of
-		 * machines".
-		 */
-		alt->skip_orig = true;
-		break;
-	default:
-		break;
-	}
 }
 
 bool arch_support_alt_relocation(struct special_alt *special_alt,
tools/objtool/check.c +10 -29
index f465020..b2f6a7f 100644
@@ -25,7 +25,6 @@
 struct alternative {
 	struct alternative *next;
 	struct instruction *insn;
-	bool skip_orig;
 };
 
 static unsigned long nr_cfi, nr_cfi_reused, nr_cfi_cache;
@@ -1696,6 +1695,7 @@ static int handle_group_alt(struct objtool_file *file,
 		orig_alt_group->first_insn = orig_insn;
 		orig_alt_group->last_insn = last_orig_insn;
 		orig_alt_group->nop = NULL;
+		orig_alt_group->ignore = orig_insn->ignore_alts;
 	} else {
 		if (orig_alt_group->last_insn->offset + orig_alt_group->last_insn->len -
 		    orig_alt_group->first_insn->offset != special_alt->orig_len) {
@@ -1735,7 +1735,6 @@ static int handle_group_alt(struct objtool_file *file,
 		nop->type = INSN_NOP;
 		nop->sym = orig_insn->sym;
 		nop->alt_group = new_alt_group;
-		nop->ignore = orig_insn->ignore_alts;
 	}
 
 	if (!special_alt->new_len) {
@@ -1752,7 +1751,6 @@ static int handle_group_alt(struct objtool_file *file,
 
 		last_new_insn = insn;
 
-		insn->ignore = orig_insn->ignore_alts;
 		insn->sym = orig_insn->sym;
 		insn->alt_group = new_alt_group;
 
@@ -1799,6 +1797,7 @@ end:
 	new_alt_group->first_insn = *new_insn;
 	new_alt_group->last_insn = last_new_insn;
 	new_alt_group->nop = nop;
+	new_alt_group->ignore = (*new_insn)->ignore_alts;
 	new_alt_group->cfi = orig_alt_group->cfi;
 	return 0;
 }
@@ -1916,8 +1915,6 @@ static int add_special_section_alts(struct objtool_file *file)
 		}
 
 		alt->insn = new_insn;
-		alt->skip_orig = special_alt->skip_orig;
-		orig_insn->ignore_alts |= special_alt->skip_alt;
 		alt->next = orig_insn->alts;
 		orig_insn->alts = alt;
 
@@ -2295,6 +2292,8 @@ static int read_annotate(struct objtool_file *file,
 static int __annotate_early(struct objtool_file *file, int type, struct instruction *insn)
 {
 	switch (type) {
+
+	/* Must be before add_special_section_alts() */
 	case ANNOTYPE_IGNORE_ALTS:
 		insn->ignore_alts = true;
 		break;
@@ -3488,11 +3487,6 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 			return 1;
 		}
 
-		if (func && insn->ignore) {
-			WARN_INSN(insn, "BUG: why am I validating an ignored function?");
-			return 1;
-		}
-
 		visited = VISITED_BRANCH << state.uaccess;
 		if (insn->visited & VISITED_BRANCH_MASK) {
 			if (!insn->hint && !insn_cfi_match(insn, &state.cfi))
@@ -3564,24 +3558,19 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 		if (propagate_alt_cfi(file, insn))
 			return 1;
 
-		if (!insn->ignore_alts && insn->alts) {
-			bool skip_orig = false;
-
+		if (insn->alts) {
 			for (alt = insn->alts; alt; alt = alt->next) {
-				if (alt->skip_orig)
-					skip_orig = true;
-
 				ret = validate_branch(file, func, alt->insn, state);
 				if (ret) {
 					BT_INSN(insn, "(alt)");
 					return ret;
 				}
 			}
-
-			if (skip_orig)
-				return 0;
 		}
 
+		if (insn->alt_group && insn->alt_group->ignore)
+			return 0;
+
 		if (handle_insn_ops(insn, next_insn, &state))
 			return 1;
 
@@ -3768,23 +3757,15 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn)
 
 		insn->visited |= VISITED_UNRET;
 
-		if (!insn->ignore_alts && insn->alts) {
+		if (insn->alts) {
 			struct alternative *alt;
-			bool skip_orig = false;
-
 			for (alt = insn->alts; alt; alt = alt->next) {
-				if (alt->skip_orig)
-					skip_orig = true;
-
 				ret = validate_unret(file, alt->insn);
 				if (ret) {
 					BT_INSN(insn, "(alt)");
 					return ret;
 				}
 			}
-
-			if (skip_orig)
-				return 0;
 		}
 
 		switch (insn->type) {
@@ -3935,7 +3916,7 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
 	struct instruction *prev_insn;
 	int i;
 
-	if (insn->ignore || insn->type == INSN_NOP || insn->type == INSN_TRAP || (func && func->ignore))
+	if (insn->type == INSN_NOP || insn->type == INSN_TRAP || (func && func->ignore))
 		return true;
 
 	/*
tools/objtool/include/objtool/check.h +2 -1
index e1cd13c..00fb745 100644
@@ -34,6 +34,8 @@ struct alt_group {
 	 * This is shared with the other alt_groups in the same alternative.
 	 */
 	struct cfi_state **cfi;
+
+	bool ignore;
 };
 
 #define INSN_CHUNK_BITS		8
@@ -54,7 +56,6 @@ struct instruction {
 
 	u32 idx			: INSN_CHUNK_BITS,
 	    dead_end		: 1,
-	    ignore		: 1,
 	    ignore_alts		: 1,
 	    hint		: 1,
 	    save		: 1,
tools/objtool/include/objtool/special.h +1 -3
index e049679..72d09c0 100644
@@ -16,8 +16,6 @@ struct special_alt {
 	struct list_head list;
 
 	bool group;
-	bool skip_orig;
-	bool skip_alt;
 	bool jump_or_nop;
 	u8 key_addend;
 
@@ -32,7 +30,7 @@ struct special_alt {
 
 int special_get_alts(struct elf *elf, struct list_head *alts);
 
-void arch_handle_alternative(unsigned short feature, struct special_alt *alt);
+void arch_handle_alternative(struct special_alt *alt);
 
 bool arch_support_alt_relocation(struct special_alt *special_alt,
 				 struct instruction *insn,
tools/objtool/special.c +2 -10
index 097a69d..6cd7b1b 100644
@@ -54,7 +54,7 @@ static const struct special_entry entries[] = {
 	{},
 };
 
-void __weak arch_handle_alternative(unsigned short feature, struct special_alt *alt)
+void __weak arch_handle_alternative(struct special_alt *alt)
 {
 }
 
@@ -92,15 +92,7 @@ static int get_alt_entry(struct elf *elf, const struct special_entry *entry,
 
 	reloc_to_sec_off(orig_reloc, &alt->orig_sec, &alt->orig_off);
 
-	if (entry->feature) {
-		unsigned short feature;
-
-		feature = bswap_if_needed(elf,
-					  *(unsigned short *)(sec->data->d_buf +
-							      offset +
-							      entry->feature));
-		arch_handle_alternative(feature, alt);
-	}
+	arch_handle_alternative(alt);
 
 	if (!entry->group || alt->new_len) {
 		new_reloc = find_reloc_by_dest(elf, sec, offset + entry->new);
author maintainer
#9 lore raw

PATCH 5/22 objtool: Fix CONFIG_OBJTOOL_WERROR for vmlinux.o

In reply to Josh Poimboeuf

With (!X86_KERNEL_IBT && !LTO_CLANG && NOINSTR_VALIDATION), objtool runs on both translation units and vmlinux.o. With CONFIG_OBJTOOL_WERROR, the TUs get --Werror but vmlinux.o doesn't. Fix that.

---
 scripts/Makefile.vmlinux_o | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
scripts/Makefile.vmlinux_o +11 -3
index 0b6e2ebf60dc..f476f5605029 100644
@@ -30,12 +30,20 @@ endif
 # objtool for vmlinux.o
 # ---------------------------------------------------------------------------
 #
-# For LTO and IBT, objtool doesn't run on individual translation units.
-# Run everything on vmlinux instead.
+# For delay-objtool (IBT or LTO), objtool doesn't run on individual translation
+# units.  Instead it runs on vmlinux.o.
+#
+# For !delay-objtool + CONFIG_NOINSTR_VALIDATION, it runs on both translation
+# units and vmlinux.o, with the latter only used for noinstr/unret validation.
 
 objtool-enabled := $(or $(delay-objtool),$(CONFIG_NOINSTR_VALIDATION))
 
-vmlinux-objtool-args-$(delay-objtool)			+= $(objtool-args-y)
+ifeq ($(delay-objtool),y)
+vmlinux-objtool-args-y					+= $(objtool-args-y)
+else
+vmlinux-objtool-args-$(CONFIG_OBJTOOL_WERROR)		+= --Werror
+endif
+
 vmlinux-objtool-args-$(CONFIG_GCOV_KERNEL)		+= --no-unreachable
 vmlinux-objtool-args-$(CONFIG_NOINSTR_VALIDATION)	+= --noinstr \
 							   $(if $(or $(CONFIG_MITIGATION_UNRET_ENTRY),$(CONFIG_MITIGATION_SRSO)), --unret)
Signature · 1 line
2.48.1
merge bot Applied
#10 lore raw

[tip: objtool/urgent] objtool: Fix CONFIG_OBJTOOL_WERROR for vmlinux.o

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     4759670bc3e670069c055c2b33174813099fea4f
Gitweb:        https://git.kernel.org/tip/4759670bc3e670069c055c2b33174813099fea4f
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:55:55 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:26 +01:00

objtool: Fix CONFIG_OBJTOOL_WERROR for vmlinux.o

With (!X86_KERNEL_IBT && !LTO_CLANG && NOINSTR_VALIDATION), objtool runs on both translation units and vmlinux.o. With CONFIG_OBJTOOL_WERROR, the TUs get --Werror but vmlinux.o doesn't. Fix that.

---
 scripts/Makefile.vmlinux_o | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
scripts/Makefile.vmlinux_o +11 -3
index 0b6e2eb..f476f56 100644
@@ -30,12 +30,20 @@ endif
 # objtool for vmlinux.o
 # ---------------------------------------------------------------------------
 #
-# For LTO and IBT, objtool doesn't run on individual translation units.
-# Run everything on vmlinux instead.
+# For delay-objtool (IBT or LTO), objtool doesn't run on individual translation
+# units.  Instead it runs on vmlinux.o.
+#
+# For !delay-objtool + CONFIG_NOINSTR_VALIDATION, it runs on both translation
+# units and vmlinux.o, with the latter only used for noinstr/unret validation.
 
 objtool-enabled := $(or $(delay-objtool),$(CONFIG_NOINSTR_VALIDATION))
 
-vmlinux-objtool-args-$(delay-objtool)			+= $(objtool-args-y)
+ifeq ($(delay-objtool),y)
+vmlinux-objtool-args-y					+= $(objtool-args-y)
+else
+vmlinux-objtool-args-$(CONFIG_OBJTOOL_WERROR)		+= --Werror
+endif
+
 vmlinux-objtool-args-$(CONFIG_GCOV_KERNEL)		+= --no-unreachable
 vmlinux-objtool-args-$(CONFIG_NOINSTR_VALIDATION)	+= --noinstr \
 							   $(if $(or $(CONFIG_MITIGATION_UNRET_ENTRY),$(CONFIG_MITIGATION_SRSO)), --unret)
author maintainer
#11 lore raw

PATCH 6/22 objtool: Fix init_module() handling

In reply to Josh Poimboeuf

If IBT is enabled and a module uses the deprecated init_module() magic function name rather than module_init(fn), its ENDBR will get removed, causing an IBT failure during module load.

Objtool does print an obscure warning, but then does nothing to either correct it or return an error.

Improve the usefulness of the warning and return an error so it will at least fail the build with CONFIG_OBJTOOL_WERROR.

---
 tools/objtool/check.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
tools/objtool/check.c +5 -2
index bac88ca82dec..8edb7044f7fc 100644
@@ -828,8 +828,11 @@ static int create_ibt_endbr_seal_sections(struct objtool_file *file)
 		if (opts.module && sym && sym->type == STT_FUNC &&
 		    insn->offset == sym->offset &&
 		    (!strcmp(sym->name, "init_module") ||
-		     !strcmp(sym->name, "cleanup_module")))
-			WARN("%s(): not an indirect call target", sym->name);
+		     !strcmp(sym->name, "cleanup_module"))) {
+			WARN("%s(): Magic init_module() function name is deprecated, use module_init(fn) instead",
+			     sym->name);
+			return -1;
+		}
 
 		if (!elf_init_reloc_text_sym(file->elf, sec,
 					     idx * sizeof(int), idx,
Signature · 1 line
2.48.1
merge bot Applied
#12 lore raw

[tip: objtool/urgent] objtool: Fix init_module() handling

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     4fab2d7628dd38f3fa8a5e615199350ecaeb17a8
Gitweb:        https://git.kernel.org/tip/4fab2d7628dd38f3fa8a5e615199350ecaeb17a8
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:55:56 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:26 +01:00

objtool: Fix init_module() handling

If IBT is enabled and a module uses the deprecated init_module() magic function name rather than module_init(fn), its ENDBR will get removed, causing an IBT failure during module load.

Objtool does print an obscure warning, but then does nothing to either correct it or return an error.

Improve the usefulness of the warning and return an error so it will at least fail the build with CONFIG_OBJTOOL_WERROR.

---
 tools/objtool/check.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
tools/objtool/check.c +5 -2
index b2f6a7f..2f7aff1 100644
@@ -828,8 +828,11 @@ static int create_ibt_endbr_seal_sections(struct objtool_file *file)
 		if (opts.module && sym && sym->type == STT_FUNC &&
 		    insn->offset == sym->offset &&
 		    (!strcmp(sym->name, "init_module") ||
-		     !strcmp(sym->name, "cleanup_module")))
-			WARN("%s(): not an indirect call target", sym->name);
+		     !strcmp(sym->name, "cleanup_module"))) {
+			WARN("%s(): Magic init_module() function name is deprecated, use module_init(fn) instead",
+			     sym->name);
+			return -1;
+		}
 
 		if (!elf_init_reloc_text_sym(file->elf, sec,
 					     idx * sizeof(int), idx,
author maintainer
#13 lore raw

PATCH 7/22 objtool: Silence more KCOV warnings

In reply to Josh Poimboeuf

In the past there were issues with KCOV triggering unreachable instruction warnings, which is why unreachable warnings are now disabled with CONFIG_KCOV.

Now some new KCOV warnings are showing up with GCC 14:

vmlinux.o: warning: objtool: cpuset_write_resmask() falls through to next function cpuset_update_active_cpus.cold() drivers/usb/core/driver.o: error: objtool: usb_deregister() falls through to next function usb_match_device() sound/soc/codecs/snd-soc-wcd934x.o: warning: objtool: .text.wcd934x_slim_irq_handler: unexpected end of section

All are caused by GCC KCOV not finishing an optimization, leaving behind a never-taken conditional branch to a basic block which falls through to the next function (or end of section).

At a high level this is similar to the unreachable warnings mentioned above, in that KCOV isn't fully removing dead code. Treat it the same way by adding these to the list of warnings to ignore with CONFIG_KCOV.

---
 tools/objtool/check.c | 6 ++++++
 1 file changed, 6 insertions(+)
tools/objtool/check.c +6 -0
index 8edb7044f7fc..032b3be409f2 100644
@@ -3493,6 +3493,9 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 			    !strncmp(func->name, "__pfx_", 6))
 				return 0;
 
+			if (file->ignore_unreachables)
+				return 0;
+
 			WARN("%s() falls through to next function %s()",
 			     func->name, insn_func(insn)->name);
 			return 1;
@@ -3702,6 +3705,9 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 		if (!next_insn) {
 			if (state.cfi.cfa.base == CFI_UNDEFINED)
 				return 0;
+			if (file->ignore_unreachables)
+				return 0;
+
 			WARN("%s: unexpected end of section", sec->name);
 			return 1;
 		}
Signature · 1 line
2.48.1
merge bot Applied
#14 lore raw

[tip: objtool/urgent] objtool: Silence more KCOV warnings

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     6b023c7842048c4bbeede802f3cf36b96c7a8b25
Gitweb:        https://git.kernel.org/tip/6b023c7842048c4bbeede802f3cf36b96c7a8b25
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:55:57 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:26 +01:00

objtool: Silence more KCOV warnings

In the past there were issues with KCOV triggering unreachable instruction warnings, which is why unreachable warnings are now disabled with CONFIG_KCOV.

Now some new KCOV warnings are showing up with GCC 14:

vmlinux.o: warning: objtool: cpuset_write_resmask() falls through to next function cpuset_update_active_cpus.cold() drivers/usb/core/driver.o: error: objtool: usb_deregister() falls through to next function usb_match_device() sound/soc/codecs/snd-soc-wcd934x.o: warning: objtool: .text.wcd934x_slim_irq_handler: unexpected end of section

All are caused by GCC KCOV not finishing an optimization, leaving behind a never-taken conditional branch to a basic block which falls through to the next function (or end of section).

At a high level this is similar to the unreachable warnings mentioned above, in that KCOV isn't fully removing dead code. Treat it the same way by adding these to the list of warnings to ignore with CONFIG_KCOV.

---
 tools/objtool/check.c | 6 ++++++
 1 file changed, 6 insertions(+)
tools/objtool/check.c +6 -0
index 2f7aff1..cb66e6b 100644
@@ -3485,6 +3485,9 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 			    !strncmp(func->name, "__pfx_", 6))
 				return 0;
 
+			if (file->ignore_unreachables)
+				return 0;
+
 			WARN("%s() falls through to next function %s()",
 			     func->name, insn_func(insn)->name);
 			return 1;
@@ -3694,6 +3697,9 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 		if (!next_insn) {
 			if (state.cfi.cfa.base == CFI_UNDEFINED)
 				return 0;
+			if (file->ignore_unreachables)
+				return 0;
+
 			WARN("%s: unexpected end of section", sec->name);
 			return 1;
 		}
author maintainer
#15 lore raw

PATCH 8/22 objtool: Properly disable uaccess validation

In reply to Josh Poimboeuf

If opts.uaccess isn't set, the uaccess validation is disabled, but only
partially: it doesn't read the uaccess_safe_builtin list but still tries to do the validation. Disable it completely to prevent false warnings.

---
 tools/objtool/check.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
tools/objtool/check.c +9 -2
index 032b3be409f2..3966fc81a77e 100644
@@ -3195,7 +3195,7 @@ static int handle_insn_ops(struct instruction *insn,
 		if (update_cfi_state(insn, next_insn, &state->cfi, op))
 			return 1;
 
-		if (!insn->alt_group)
+		if (!opts.uaccess || !insn->alt_group)
 			continue;
 
 		if (op->dest.type == OP_DEST_PUSHF) {
@@ -3655,6 +3655,9 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 			return 0;
 
 		case INSN_STAC:
+			if (!opts.uaccess)
+				break;
+
 			if (state.uaccess) {
 				WARN_INSN(insn, "recursive UACCESS enable");
 				return 1;
@@ -3664,6 +3667,9 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 			break;
 
 		case INSN_CLAC:
+			if (!opts.uaccess)
+				break;
+
 			if (!state.uaccess && func) {
 				WARN_INSN(insn, "redundant UACCESS disable");
 				return 1;
@@ -4122,7 +4128,8 @@ static int validate_symbol(struct objtool_file *file, struct section *sec,
 	if (!insn || insn->visited)
 		return 0;
 
-	state->uaccess = sym->uaccess_safe;
+	if (opts.uaccess)
+		state->uaccess = sym->uaccess_safe;
 
 	ret = validate_branch(file, insn_func(insn), insn, *state);
 	if (ret)
Signature · 1 line
2.48.1
merge bot Applied
#16 lore raw

[tip: objtool/urgent] objtool: Properly disable uaccess validation

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     e1a9dda74dbffbc3fa2069ff418a1876dc99fb14
Gitweb:        https://git.kernel.org/tip/e1a9dda74dbffbc3fa2069ff418a1876dc99fb14
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:55:58 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:26 +01:00

objtool: Properly disable uaccess validation

If opts.uaccess isn't set, the uaccess validation is disabled, but only
partially: it doesn't read the uaccess_safe_builtin list but still tries to do the validation. Disable it completely to prevent false warnings.

---
 tools/objtool/check.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
tools/objtool/check.c +9 -2
index cb66e6b..b0ef14a 100644
@@ -3187,7 +3187,7 @@ static int handle_insn_ops(struct instruction *insn,
 		if (update_cfi_state(insn, next_insn, &state->cfi, op))
 			return 1;
 
-		if (!insn->alt_group)
+		if (!opts.uaccess || !insn->alt_group)
 			continue;
 
 		if (op->dest.type == OP_DEST_PUSHF) {
@@ -3647,6 +3647,9 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 			return 0;
 
 		case INSN_STAC:
+			if (!opts.uaccess)
+				break;
+
 			if (state.uaccess) {
 				WARN_INSN(insn, "recursive UACCESS enable");
 				return 1;
@@ -3656,6 +3659,9 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 			break;
 
 		case INSN_CLAC:
+			if (!opts.uaccess)
+				break;
+
 			if (!state.uaccess && func) {
 				WARN_INSN(insn, "redundant UACCESS disable");
 				return 1;
@@ -4114,7 +4120,8 @@ static int validate_symbol(struct objtool_file *file, struct section *sec,
 	if (!insn || insn->visited)
 		return 0;
 
-	state->uaccess = sym->uaccess_safe;
+	if (opts.uaccess)
+		state->uaccess = sym->uaccess_safe;
 
 	ret = validate_branch(file, insn_func(insn), insn, *state);
 	if (ret)
author maintainer
#17 lore raw

PATCH 9/22 objtool: Improve error handling

In reply to Josh Poimboeuf

Fix some error handling issues, improve error messages, properly distinguish betwee errors and warnings, and generally try to make all the error handling more consistent.

---
 tools/objtool/builtin-check.c           |  37 ++-
 tools/objtool/check.c                   | 370 ++++++++++++------------
 tools/objtool/elf.c                     |  22 +-
 tools/objtool/include/objtool/objtool.h |   2 +-
 tools/objtool/include/objtool/warn.h    |  13 +-
 tools/objtool/objtool.c                 |  11 +-
 6 files changed, 233 insertions(+), 222 deletions(-)
tools/objtool/builtin-check.c +17 -20
index 5f761f420b8c..c973a751fb7d 100644
@@ -8,15 +8,12 @@
 #include <stdlib.h>
 #include <fcntl.h>
 #include <unistd.h>
+#include <errno.h>
 #include <sys/stat.h>
 #include <sys/sendfile.h>
 #include <objtool/builtin.h>
 #include <objtool/objtool.h>
-
-#define ERROR(format, ...)				\
-	fprintf(stderr,					\
-		"error: objtool: " format "\n",		\
-		##__VA_ARGS__)
+#include <objtool/warn.h>
 
 const char *objname;
 
@@ -139,22 +136,22 @@ int cmd_parse_options(int argc, const char **argv, const char * const usage[])
 static bool opts_valid(void)
 {
 	if (opts.mnop && !opts.mcount) {
-		ERROR("--mnop requires --mcount");
+		WARN("--mnop requires --mcount");
 		return false;
 	}
 
 	if (opts.noinstr && !opts.link) {
-		ERROR("--noinstr requires --link");
+		WARN("--noinstr requires --link");
 		return false;
 	}
 
 	if (opts.ibt && !opts.link) {
-		ERROR("--ibt requires --link");
+		WARN("--ibt requires --link");
 		return false;
 	}
 
 	if (opts.unret && !opts.link) {
-		ERROR("--unret requires --link");
+		WARN("--unret requires --link");
 		return false;
 	}
 
@@ -171,7 +168,7 @@ static bool opts_valid(void)
 	    opts.static_call		||
 	    opts.uaccess) {
 		if (opts.dump_orc) {
-			ERROR("--dump can't be combined with other actions");
+			WARN("--dump can't be combined with other actions");
 			return false;
 		}
 
@@ -181,7 +178,7 @@ static bool opts_valid(void)
 	if (opts.dump_orc)
 		return true;
 
-	ERROR("At least one action required");
+	WARN("At least one action required");
 	return false;
 }
 
@@ -194,30 +191,30 @@ static int copy_file(const char *src, const char *dst)
 
 	src_fd = open(src, O_RDONLY);
 	if (src_fd == -1) {
-		ERROR("can't open '%s' for reading", src);
+		WARN("can't open %s for reading: %s", src, strerror(errno));
 		return 1;
 	}
 
 	dst_fd = open(dst, O_WRONLY | O_CREAT | O_TRUNC, 0400);
 	if (dst_fd == -1) {
-		ERROR("can't open '%s' for writing", dst);
+		WARN("can't open %s for writing: %s", dst, strerror(errno));
 		return 1;
 	}
 
 	if (fstat(src_fd, &stat) == -1) {
-		perror("fstat");
+		WARN_GLIBC("fstat");
 		return 1;
 	}
 
 	if (fchmod(dst_fd, stat.st_mode) == -1) {
-		perror("fchmod");
+		WARN_GLIBC("fchmod");
 		return 1;
 	}
 
 	for (to_copy = stat.st_size; to_copy > 0; to_copy -= copied) {
 		copied = sendfile(dst_fd, src_fd, &offset, to_copy);
 		if (copied == -1) {
-			perror("sendfile");
+			WARN_GLIBC("sendfile");
 			return 1;
 		}
 	}
@@ -233,14 +230,14 @@ static char **save_argv(int argc, const char **argv)
 
 	orig_argv = calloc(argc, sizeof(char *));
 	if (!orig_argv) {
-		perror("calloc");
+		WARN_GLIBC("calloc");
 		return NULL;
 	}
 
 	for (int i = 0; i < argc; i++) {
 		orig_argv[i] = strdup(argv[i]);
 		if (!orig_argv[i]) {
-			perror("strdup");
+			WARN_GLIBC("strdup(%s)", orig_argv[i]);
 			return NULL;
 		}
 	};
@@ -285,7 +282,7 @@ int objtool_run(int argc, const char **argv)
 		goto err;
 
 	if (!opts.link && has_multiple_files(file->elf)) {
-		ERROR("Linked object requires --link");
+		WARN("Linked object requires --link");
 		goto err;
 	}
 
@@ -313,7 +310,7 @@ int objtool_run(int argc, const char **argv)
 	 */
 	backup = malloc(strlen(objname) + strlen(ORIG_SUFFIX) + 1);
 	if (!backup) {
-		perror("malloc");
+		WARN_GLIBC("malloc");
 		return 1;
 	}
 
tools/objtool/check.c +189 -181

906 lines not on this page · raw message

tools/objtool/elf.c +11 -11
index 0f38167bd840..b352a78b596c 100644
@@ -331,7 +331,7 @@ static int read_sections(struct elf *elf)
 
 	elf->section_data = calloc(sections_nr, sizeof(*sec));
 	if (!elf->section_data) {
-		perror("calloc");
+		WARN_GLIBC("calloc");
 		return -1;
 	}
 	for (i = 0; i < sections_nr; i++) {
@@ -467,7 +467,7 @@ static int read_symbols(struct elf *elf)
 
 	elf->symbol_data = calloc(symbols_nr, sizeof(*sym));
 	if (!elf->symbol_data) {
-		perror("calloc");
+		WARN_GLIBC("calloc");
 		return -1;
 	}
 	for (i = 0; i < symbols_nr; i++) {
@@ -799,7 +799,7 @@ elf_create_section_symbol(struct elf *elf, struct section *sec)
 	struct symbol *sym = calloc(1, sizeof(*sym));
 
 	if (!sym) {
-		perror("malloc");
+		WARN_GLIBC("malloc");
 		return NULL;
 	}
 
@@ -829,7 +829,7 @@ elf_create_prefix_symbol(struct elf *elf, struct symbol *orig, long size)
 	char *name = malloc(namelen);
 
 	if (!sym || !name) {
-		perror("malloc");
+		WARN_GLIBC("malloc");
 		return NULL;
 	}
 
@@ -963,7 +963,7 @@ static int read_relocs(struct elf *elf)
 		nr_reloc = 0;
 		rsec->relocs = calloc(sec_num_entries(rsec), sizeof(*reloc));
 		if (!rsec->relocs) {
-			perror("calloc");
+			WARN_GLIBC("calloc");
 			return -1;
 		}
 		for (i = 0; i < sec_num_entries(rsec); i++) {
@@ -1005,7 +1005,7 @@ struct elf *elf_open_read(const char *name, int flags)
 
 	elf = malloc(sizeof(*elf));
 	if (!elf) {
-		perror("malloc");
+		WARN_GLIBC("malloc");
 		return NULL;
 	}
 	memset(elf, 0, sizeof(*elf));
@@ -1099,7 +1099,7 @@ struct section *elf_create_section(struct elf *elf, const char *name,
 
 	sec = malloc(sizeof(*sec));
 	if (!sec) {
-		perror("malloc");
+		WARN_GLIBC("malloc");
 		return NULL;
 	}
 	memset(sec, 0, sizeof(*sec));
@@ -1114,7 +1114,7 @@ struct section *elf_create_section(struct elf *elf, const char *name,
 
 	sec->name = strdup(name);
 	if (!sec->name) {
-		perror("strdup");
+		WARN_GLIBC("strdup");
 		return NULL;
 	}
 
@@ -1132,7 +1132,7 @@ struct section *elf_create_section(struct elf *elf, const char *name,
 	if (size) {
 		sec->data->d_buf = malloc(size);
 		if (!sec->data->d_buf) {
-			perror("malloc");
+			WARN_GLIBC("malloc");
 			return NULL;
 		}
 		memset(sec->data->d_buf, 0, size);
@@ -1179,7 +1179,7 @@ static struct section *elf_create_rela_section(struct elf *elf,
 
 	rsec_name = malloc(strlen(sec->name) + strlen(".rela") + 1);
 	if (!rsec_name) {
-		perror("malloc");
+		WARN_GLIBC("malloc");
 		return NULL;
 	}
 	strcpy(rsec_name, ".rela");
@@ -1199,7 +1199,7 @@ static struct section *elf_create_rela_section(struct elf *elf,
 
 	rsec->relocs = calloc(sec_num_entries(rsec), sizeof(struct reloc));
 	if (!rsec->relocs) {
-		perror("calloc");
+		WARN_GLIBC("calloc");
 		return NULL;
 	}
 
tools/objtool/include/objtool/objtool.h +1 -1
index 94a33ee7b363..c0dc86a78ff6 100644
@@ -41,7 +41,7 @@ struct objtool_file {
 
 struct objtool_file *objtool_open_read(const char *_objname);
 
-void objtool_pv_add(struct objtool_file *file, int idx, struct symbol *func);
+int objtool_pv_add(struct objtool_file *file, int idx, struct symbol *func);
 
 int check(struct objtool_file *file);
 int orc_dump(const char *objname);
tools/objtool/include/objtool/warn.h +9 -4
index e72b9d630551..b29ac144e4f5 100644
@@ -11,6 +11,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
+#include <errno.h>
 #include <objtool/builtin.h>
 #include <objtool/elf.h>
 
@@ -43,8 +44,9 @@ static inline char *offstr(struct section *sec, unsigned long offset)
 
 #define WARN(format, ...)				\
 	fprintf(stderr,					\
-		"%s: %s: objtool: " format "\n",	\
-		objname,				\
+		"%s%s%s: objtool: " format "\n",	\
+		objname ?: "",				\
+		objname ? ": " : "",			\
 		opts.werror ? "error" : "warning",	\
 		##__VA_ARGS__)
 
@@ -83,7 +85,10 @@ static inline char *offstr(struct section *sec, unsigned long offset)
 	}							\
 })
 
-#define WARN_ELF(format, ...)				\
-	WARN(format ": %s", ##__VA_ARGS__, elf_errmsg(-1))
+#define WARN_ELF(format, ...)					\
+	WARN("%s: " format " failed: %s", __func__, ##__VA_ARGS__, elf_errmsg(-1))
+
+#define WARN_GLIBC(format, ...)					\
+	WARN("%s: " format " failed: %s", __func__, ##__VA_ARGS__, strerror(errno))
 
 #endif /* _WARN_H */
tools/objtool/objtool.c +6 -5
index 1c73fb62fd57..e4b49c534e4d 100644
@@ -44,14 +44,14 @@ struct objtool_file *objtool_open_read(const char *filename)
 	return &file;
 }
 
-void objtool_pv_add(struct objtool_file *f, int idx, struct symbol *func)
+int objtool_pv_add(struct objtool_file *f, int idx, struct symbol *func)
 {
 	if (!opts.noinstr)
-		return;
+		return 0;
 
 	if (!f->pv_ops) {
 		WARN("paravirt confusion");
-		return;
+		return -1;
 	}
 
 	/*
@@ -60,14 +60,15 @@ void objtool_pv_add(struct objtool_file *f, int idx, struct symbol *func)
 	 */
 	if (!strcmp(func->name, "_paravirt_nop") ||
 	    !strcmp(func->name, "_paravirt_ident_64"))
-		return;
+		return 0;
 
 	/* already added this function */
 	if (!list_empty(&func->pv_target))
-		return;
+		return 0;
 
 	list_add(&func->pv_target, &f->pv_ops[idx].targets);
 	f->pv_ops[idx].clean = false;
+	return 0;
 }
 
 int main(int argc, const char **argv)
Signature · 1 line
2.48.1
merge bot Applied
#18 lore raw

[tip: objtool/urgent] objtool: Improve error handling

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     c5995abe15476798b2e2f0163a33404c41aafc8f
Gitweb:        https://git.kernel.org/tip/c5995abe15476798b2e2f0163a33404c41aafc8f
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:55:59 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:27 +01:00

objtool: Improve error handling

Fix some error handling issues, improve error messages, properly distinguish betwee errors and warnings, and generally try to make all the error handling more consistent.

---
 tools/objtool/builtin-check.c           |  37 +--
 tools/objtool/check.c                   | 368 +++++++++++------------
 tools/objtool/elf.c                     |  22 +-
 tools/objtool/include/objtool/objtool.h |   2 +-
 tools/objtool/include/objtool/warn.h    |  13 +-
 tools/objtool/objtool.c                 |  11 +-
 6 files changed, 232 insertions(+), 221 deletions(-)
tools/objtool/builtin-check.c +17 -20
index 5f761f4..c973a75 100644
@@ -8,15 +8,12 @@
 #include <stdlib.h>
 #include <fcntl.h>
 #include <unistd.h>
+#include <errno.h>
 #include <sys/stat.h>
 #include <sys/sendfile.h>
 #include <objtool/builtin.h>
 #include <objtool/objtool.h>
-
-#define ERROR(format, ...)				\
-	fprintf(stderr,					\
-		"error: objtool: " format "\n",		\
-		##__VA_ARGS__)
+#include <objtool/warn.h>
 
 const char *objname;
 
@@ -139,22 +136,22 @@ int cmd_parse_options(int argc, const char **argv, const char * const usage[])
 static bool opts_valid(void)
 {
 	if (opts.mnop && !opts.mcount) {
-		ERROR("--mnop requires --mcount");
+		WARN("--mnop requires --mcount");
 		return false;
 	}
 
 	if (opts.noinstr && !opts.link) {
-		ERROR("--noinstr requires --link");
+		WARN("--noinstr requires --link");
 		return false;
 	}
 
 	if (opts.ibt && !opts.link) {
-		ERROR("--ibt requires --link");
+		WARN("--ibt requires --link");
 		return false;
 	}
 
 	if (opts.unret && !opts.link) {
-		ERROR("--unret requires --link");
+		WARN("--unret requires --link");
 		return false;
 	}
 
@@ -171,7 +168,7 @@ static bool opts_valid(void)
 	    opts.static_call		||
 	    opts.uaccess) {
 		if (opts.dump_orc) {
-			ERROR("--dump can't be combined with other actions");
+			WARN("--dump can't be combined with other actions");
 			return false;
 		}
 
@@ -181,7 +178,7 @@ static bool opts_valid(void)
 	if (opts.dump_orc)
 		return true;
 
-	ERROR("At least one action required");
+	WARN("At least one action required");
 	return false;
 }
 
@@ -194,30 +191,30 @@ static int copy_file(const char *src, const char *dst)
 
 	src_fd = open(src, O_RDONLY);
 	if (src_fd == -1) {
-		ERROR("can't open '%s' for reading", src);
+		WARN("can't open %s for reading: %s", src, strerror(errno));
 		return 1;
 	}
 
 	dst_fd = open(dst, O_WRONLY | O_CREAT | O_TRUNC, 0400);
 	if (dst_fd == -1) {
-		ERROR("can't open '%s' for writing", dst);
+		WARN("can't open %s for writing: %s", dst, strerror(errno));
 		return 1;
 	}
 
 	if (fstat(src_fd, &stat) == -1) {
-		perror("fstat");
+		WARN_GLIBC("fstat");
 		return 1;
 	}
 
 	if (fchmod(dst_fd, stat.st_mode) == -1) {
-		perror("fchmod");
+		WARN_GLIBC("fchmod");
 		return 1;
 	}
 
 	for (to_copy = stat.st_size; to_copy > 0; to_copy -= copied) {
 		copied = sendfile(dst_fd, src_fd, &offset, to_copy);
 		if (copied == -1) {
-			perror("sendfile");
+			WARN_GLIBC("sendfile");
 			return 1;
 		}
 	}
@@ -233,14 +230,14 @@ static char **save_argv(int argc, const char **argv)
 
 	orig_argv = calloc(argc, sizeof(char *));
 	if (!orig_argv) {
-		perror("calloc");
+		WARN_GLIBC("calloc");
 		return NULL;
 	}
 
 	for (int i = 0; i < argc; i++) {
 		orig_argv[i] = strdup(argv[i]);
 		if (!orig_argv[i]) {
-			perror("strdup");
+			WARN_GLIBC("strdup(%s)", orig_argv[i]);
 			return NULL;
 		}
 	};
@@ -285,7 +282,7 @@ int objtool_run(int argc, const char **argv)
 		goto err;
 
 	if (!opts.link && has_multiple_files(file->elf)) {
-		ERROR("Linked object requires --link");
+		WARN("Linked object requires --link");
 		goto err;
 	}
 
@@ -313,7 +310,7 @@ err:
 	 */
 	backup = malloc(strlen(objname) + strlen(ORIG_SUFFIX) + 1);
 	if (!backup) {
-		perror("malloc");
+		WARN_GLIBC("malloc");
 		return 1;
 	}
 
tools/objtool/check.c +188 -180

901 lines not on this page · raw message

tools/objtool/elf.c +11 -11
index 0f38167..b352a78 100644
@@ -331,7 +331,7 @@ static int read_sections(struct elf *elf)
 
 	elf->section_data = calloc(sections_nr, sizeof(*sec));
 	if (!elf->section_data) {
-		perror("calloc");
+		WARN_GLIBC("calloc");
 		return -1;
 	}
 	for (i = 0; i < sections_nr; i++) {
@@ -467,7 +467,7 @@ static int read_symbols(struct elf *elf)
 
 	elf->symbol_data = calloc(symbols_nr, sizeof(*sym));
 	if (!elf->symbol_data) {
-		perror("calloc");
+		WARN_GLIBC("calloc");
 		return -1;
 	}
 	for (i = 0; i < symbols_nr; i++) {
@@ -799,7 +799,7 @@ elf_create_section_symbol(struct elf *elf, struct section *sec)
 	struct symbol *sym = calloc(1, sizeof(*sym));
 
 	if (!sym) {
-		perror("malloc");
+		WARN_GLIBC("malloc");
 		return NULL;
 	}
 
@@ -829,7 +829,7 @@ elf_create_prefix_symbol(struct elf *elf, struct symbol *orig, long size)
 	char *name = malloc(namelen);
 
 	if (!sym || !name) {
-		perror("malloc");
+		WARN_GLIBC("malloc");
 		return NULL;
 	}
 
@@ -963,7 +963,7 @@ static int read_relocs(struct elf *elf)
 		nr_reloc = 0;
 		rsec->relocs = calloc(sec_num_entries(rsec), sizeof(*reloc));
 		if (!rsec->relocs) {
-			perror("calloc");
+			WARN_GLIBC("calloc");
 			return -1;
 		}
 		for (i = 0; i < sec_num_entries(rsec); i++) {
@@ -1005,7 +1005,7 @@ struct elf *elf_open_read(const char *name, int flags)
 
 	elf = malloc(sizeof(*elf));
 	if (!elf) {
-		perror("malloc");
+		WARN_GLIBC("malloc");
 		return NULL;
 	}
 	memset(elf, 0, sizeof(*elf));
@@ -1099,7 +1099,7 @@ struct section *elf_create_section(struct elf *elf, const char *name,
 
 	sec = malloc(sizeof(*sec));
 	if (!sec) {
-		perror("malloc");
+		WARN_GLIBC("malloc");
 		return NULL;
 	}
 	memset(sec, 0, sizeof(*sec));
@@ -1114,7 +1114,7 @@ struct section *elf_create_section(struct elf *elf, const char *name,
 
 	sec->name = strdup(name);
 	if (!sec->name) {
-		perror("strdup");
+		WARN_GLIBC("strdup");
 		return NULL;
 	}
 
@@ -1132,7 +1132,7 @@ struct section *elf_create_section(struct elf *elf, const char *name,
 	if (size) {
 		sec->data->d_buf = malloc(size);
 		if (!sec->data->d_buf) {
-			perror("malloc");
+			WARN_GLIBC("malloc");
 			return NULL;
 		}
 		memset(sec->data->d_buf, 0, size);
@@ -1179,7 +1179,7 @@ static struct section *elf_create_rela_section(struct elf *elf,
 
 	rsec_name = malloc(strlen(sec->name) + strlen(".rela") + 1);
 	if (!rsec_name) {
-		perror("malloc");
+		WARN_GLIBC("malloc");
 		return NULL;
 	}
 	strcpy(rsec_name, ".rela");
@@ -1199,7 +1199,7 @@ static struct section *elf_create_rela_section(struct elf *elf,
 
 	rsec->relocs = calloc(sec_num_entries(rsec), sizeof(struct reloc));
 	if (!rsec->relocs) {
-		perror("calloc");
+		WARN_GLIBC("calloc");
 		return NULL;
 	}
 
tools/objtool/include/objtool/objtool.h +1 -1
index 94a33ee..c0dc86a 100644
@@ -41,7 +41,7 @@ struct objtool_file {
 
 struct objtool_file *objtool_open_read(const char *_objname);
 
-void objtool_pv_add(struct objtool_file *file, int idx, struct symbol *func);
+int objtool_pv_add(struct objtool_file *file, int idx, struct symbol *func);
 
 int check(struct objtool_file *file);
 int orc_dump(const char *objname);
tools/objtool/include/objtool/warn.h +9 -4
index e72b9d6..b29ac14 100644
@@ -11,6 +11,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
+#include <errno.h>
 #include <objtool/builtin.h>
 #include <objtool/elf.h>
 
@@ -43,8 +44,9 @@ static inline char *offstr(struct section *sec, unsigned long offset)
 
 #define WARN(format, ...)				\
 	fprintf(stderr,					\
-		"%s: %s: objtool: " format "\n",	\
-		objname,				\
+		"%s%s%s: objtool: " format "\n",	\
+		objname ?: "",				\
+		objname ? ": " : "",			\
 		opts.werror ? "error" : "warning",	\
 		##__VA_ARGS__)
 
@@ -83,7 +85,10 @@ static inline char *offstr(struct section *sec, unsigned long offset)
 	}							\
 })
 
-#define WARN_ELF(format, ...)				\
-	WARN(format ": %s", ##__VA_ARGS__, elf_errmsg(-1))
+#define WARN_ELF(format, ...)					\
+	WARN("%s: " format " failed: %s", __func__, ##__VA_ARGS__, elf_errmsg(-1))
+
+#define WARN_GLIBC(format, ...)					\
+	WARN("%s: " format " failed: %s", __func__, ##__VA_ARGS__, strerror(errno))
 
 #endif /* _WARN_H */
tools/objtool/objtool.c +6 -5
index 1c73fb6..e4b49c5 100644
@@ -44,14 +44,14 @@ struct objtool_file *objtool_open_read(const char *filename)
 	return &file;
 }
 
-void objtool_pv_add(struct objtool_file *f, int idx, struct symbol *func)
+int objtool_pv_add(struct objtool_file *f, int idx, struct symbol *func)
 {
 	if (!opts.noinstr)
-		return;
+		return 0;
 
 	if (!f->pv_ops) {
 		WARN("paravirt confusion");
-		return;
+		return -1;
 	}
 
 	/*
@@ -60,14 +60,15 @@ void objtool_pv_add(struct objtool_file *f, int idx, struct symbol *func)
 	 */
 	if (!strcmp(func->name, "_paravirt_nop") ||
 	    !strcmp(func->name, "_paravirt_ident_64"))
-		return;
+		return 0;
 
 	/* already added this function */
 	if (!list_empty(&func->pv_target))
-		return;
+		return 0;
 
 	list_add(&func->pv_target, &f->pv_ops[idx].targets);
 	f->pv_ops[idx].clean = false;
+	return 0;
 }
 
 int main(int argc, const char **argv)
author maintainer
#19 lore raw

PATCH 10/22 objtool: Reduce CONFIG_OBJTOOL_WERROR verbosity

In reply to Josh Poimboeuf

Remove the following from CONFIG_OBJTOOL_WERROR:

* backtrace

* "upgraded warnings to errors" message

* cmdline args

This makes the default output less cluttered and makes it easier to spot the actual warnings. Note the above options are still are available with --verbose or OBJTOOL_VERBOSE=1.

Also, do the cmdline arg printing on all warnings, regardless of werror.

---
 scripts/Makefile.lib                    |   2 +-
 tools/objtool/builtin-check.c           | 113 ++++++++++++------------
 tools/objtool/check.c                   |  23 ++---
 tools/objtool/include/objtool/builtin.h |   6 +-
 4 files changed, 73 insertions(+), 71 deletions(-)
scripts/Makefile.lib +1 -1
index 99e281966ba3..cdf45722626c 100644
@@ -277,7 +277,7 @@ objtool-args-$(CONFIG_HAVE_STATIC_CALL_INLINE)		+= --static-call
 objtool-args-$(CONFIG_HAVE_UACCESS_VALIDATION)		+= --uaccess
 objtool-args-$(CONFIG_GCOV_KERNEL)			+= --no-unreachable
 objtool-args-$(CONFIG_PREFIX_SYMBOLS)			+= --prefix=$(CONFIG_FUNCTION_PADDING_BYTES)
-objtool-args-$(CONFIG_OBJTOOL_WERROR)			+= --Werror --backtrace
+objtool-args-$(CONFIG_OBJTOOL_WERROR)			+= --Werror
 
 objtool-args = $(objtool-args-y)					\
 	$(if $(delay-objtool), --link)					\
tools/objtool/builtin-check.c +55 -58

161 lines not on this page · raw message

tools/objtool/check.c +13 -10
index fbe2a5ef2c40..03ec485a376a 100644
@@ -4740,9 +4740,6 @@ int check(struct objtool_file *file)
 
 	free_insns(file);
 
-	if (opts.verbose)
-		disas_warned_funcs(file);
-
 	if (opts.stats) {
 		printf("nr_insns_visited: %ld\n", nr_insns_visited);
 		printf("nr_cfi: %ld\n", nr_cfi);
@@ -4751,19 +4748,25 @@ int check(struct objtool_file *file)
 	}
 
 out:
+	if (!ret && !warnings)
+		return 0;
+
+	if (opts.verbose) {
+		if (opts.werror && warnings)
+			WARN("%d warning(s) upgraded to errors", warnings);
+		print_args();
+		disas_warned_funcs(file);
+	}
+
 	/*
 	 * CONFIG_OBJTOOL_WERROR upgrades all warnings (and errors) to actual
 	 * errors.
 	 *
-	 * Note that even "fatal" type errors don't actually return an error
-	 * without CONFIG_OBJTOOL_WERROR.  That probably needs improved at some
-	 * point.
+	 * Note that even fatal errors don't yet actually return an error
+	 * without CONFIG_OBJTOOL_WERROR.  That will be fixed soon-ish.
 	 */
-	if (opts.werror && (ret || warnings)) {
-		if (warnings)
-			WARN("%d warning(s) upgraded to errors", warnings);
+	if (opts.werror)
 		return 1;
-	}
 
 	return 0;
 }
tools/objtool/include/objtool/builtin.h +4 -2
index 0fafd0f7a209..6b08666fa69d 100644
@@ -43,8 +43,10 @@ struct opts {
 
 extern struct opts opts;
 
-extern int cmd_parse_options(int argc, const char **argv, const char * const usage[]);
+int cmd_parse_options(int argc, const char **argv, const char * const usage[]);
 
-extern int objtool_run(int argc, const char **argv);
+int objtool_run(int argc, const char **argv);
+
+void print_args(void);
 
 #endif /* _BUILTIN_H */
Signature · 1 line
2.48.1
merge bot Applied
#20 lore raw

[tip: objtool/urgent] objtool: Reduce CONFIG_OBJTOOL_WERROR verbosity

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     d39f82a058c0269392a797cb04f2a6064e5dcab6
Gitweb:        https://git.kernel.org/tip/d39f82a058c0269392a797cb04f2a6064e5dcab6
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:00 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:27 +01:00

objtool: Reduce CONFIG_OBJTOOL_WERROR verbosity

Remove the following from CONFIG_OBJTOOL_WERROR:

* backtrace

* "upgraded warnings to errors" message

* cmdline args

This makes the default output less cluttered and makes it easier to spot the actual warnings. Note the above options are still are available with --verbose or OBJTOOL_VERBOSE=1.

Also, do the cmdline arg printing on all warnings, regardless of werror.

---
 scripts/Makefile.lib                    |   2 +-
 tools/objtool/builtin-check.c           | 113 +++++++++++------------
 tools/objtool/check.c                   |  23 ++---
 tools/objtool/include/objtool/builtin.h |   6 +-
 4 files changed, 73 insertions(+), 71 deletions(-)
scripts/Makefile.lib +1 -1
index 57620b4..b935974 100644
@@ -277,7 +277,7 @@ objtool-args-$(CONFIG_HAVE_STATIC_CALL_INLINE)		+= --static-call
 objtool-args-$(CONFIG_HAVE_UACCESS_VALIDATION)		+= --uaccess
 objtool-args-$(CONFIG_GCOV_KERNEL)			+= --no-unreachable
 objtool-args-$(CONFIG_PREFIX_SYMBOLS)			+= --prefix=$(CONFIG_FUNCTION_PADDING_BYTES)
-objtool-args-$(CONFIG_OBJTOOL_WERROR)			+= --Werror --backtrace
+objtool-args-$(CONFIG_OBJTOOL_WERROR)			+= --Werror
 
 objtool-args = $(objtool-args-y)					\
 	$(if $(delay-objtool), --link)					\
tools/objtool/builtin-check.c +55 -58

161 lines not on this page · raw message

tools/objtool/check.c +13 -10
index f4e7ee8..ac21f28 100644
@@ -4732,9 +4732,6 @@ int check(struct objtool_file *file)
 
 	free_insns(file);
 
-	if (opts.verbose)
-		disas_warned_funcs(file);
-
 	if (opts.stats) {
 		printf("nr_insns_visited: %ld\n", nr_insns_visited);
 		printf("nr_cfi: %ld\n", nr_cfi);
@@ -4743,19 +4740,25 @@ int check(struct objtool_file *file)
 	}
 
 out:
+	if (!ret && !warnings)
+		return 0;
+
+	if (opts.verbose) {
+		if (opts.werror && warnings)
+			WARN("%d warning(s) upgraded to errors", warnings);
+		print_args();
+		disas_warned_funcs(file);
+	}
+
 	/*
 	 * CONFIG_OBJTOOL_WERROR upgrades all warnings (and errors) to actual
 	 * errors.
 	 *
-	 * Note that even "fatal" type errors don't actually return an error
-	 * without CONFIG_OBJTOOL_WERROR.  That probably needs improved at some
-	 * point.
+	 * Note that even fatal errors don't yet actually return an error
+	 * without CONFIG_OBJTOOL_WERROR.  That will be fixed soon-ish.
 	 */
-	if (opts.werror && (ret || warnings)) {
-		if (warnings)
-			WARN("%d warning(s) upgraded to errors", warnings);
+	if (opts.werror)
 		return 1;
-	}
 
 	return 0;
 }
tools/objtool/include/objtool/builtin.h +4 -2
index 0fafd0f..6b08666 100644
@@ -43,8 +43,10 @@ struct opts {
 
 extern struct opts opts;
 
-extern int cmd_parse_options(int argc, const char **argv, const char * const usage[]);
+int cmd_parse_options(int argc, const char **argv, const char * const usage[]);
 
-extern int objtool_run(int argc, const char **argv);
+int objtool_run(int argc, const char **argv);
+
+void print_args(void);
 
 #endif /* _BUILTIN_H */
author maintainer
#21 lore raw

PATCH 11/22 objtool: Fix up some outdated references to ENTRY/ENDPROC

In reply to Josh Poimboeuf

ENTRY and ENDPROC were deprecated years ago and replaced with SYM_FUNC_{START,END}. Fix up a few outdated references in the objtool documentation and comments. Also fix a few typos.

---
 include/linux/linkage.h                 |  4 ----
 include/linux/objtool.h                 |  2 +-
 tools/objtool/Documentation/objtool.txt | 10 +++++-----
 3 files changed, 6 insertions(+), 10 deletions(-)
include/linux/linkage.h +0 -4
index 5c8865bb59d9..b11660b706c5 100644
@@ -134,10 +134,6 @@
 	.size name, .-name
 #endif
 
-/* If symbol 'name' is treated as a subroutine (gets called, and returns)
- * then please use ENDPROC to mark 'name' as STT_FUNC for the benefit of
- * static analysis tools such as stack depth analyzer.
- */
 #ifndef ENDPROC
 /* deprecated, use SYM_FUNC_END */
 #define ENDPROC(name) \
include/linux/objtool.h +1 -1
index c722a921165b..248d9363ca85 100644
@@ -69,7 +69,7 @@
  * In asm, there are two kinds of code: normal C-type callable functions and
  * the rest.  The normal callable functions can be called by other code, and
  * don't do anything unusual with the stack.  Such normal callable functions
- * are annotated with the ENTRY/ENDPROC macros.  Most asm code falls in this
+ * are annotated with SYM_FUNC_{START,END}.  Most asm code falls in this
  * category.  In this case, no special debugging annotations are needed because
  * objtool can automatically generate the ORC data for the ORC unwinder to read
  * at runtime.
tools/objtool/Documentation/objtool.txt +5 -5
index 28ac57b9e102..9e97fc25b2d8 100644
@@ -34,7 +34,7 @@ Objtool has the following features:
 - Return thunk annotation -- annotates all return thunk sites so kernel
   can patch them inline, depending on enabled mitigations
 
-- Return thunk training valiation -- validate that all entry paths
+- Return thunk untraining validation -- validate that all entry paths
   untrain a "safe return" before the first return (or call)
 
 - Non-instrumentation validation -- validates non-instrumentable
@@ -281,8 +281,8 @@ the objtool maintainers.
    If the error is for an asm file, and func() is indeed a callable
    function, add proper frame pointer logic using the FRAME_BEGIN and
    FRAME_END macros.  Otherwise, if it's not a callable function, remove
-   its ELF function annotation by changing ENDPROC to END, and instead
-   use the manual unwind hint macros in asm/unwind_hints.h.
+   its ELF function annotation by using SYM_CODE_{START,END} and use the
+   manual unwind hint macros in asm/unwind_hints.h.
 
    If it's a GCC-compiled .c file, the error may be because the function
    uses an inline asm() statement which has a "call" instruction.  An
@@ -352,7 +352,7 @@ the objtool maintainers.
    This is a kernel entry/exit instruction like sysenter or iret.  Such
    instructions aren't allowed in a callable function, and are most
    likely part of the kernel entry code.  Such code should probably be
-   placed in a SYM_FUNC_CODE block with unwind hints.
+   placed in a SYM_CODE_{START,END} block with unwind hints.
 
 
 6. file.o: warning: objtool: func()+0x26: sibling call from callable instruction with modified stack frame
@@ -381,7 +381,7 @@ the objtool maintainers.
 
    Another possibility is that the code has some asm or inline asm which
    does some unusual things to the stack or the frame pointer.  In such
-   cases it's probably appropriate to use SYM_FUNC_CODE with unwind
+   cases it's probably appropriate to use SYM_CODE_{START,END} with unwind
    hints.
 
 
Signature · 1 line
2.48.1
merge bot Applied
#22 lore raw

[tip: objtool/urgent] objtool: Fix up some outdated references to ENTRY/ENDPROC

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     24fe172b50b5749c315349e740e4a09e3a0165d5
Gitweb:        https://git.kernel.org/tip/24fe172b50b5749c315349e740e4a09e3a0165d5
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:01 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:27 +01:00

objtool: Fix up some outdated references to ENTRY/ENDPROC

ENTRY and ENDPROC were deprecated years ago and replaced with SYM_FUNC_{START,END}. Fix up a few outdated references in the objtool documentation and comments. Also fix a few typos.

---
 include/linux/linkage.h                 |  4 ----
 include/linux/objtool.h                 |  2 +-
 tools/objtool/Documentation/objtool.txt | 10 +++++-----
 3 files changed, 6 insertions(+), 10 deletions(-)
include/linux/linkage.h +0 -4
index 5c8865b..b11660b 100644
@@ -134,10 +134,6 @@
 	.size name, .-name
 #endif
 
-/* If symbol 'name' is treated as a subroutine (gets called, and returns)
- * then please use ENDPROC to mark 'name' as STT_FUNC for the benefit of
- * static analysis tools such as stack depth analyzer.
- */
 #ifndef ENDPROC
 /* deprecated, use SYM_FUNC_END */
 #define ENDPROC(name) \
include/linux/objtool.h +1 -1
index 3ca965a..366ad00 100644
@@ -69,7 +69,7 @@
  * In asm, there are two kinds of code: normal C-type callable functions and
  * the rest.  The normal callable functions can be called by other code, and
  * don't do anything unusual with the stack.  Such normal callable functions
- * are annotated with the ENTRY/ENDPROC macros.  Most asm code falls in this
+ * are annotated with SYM_FUNC_{START,END}.  Most asm code falls in this
  * category.  In this case, no special debugging annotations are needed because
  * objtool can automatically generate the ORC data for the ORC unwinder to read
  * at runtime.
tools/objtool/Documentation/objtool.txt +5 -5
index 28ac57b..9e97fc2 100644
@@ -34,7 +34,7 @@ Objtool has the following features:
 - Return thunk annotation -- annotates all return thunk sites so kernel
   can patch them inline, depending on enabled mitigations
 
-- Return thunk training valiation -- validate that all entry paths
+- Return thunk untraining validation -- validate that all entry paths
   untrain a "safe return" before the first return (or call)
 
 - Non-instrumentation validation -- validates non-instrumentable
@@ -281,8 +281,8 @@ the objtool maintainers.
    If the error is for an asm file, and func() is indeed a callable
    function, add proper frame pointer logic using the FRAME_BEGIN and
    FRAME_END macros.  Otherwise, if it's not a callable function, remove
-   its ELF function annotation by changing ENDPROC to END, and instead
-   use the manual unwind hint macros in asm/unwind_hints.h.
+   its ELF function annotation by using SYM_CODE_{START,END} and use the
+   manual unwind hint macros in asm/unwind_hints.h.
 
    If it's a GCC-compiled .c file, the error may be because the function
    uses an inline asm() statement which has a "call" instruction.  An
@@ -352,7 +352,7 @@ the objtool maintainers.
    This is a kernel entry/exit instruction like sysenter or iret.  Such
    instructions aren't allowed in a callable function, and are most
    likely part of the kernel entry code.  Such code should probably be
-   placed in a SYM_FUNC_CODE block with unwind hints.
+   placed in a SYM_CODE_{START,END} block with unwind hints.
 
 
 6. file.o: warning: objtool: func()+0x26: sibling call from callable instruction with modified stack frame
@@ -381,7 +381,7 @@ the objtool maintainers.
 
    Another possibility is that the code has some asm or inline asm which
    does some unusual things to the stack or the frame pointer.  In such
-   cases it's probably appropriate to use SYM_FUNC_CODE with unwind
+   cases it's probably appropriate to use SYM_CODE_{START,END} with unwind
    hints.
 
 
author maintainer
#23 lore raw

PATCH 12/22 objtool: Remove --no-unreachable for noinstr-only vmlinux.o runs

In reply to Josh Poimboeuf

For (!X86_KERNEL_IBT && !LTO_CLANG && NOINSTR_VALIDATION), objtool runs on both translation units and vmlinux.o. The vmlinux.o run only does noinstr/noret validation. In that case --no-unreachable has no effect. Remove it.

Note that for ((X86_KERNEL_IBT || LTO_CLANG) && KCOV), --no-unreachable still gets set in objtool-args-y by scripts/Makefile.lib.

---
 scripts/Makefile.vmlinux_o | 1 -
 1 file changed, 1 deletion(-)
scripts/Makefile.vmlinux_o +0 -1
index f476f5605029..938c7457717e 100644
@@ -44,7 +44,6 @@ else
 vmlinux-objtool-args-$(CONFIG_OBJTOOL_WERROR)		+= --Werror
 endif
 
-vmlinux-objtool-args-$(CONFIG_GCOV_KERNEL)		+= --no-unreachable
 vmlinux-objtool-args-$(CONFIG_NOINSTR_VALIDATION)	+= --noinstr \
 							   $(if $(or $(CONFIG_MITIGATION_UNRET_ENTRY),$(CONFIG_MITIGATION_SRSO)), --unret)
 
Signature · 1 line
2.48.1
merge bot Applied
#24 lore raw

[tip: objtool/urgent] objtool: Remove --no-unreachable for noinstr-only vmlinux.o runs

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     876a4bce3849b235d752d13ec3180e15a35e52de
Gitweb:        https://git.kernel.org/tip/876a4bce3849b235d752d13ec3180e15a35e52de
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:02 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:27 +01:00

objtool: Remove --no-unreachable for noinstr-only vmlinux.o runs

For (!X86_KERNEL_IBT && !LTO_CLANG && NOINSTR_VALIDATION), objtool runs on both translation units and vmlinux.o. The vmlinux.o run only does noinstr/noret validation. In that case --no-unreachable has no effect. Remove it.

Note that for ((X86_KERNEL_IBT || LTO_CLANG) && KCOV), --no-unreachable still gets set in objtool-args-y by scripts/Makefile.lib.

---
 scripts/Makefile.vmlinux_o | 1 -
 1 file changed, 1 deletion(-)
scripts/Makefile.vmlinux_o +0 -1
index f476f56..938c745 100644
@@ -44,7 +44,6 @@ else
 vmlinux-objtool-args-$(CONFIG_OBJTOOL_WERROR)		+= --Werror
 endif
 
-vmlinux-objtool-args-$(CONFIG_GCOV_KERNEL)		+= --no-unreachable
 vmlinux-objtool-args-$(CONFIG_NOINSTR_VALIDATION)	+= --noinstr \
 							   $(if $(or $(CONFIG_MITIGATION_UNRET_ENTRY),$(CONFIG_MITIGATION_SRSO)), --unret)
 
author maintainer
#25 lore raw

PATCH 13/22 objtool: Remove redundant opts.noinstr dependency

In reply to Josh Poimboeuf

The --noinstr dependecy on --link is already enforced in the cmdline arg parsing code. Remove the redundant check.

---
 tools/objtool/check.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)
tools/objtool/check.c +1 -6
index 03ec485a376a..f4b9fcc395eb 100644
@@ -340,12 +340,7 @@ static void init_insn_state(struct objtool_file *file, struct insn_state *state,
 	memset(state, 0, sizeof(*state));
 	init_cfi_state(&state->cfi);
 
-	/*
-	 * We need the full vmlinux for noinstr validation, otherwise we can
-	 * not correctly determine insn_call_dest(insn)->sec (external symbols
-	 * do not have a section).
-	 */
-	if (opts.link && opts.noinstr && sec)
+	if (opts.noinstr && sec)
 		state->noinstr = sec->noinstr;
 }
 
Signature · 1 line
2.48.1
merge bot Applied
#26 lore raw

[tip: objtool/urgent] objtool: Remove redundant opts.noinstr dependency

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     a8d39a62c6f5ad76b8a1ebfbf10dd9fe8ca2bbcc
Gitweb:        https://git.kernel.org/tip/a8d39a62c6f5ad76b8a1ebfbf10dd9fe8ca2bbcc
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:03 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:28 +01:00

objtool: Remove redundant opts.noinstr dependency

The --noinstr dependecy on --link is already enforced in the cmdline arg parsing code. Remove the redundant check.

---
 tools/objtool/check.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)
tools/objtool/check.c +1 -6
index ac21f28..0caabf0 100644
@@ -340,12 +340,7 @@ static void init_insn_state(struct objtool_file *file, struct insn_state *state,
 	memset(state, 0, sizeof(*state));
 	init_cfi_state(&state->cfi);
 
-	/*
-	 * We need the full vmlinux for noinstr validation, otherwise we can
-	 * not correctly determine insn_call_dest(insn)->sec (external symbols
-	 * do not have a section).
-	 */
-	if (opts.link && opts.noinstr && sec)
+	if (opts.noinstr && sec)
 		state->noinstr = sec->noinstr;
 }
 
author maintainer
#27 lore raw

PATCH 14/22 spi: amd: Fix out-of-bounds stack access in amd_set_spi_freq()

In reply to Josh Poimboeuf

If speed_hz < AMD_SPI_MIN_HZ, amd_set_spi_freq() iterates over the entire amd_spi_freq array without breaking out early, causing 'i' to go beyond the array bounds.

Fix that by stopping the loop when it gets to the last entry, so the low speed_hz value gets clamped up to AMD_SPI_MIN_HZ.

Fixes the following warning with an UBSAN kernel:

drivers/spi/spi-amd.o: error: objtool: amd_set_spi_freq() falls through to next function amd_spi_set_opcode()

Cc Raju Rangoju <[email protected]>
CC Mark Brown <[email protected]>
Fixes 3fe26121dc3a ("spi: amd: Configure device speed")
---
 drivers/spi/spi-amd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/spi/spi-amd.c +1 -1
index c85997478b81..17fc0b17e756 100644
@@ -302,7 +302,7 @@ static void amd_set_spi_freq(struct amd_spi *amd_spi, u32 speed_hz)
 {
 	unsigned int i, spd7_val, alt_spd;
 
-	for (i = 0; i < ARRAY_SIZE(amd_spi_freq); i++)
+	for (i = 0; i < ARRAY_SIZE(amd_spi_freq)-1; i++)
 		if (speed_hz >= amd_spi_freq[i].speed_hz)
 			break;
 
Signature · 1 line
2.48.1
merge bot Applied
#28 lore raw

[tip: objtool/urgent] objtool, spi: amd: Fix out-of-bounds stack access in amd_set_spi_freq()

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     0239716b82b75dc5cb64c5ec5405dac6c3448d48
Gitweb:        https://git.kernel.org/tip/0239716b82b75dc5cb64c5ec5405dac6c3448d48
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:04 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:28 +01:00

objtool, spi: amd: Fix out-of-bounds stack access in amd_set_spi_freq()

If speed_hz < AMD_SPI_MIN_HZ, amd_set_spi_freq() iterates over the entire amd_spi_freq array without breaking out early, causing 'i' to go beyond the array bounds.

Fix that by stopping the loop when it gets to the last entry, so the low speed_hz value gets clamped up to AMD_SPI_MIN_HZ.

Fixes the following warning with an UBSAN kernel:

drivers/spi/spi-amd.o: error: objtool: amd_set_spi_freq() falls through to next function amd_spi_set_opcode()

---
 drivers/spi/spi-amd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/spi/spi-amd.c +1 -1
index c859974..17fc0b1 100644
@@ -302,7 +302,7 @@ static void amd_set_spi_freq(struct amd_spi *amd_spi, u32 speed_hz)
 {
 	unsigned int i, spd7_val, alt_spd;
 
-	for (i = 0; i < ARRAY_SIZE(amd_spi_freq); i++)
+	for (i = 0; i < ARRAY_SIZE(amd_spi_freq)-1; i++)
 		if (speed_hz >= amd_spi_freq[i].speed_hz)
 			break;
 
maintainer Acked-by
#29 lore raw

Re: [tip: objtool/urgent] objtool, spi: amd: Fix out-of-bounds stack access in amd_set_spi_freq()

Quoted 8 lines · On Tue, Mar 25, 2025 at 08:34:58AM -0000, tip-bot2 for Josh Poimboeuf wrote:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     0239716b82b75dc5cb64c5ec5405dac6c3448d48
Gitweb:        https://git.kernel.org/tip/0239716b82b75dc5cb64c5ec5405dac6c3448d48
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:04 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:28 +01:00
merge bot Applied
#30 lore raw

[tip: objtool/urgent] objtool, spi: amd: Fix out-of-bounds stack access in amd_set_spi_freq()

In reply to Josh Poimboeuf
The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     76e51db43fe4aaaebcc5ddda67b0807f7c9bdecc
Gitweb:        https://git.kernel.org/tip/76e51db43fe4aaaebcc5ddda67b0807f7c9bdecc
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:04 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 23:00:03 +01:00

objtool, spi: amd: Fix out-of-bounds stack access in amd_set_spi_freq()

If speed_hz < AMD_SPI_MIN_HZ, amd_set_spi_freq() iterates over the entire amd_spi_freq array without breaking out early, causing 'i' to go beyond the array bounds.

Fix that by stopping the loop when it gets to the last entry, so the low speed_hz value gets clamped up to AMD_SPI_MIN_HZ.

Fixes the following warning with an UBSAN kernel:

drivers/spi/spi-amd.o: error: objtool: amd_set_spi_freq() falls through to next function amd_spi_set_opcode()

---
 drivers/spi/spi-amd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/spi/spi-amd.c +1 -1
index c859974..17fc0b1 100644
@@ -302,7 +302,7 @@ static void amd_set_spi_freq(struct amd_spi *amd_spi, u32 speed_hz)
 {
 	unsigned int i, spd7_val, alt_spd;
 
-	for (i = 0; i < ARRAY_SIZE(amd_spi_freq); i++)
+	for (i = 0; i < ARRAY_SIZE(amd_spi_freq)-1; i++)
 		if (speed_hz >= amd_spi_freq[i].speed_hz)
 			break;
 
author maintainer
#31 lore raw

PATCH 15/22 nvmet: Fix out-of-bounds stack access in nvmet_ctrl_state_show()

In reply to Josh Poimboeuf

The csts_state_names array only has six sparse entries, but the iteration code in nvmet_ctrl_state_show() iterates seven, resulting in a potential out-of-bounds stack read. Fix that.

Fixes the following warning with an UBSAN kernel:

vmlinux.o: warning: objtool: .text.nvmet_ctrl_state_show: unexpected end of section

Cc Christoph Hellwig <[email protected]>
Cc Sagi Grimberg <[email protected]>
Cc Chaitanya Kulkarni <[email protected]>
Fixes 649fd41420a8 ("nvmet: add debugfs support")
---
 drivers/nvme/target/debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/nvme/target/debugfs.c +1 -1
index 220c7391fc19..c6571fbd35e3 100644
@@ -78,7 +78,7 @@ static int nvmet_ctrl_state_show(struct seq_file *m, void *p)
 	bool sep = false;
 	int i;
 
-	for (i = 0; i < 7; i++) {
+	for (i = 0; i < ARRAY_SIZE(csts_state_names); i++) {
 		int state = BIT(i);
 
 		if (!(ctrl->csts & state))
Signature · 1 line
2.48.1
merge bot Applied
#32 lore raw

[tip: objtool/urgent] objtool, nvmet: Fix out-of-bounds stack access in nvmet_ctrl_state_show()

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     ea0d2a4c77d98e983b42f61790a0760a68983156
Gitweb:        https://git.kernel.org/tip/ea0d2a4c77d98e983b42f61790a0760a68983156
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:05 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:28 +01:00

objtool, nvmet: Fix out-of-bounds stack access in nvmet_ctrl_state_show()

The csts_state_names[] array only has six sparse entries, but the iteration code in nvmet_ctrl_state_show() iterates seven, resulting in a potential out-of-bounds stack read. Fix that.

Fixes the following warning with an UBSAN kernel:

vmlinux.o: warning: objtool: .text.nvmet_ctrl_state_show: unexpected end of section

---
 drivers/nvme/target/debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/nvme/target/debugfs.c +1 -1
index 220c739..c6571fb 100644
@@ -78,7 +78,7 @@ static int nvmet_ctrl_state_show(struct seq_file *m, void *p)
 	bool sep = false;
 	int i;
 
-	for (i = 0; i < 7; i++) {
+	for (i = 0; i < ARRAY_SIZE(csts_state_names); i++) {
 		int state = BIT(i);
 
 		if (!(ctrl->csts & state))
merge bot Applied
#33 lore raw

[tip: objtool/urgent] objtool, nvmet: Fix out-of-bounds stack access in nvmet_ctrl_state_show()

In reply to Josh Poimboeuf
The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     107a23185d990e3df6638d9a84c835f963fe30a6
Gitweb:        https://git.kernel.org/tip/107a23185d990e3df6638d9a84c835f963fe30a6
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:05 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 23:00:15 +01:00

objtool, nvmet: Fix out-of-bounds stack access in nvmet_ctrl_state_show()

The csts_state_names[] array only has six sparse entries, but the iteration code in nvmet_ctrl_state_show() iterates seven, resulting in a potential out-of-bounds stack read. Fix that.

Fixes the following warning with an UBSAN kernel:

vmlinux.o: warning: objtool: .text.nvmet_ctrl_state_show: unexpected end of section

---
 drivers/nvme/target/debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/nvme/target/debugfs.c +1 -1
index 220c739..c6571fb 100644
@@ -78,7 +78,7 @@ static int nvmet_ctrl_state_show(struct seq_file *m, void *p)
 	bool sep = false;
 	int i;
 
-	for (i = 0; i < 7; i++) {
+	for (i = 0; i < ARRAY_SIZE(csts_state_names); i++) {
 		int state = BIT(i);
 
 		if (!(ctrl->csts & state))
Reviewed-by
#34 lore raw

Re: [tip: objtool/urgent] objtool, nvmet: Fix out-of-bounds stack access in nvmet_ctrl_state_show()

(+linux-nvme)

Quoted 38 lines · On 3/25/25 15:09, tip-bot2 for Josh Poimboeuf wrote:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     107a23185d990e3df6638d9a84c835f963fe30a6
Gitweb:        https://git.kernel.org/tip/107a23185d990e3df6638d9a84c835f963fe30a6
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:05 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 23:00:15 +01:00

objtool, nvmet: Fix out-of-bounds stack access in nvmet_ctrl_state_show()

The csts_state_names[] array only has six sparse entries, but the iteration code in nvmet_ctrl_state_show() iterates seven, resulting in a potential out-of-bounds stack read. Fix that.

Fixes the following warning with an UBSAN kernel:

vmlinux.o: warning: objtool: .text.nvmet_ctrl_state_show: unexpected end of section

---
  drivers/nvme/target/debugfs.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/nvme/target/debugfs.c +0 -0
index 220c739..c6571fb 100644

drivers/nvme/target/debugfs.c static int nvmet_ctrl_state_show(struct seq_file *m, void *p) · line 84

Quoted 8 lines
drivers/nvme/target/debugfs.c +1 -1
@@ -78,7 +78,7 @@ static int nvmet_ctrl_state_show(struct seq_file *m, void *p)
  	bool sep = false;
  	int i;
 
-	for (i = 0; i < 7; i++) {
+	for (i = 0; i < ARRAY_SIZE(csts_state_names); i++) {
  		int state = BIT(i);
 
  		if (!(ctrl->csts & state))

Looks good.

-ck

author maintainer
#35 lore raw

PATCH 16/22 media: dib8000: Prevent divide-by-zero in dib8000_set_dds()

In reply to Josh Poimboeuf

If dib8000_set_dds()'s call to dib8000_read32() returns zero, the result is a divide-by-zero. Prevent that from happening.

Fixes the following warning with an UBSAN kernel:

drivers/media/dvb-frontends/dib8000.o: warning: objtool: dib8000_tune() falls through to next function dib8096p_cfg_DibRx()

Cc Mauro Carvalho Chehab <[email protected]>
Fixes 173a64cb3fcf ("[media] dib8000: enhancement")
---
 drivers/media/dvb-frontends/dib8000.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/media/dvb-frontends/dib8000.c +4 -1
index 2f5165918163..cfe59c3255f7 100644
@@ -2701,8 +2701,11 @@ static void dib8000_set_dds(struct dib8000_state *state, s32 offset_khz)
 	u8 ratio;
 
 	if (state->revision == 0x8090) {
+		u32 internal = dib8000_read32(state, 23) / 1000;
+
 		ratio = 4;
-		unit_khz_dds_val = (1<<26) / (dib8000_read32(state, 23) / 1000);
+
+		unit_khz_dds_val = (1<<26) / (internal ?: 1);
 		if (offset_khz < 0)
 			dds = (1 << 26) - (abs_offset_khz * unit_khz_dds_val);
 		else
Signature · 1 line
2.48.1
merge bot Applied
#36 lore raw

[tip: objtool/urgent] objtool, media: dib8000: Prevent divide-by-zero in dib8000_set_dds()

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     c13d957b38b43cfee77f210c2f6c64c2957afed0
Gitweb:        https://git.kernel.org/tip/c13d957b38b43cfee77f210c2f6c64c2957afed0
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:06 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:28 +01:00

objtool, media: dib8000: Prevent divide-by-zero in dib8000_set_dds()

If dib8000_set_dds()'s call to dib8000_read32() returns zero, the result is a divide-by-zero. Prevent that from happening.

Fixes the following warning with an UBSAN kernel:

drivers/media/dvb-frontends/dib8000.o: warning: objtool: dib8000_tune() falls through to next function dib8096p_cfg_DibRx()

---
 drivers/media/dvb-frontends/dib8000.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/media/dvb-frontends/dib8000.c +4 -1
index 2f51659..cfe59c3 100644
@@ -2701,8 +2701,11 @@ static void dib8000_set_dds(struct dib8000_state *state, s32 offset_khz)
 	u8 ratio;
 
 	if (state->revision == 0x8090) {
+		u32 internal = dib8000_read32(state, 23) / 1000;
+
 		ratio = 4;
-		unit_khz_dds_val = (1<<26) / (dib8000_read32(state, 23) / 1000);
+
+		unit_khz_dds_val = (1<<26) / (internal ?: 1);
 		if (offset_khz < 0)
 			dds = (1 << 26) - (abs_offset_khz * unit_khz_dds_val);
 		else
merge bot Applied
#37 lore raw

[tip: objtool/urgent] objtool, media: dib8000: Prevent divide-by-zero in dib8000_set_dds()

In reply to Josh Poimboeuf
The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     e63d465f59011dede0a0f1d21718b59a64c3ff5c
Gitweb:        https://git.kernel.org/tip/e63d465f59011dede0a0f1d21718b59a64c3ff5c
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:06 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 23:00:15 +01:00

objtool, media: dib8000: Prevent divide-by-zero in dib8000_set_dds()

If dib8000_set_dds()'s call to dib8000_read32() returns zero, the result is a divide-by-zero. Prevent that from happening.

Fixes the following warning with an UBSAN kernel:

drivers/media/dvb-frontends/dib8000.o: warning: objtool: dib8000_tune() falls through to next function dib8096p_cfg_DibRx()

---
 drivers/media/dvb-frontends/dib8000.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/media/dvb-frontends/dib8000.c +4 -1
index 2f51659..cfe59c3 100644
@@ -2701,8 +2701,11 @@ static void dib8000_set_dds(struct dib8000_state *state, s32 offset_khz)
 	u8 ratio;
 
 	if (state->revision == 0x8090) {
+		u32 internal = dib8000_read32(state, 23) / 1000;
+
 		ratio = 4;
-		unit_khz_dds_val = (1<<26) / (dib8000_read32(state, 23) / 1000);
+
+		unit_khz_dds_val = (1<<26) / (internal ?: 1);
 		if (offset_khz < 0)
 			dds = (1 << 26) - (abs_offset_khz * unit_khz_dds_val);
 		else
#38 lore raw

Re: [tip: objtool/urgent] objtool, media: dib8000: Prevent divide-by-zero in dib8000_set_dds()

Em Tue, 25 Mar 2025 22:09:58 -0000
"tip-bot2 for Josh Poimboeuf" <[email protected]> escreveu:

Quoted 35 lines

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     e63d465f59011dede0a0f1d21718b59a64c3ff5c
Gitweb:        https://git.kernel.org/tip/e63d465f59011dede0a0f1d21718b59a64c3ff5c
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:06 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 23:00:15 +01:00

objtool, media: dib8000: Prevent divide-by-zero in dib8000_set_dds()

If dib8000_set_dds()'s call to dib8000_read32() returns zero, the result is a divide-by-zero. Prevent that from happening.

Fixes the following warning with an UBSAN kernel:

drivers/media/dvb-frontends/dib8000.o: warning: objtool: dib8000_tune() falls through to next function dib8096p_cfg_DibRx()

---
 drivers/media/dvb-frontends/dib8000.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/media/dvb-frontends/dib8000.c +0 -0
index 2f51659..cfe59c3 100644

drivers/media/dvb-frontends/dib8000.c static void dib8000_set_dds(struct dib8000_state *state, s32 offset_khz) · line 2708

Quoted 9 lines
drivers/media/dvb-frontends/dib8000.c +4 -1
@@ -2701,8 +2701,11 @@ static void dib8000_set_dds(struct dib8000_state *state, s32 offset_khz)
 	u8 ratio;
 
 	if (state->revision == 0x8090) {
+		u32 internal = dib8000_read32(state, 23) / 1000;
+
 		ratio = 4;
-		unit_khz_dds_val = (1<<26) / (dib8000_read32(state, 23) / 1000);
+
+		unit_khz_dds_val = (1<<26) / (internal ?: 1);

This is theoretical, as in practice dib8096 won't likely be tuning if reading this register would return zero, but at least for my eyes, it would sound better to set unit_khz_dds_val to 1 internal is zero, instead of 1<<26.

drivers/media/dvb-frontends/dib8000.c static void dib8000_set_dds(struct dib8000_state *state, s32 offset_khz) · line 2711

Quoted 3 lines
drivers/media/dvb-frontends/dib8000.c +0 -0
 		if (offset_khz < 0)
 			dds = (1 << 26) - (abs_offset_khz * unit_khz_dds_val);
 		else

Regards,
Mauro

author maintainer
#39 lore raw

Re: [tip: objtool/urgent] objtool, media: dib8000: Prevent divide-by-zero in dib8000_set_dds()

Quoted 11 lines · Mauro Carvalho Chehab

Review

Quoted 6 lines
(quoted diff) +4 -1
+		u32 internal = dib8000_read32(state, 23) / 1000;
+
 		ratio = 4;
-		unit_khz_dds_val = (1<<26) / (dib8000_read32(state, 23) / 1000);
+
+		unit_khz_dds_val = (1<<26) / (internal ?: 1);

This is theoretical, as in practice dib8096 won't likely be tuning if reading this register would return zero, but at least for my eyes, it would sound better to set unit_khz_dds_val to 1 internal is zero, instead of 1<<26.

I don't pretend to understand this device, I just figured one is the closest you can get to zero :-)

So something like this instead?

drivers/media/dvb-frontends/dib8000.c +1 -1
index cfe59c3255f7..c80134ff511b 100644
@@ -2705,7 +2705,7 @@ static void dib8000_set_dds(struct dib8000_state *state, s32 offset_khz)
 
 		ratio = 4;
 
-		unit_khz_dds_val = (1<<26) / (internal ?: 1);
+		unit_khz_dds_val = internal ? ((1<<26) / internal) : 1;
 		if (offset_khz < 0)
 			dds = (1 << 26) - (abs_offset_khz * unit_khz_dds_val);
 		else
author maintainer
#40 lore raw

PATCH 17/22 panic: Disable SMAP in __stack_chk_fail()

In reply to Josh Poimboeuf

__stack_chk_fail() can be called from uaccess-enabled code. Make sure uaccess gets disabled before calling panic().

Fixes the following warning:

kernel/trace/trace_branch.o: error: objtool: ftrace_likely_update+0x1ea: call to __stack_chk_fail() with UACCESS enabled

---
 kernel/panic.c        | 6 ++++++
 tools/objtool/check.c | 5 ++++-
 2 files changed, 10 insertions(+), 1 deletion(-)
kernel/panic.c +6 -0
index d8635d5cecb2..f9f0c5148f6a 100644
@@ -832,9 +832,15 @@ device_initcall(register_warn_debugfs);
  */
 __visible noinstr void __stack_chk_fail(void)
 {
+	unsigned long flags;
+
 	instrumentation_begin();
+	flags = user_access_save();
+
 	panic("stack-protector: Kernel stack is corrupted in: %pB",
 		__builtin_return_address(0));
+
+	user_access_restore(flags);
 	instrumentation_end();
 }
 EXPORT_SYMBOL(__stack_chk_fail);
tools/objtool/check.c +4 -1
index f4b9fcc395eb..a953ce5fe395 100644
@@ -1194,12 +1194,15 @@ static const char *uaccess_safe_builtin[] = {
 	"__ubsan_handle_load_invalid_value",
 	/* STACKLEAK */
 	"stackleak_track_stack",
+	/* TRACE_BRANCH_PROFILING */
+	"ftrace_likely_update",
+	/* STACKPROTECTOR */
+	"__stack_chk_fail",
 	/* misc */
 	"csum_partial_copy_generic",
 	"copy_mc_fragile",
 	"copy_mc_fragile_handle_tail",
 	"copy_mc_enhanced_fast_string",
-	"ftrace_likely_update", /* CONFIG_TRACE_BRANCH_PROFILING */
 	"rep_stos_alternative",
 	"rep_movs_alternative",
 	"__copy_user_nocache",
Signature · 1 line
2.48.1
merge bot Applied
#41 lore raw

[tip: objtool/urgent] objtool, panic: Disable SMAP in __stack_chk_fail()

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     c2299757eb5bbc121177a23559eff8f66e944381
Gitweb:        https://git.kernel.org/tip/c2299757eb5bbc121177a23559eff8f66e944381
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:07 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:28 +01:00

objtool, panic: Disable SMAP in __stack_chk_fail()

__stack_chk_fail() can be called from uaccess-enabled code. Make sure uaccess gets disabled before calling panic().

Fixes the following warning:

kernel/trace/trace_branch.o: error: objtool: ftrace_likely_update+0x1ea: call to __stack_chk_fail() with UACCESS enabled

---
 kernel/panic.c        | 6 ++++++
 tools/objtool/check.c | 5 ++++-
 2 files changed, 10 insertions(+), 1 deletion(-)
kernel/panic.c +6 -0
index d8635d5..f9f0c51 100644
@@ -832,9 +832,15 @@ device_initcall(register_warn_debugfs);
  */
 __visible noinstr void __stack_chk_fail(void)
 {
+	unsigned long flags;
+
 	instrumentation_begin();
+	flags = user_access_save();
+
 	panic("stack-protector: Kernel stack is corrupted in: %pB",
 		__builtin_return_address(0));
+
+	user_access_restore(flags);
 	instrumentation_end();
 }
 EXPORT_SYMBOL(__stack_chk_fail);
tools/objtool/check.c +4 -1
index 0caabf0..3bf2992 100644
@@ -1194,12 +1194,15 @@ static const char *uaccess_safe_builtin[] = {
 	"__ubsan_handle_load_invalid_value",
 	/* STACKLEAK */
 	"stackleak_track_stack",
+	/* TRACE_BRANCH_PROFILING */
+	"ftrace_likely_update",
+	/* STACKPROTECTOR */
+	"__stack_chk_fail",
 	/* misc */
 	"csum_partial_copy_generic",
 	"copy_mc_fragile",
 	"copy_mc_fragile_handle_tail",
 	"copy_mc_enhanced_fast_string",
-	"ftrace_likely_update", /* CONFIG_TRACE_BRANCH_PROFILING */
 	"rep_stos_alternative",
 	"rep_movs_alternative",
 	"__copy_user_nocache",
merge bot Applied
#42 lore raw

[tip: objtool/urgent] objtool, panic: Disable SMAP in __stack_chk_fail()

In reply to Josh Poimboeuf
The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     72c774aa9d1e16bfd247096935e7dae194d84929
Gitweb:        https://git.kernel.org/tip/72c774aa9d1e16bfd247096935e7dae194d84929
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:07 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 23:00:15 +01:00

objtool, panic: Disable SMAP in __stack_chk_fail()

__stack_chk_fail() can be called from uaccess-enabled code. Make sure uaccess gets disabled before calling panic().

Fixes the following warning:

kernel/trace/trace_branch.o: error: objtool: ftrace_likely_update+0x1ea: call to __stack_chk_fail() with UACCESS enabled

---
 kernel/panic.c        | 6 ++++++
 tools/objtool/check.c | 5 ++++-
 2 files changed, 10 insertions(+), 1 deletion(-)
kernel/panic.c +6 -0
index d8635d5..f9f0c51 100644
@@ -832,9 +832,15 @@ device_initcall(register_warn_debugfs);
  */
 __visible noinstr void __stack_chk_fail(void)
 {
+	unsigned long flags;
+
 	instrumentation_begin();
+	flags = user_access_save();
+
 	panic("stack-protector: Kernel stack is corrupted in: %pB",
 		__builtin_return_address(0));
+
+	user_access_restore(flags);
 	instrumentation_end();
 }
 EXPORT_SYMBOL(__stack_chk_fail);
tools/objtool/check.c +4 -1
index 0caabf0..3bf2992 100644
@@ -1194,12 +1194,15 @@ static const char *uaccess_safe_builtin[] = {
 	"__ubsan_handle_load_invalid_value",
 	/* STACKLEAK */
 	"stackleak_track_stack",
+	/* TRACE_BRANCH_PROFILING */
+	"ftrace_likely_update",
+	/* STACKPROTECTOR */
+	"__stack_chk_fail",
 	/* misc */
 	"csum_partial_copy_generic",
 	"copy_mc_fragile",
 	"copy_mc_fragile_handle_tail",
 	"copy_mc_enhanced_fast_string",
-	"ftrace_likely_update", /* CONFIG_TRACE_BRANCH_PROFILING */
 	"rep_stos_alternative",
 	"rep_movs_alternative",
 	"__copy_user_nocache",
author maintainer
#43 lore raw

PATCH 18/22 Input: cyapa - remove undefined behavior in cyapa_update_fw_store()

In reply to Josh Poimboeuf

In cyapa_update_fw_store(), if count is zero, the write to fw_name[count-1] underflows the array.

Presumably that's not possible in normal operation, as its caller sysfs_kf_write() already checks for zero. Regardless it's a good idea to check for the error explicitly and avoid undefined behavior.

Fixes the following warning with an UBSAN kernel:

vmlinux.o: warning: objtool: .text.cyapa_update_fw_store: unexpected end of section

---
 drivers/input/mouse/cyapa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/input/mouse/cyapa.c +2 -2
index 2f2d925a55d7..00c87c0532a6 100644
@@ -1080,8 +1080,8 @@ static ssize_t cyapa_update_fw_store(struct device *dev,
 	char fw_name[NAME_MAX];
 	int ret, error;
 
-	if (count >= NAME_MAX) {
-		dev_err(dev, "File name too long\n");
+	if (!count || count >= NAME_MAX) {
+		dev_err(dev, "Bad file name size\n");
 		return -EINVAL;
 	}
 
Signature · 1 line
2.48.1
merge bot Applied
#44 lore raw

[tip: objtool/urgent] objtool, Input: cyapa - Remove undefined behavior in cyapa_update_fw_store()

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     e3c9ccbbf5c5e1238d4ac8577559e61576b9210f
Gitweb:        https://git.kernel.org/tip/e3c9ccbbf5c5e1238d4ac8577559e61576b9210f
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:08 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:29 +01:00

objtool, Input: cyapa - Remove undefined behavior in cyapa_update_fw_store()

In cyapa_update_fw_store(), if 'count' is zero, the write to fw_name[count-1] underflows the array.

Presumably that's not possible in normal operation, as its caller sysfs_kf_write() already checks for zero. Regardless it's a good idea to check for the error explicitly and avoid undefined behavior.

Fixes the following warning with an UBSAN kernel:

vmlinux.o: warning: objtool: .text.cyapa_update_fw_store: unexpected end of section

---
 drivers/input/mouse/cyapa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/input/mouse/cyapa.c +2 -2
index 2f2d925..00c87c0 100644
@@ -1080,8 +1080,8 @@ static ssize_t cyapa_update_fw_store(struct device *dev,
 	char fw_name[NAME_MAX];
 	int ret, error;
 
-	if (count >= NAME_MAX) {
-		dev_err(dev, "File name too long\n");
+	if (!count || count >= NAME_MAX) {
+		dev_err(dev, "Bad file name size\n");
 		return -EINVAL;
 	}
 
merge bot Applied
#45 lore raw

[tip: objtool/urgent] objtool, Input: cyapa - Remove undefined behavior in cyapa_update_fw_store()

In reply to Josh Poimboeuf
The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     7501153750b47416c9891038330359fad0205839
Gitweb:        https://git.kernel.org/tip/7501153750b47416c9891038330359fad0205839
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:08 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 23:00:15 +01:00

objtool, Input: cyapa - Remove undefined behavior in cyapa_update_fw_store()

In cyapa_update_fw_store(), if 'count' is zero, the write to fw_name[count-1] underflows the array.

Presumably that's not possible in normal operation, as its caller sysfs_kf_write() already checks for zero. Regardless it's a good idea to check for the error explicitly and avoid undefined behavior.

Fixes the following warning with an UBSAN kernel:

vmlinux.o: warning: objtool: .text.cyapa_update_fw_store: unexpected end of section

---
 drivers/input/mouse/cyapa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/input/mouse/cyapa.c +2 -2
index 2f2d925..00c87c0 100644
@@ -1080,8 +1080,8 @@ static ssize_t cyapa_update_fw_store(struct device *dev,
 	char fw_name[NAME_MAX];
 	int ret, error;
 
-	if (count >= NAME_MAX) {
-		dev_err(dev, "File name too long\n");
+	if (!count || count >= NAME_MAX) {
+		dev_err(dev, "Bad file name size\n");
 		return -EINVAL;
 	}
 
author maintainer
#46 lore raw

PATCH 19/22 ASoC: codecs: wcd934x: Remove undefined behavior in wcd934x_slim_irq_handler()

In reply to Josh Poimboeuf

If port_id is negative, the shift counts in wcd934x_slim_irq_handler() also become negative, resulting in undefined behavior due to shift out of bounds.

If I'm reading the code correctly, that appears to be not possible, but with KCOV enabled, Clang's range analysis isn't always able to determine that and generates undefined behavior.

As a result the code generation isn't optimal, and undefined behavior should be avoided regardless. Improve code generation and remove the undefined behavior by converting the signed variables to unsigned.

Fixes the following warning with UBSAN:

sound/soc/codecs/snd-soc-wcd934x.o: warning: objtool: .text.wcd934x_slim_irq_handler: unexpected end of section

---
 sound/soc/codecs/wcd934x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/codecs/wcd934x.c +1 -1
index 910852eb9698..c7f1b28f3b23 100644
@@ -2273,7 +2273,7 @@ static irqreturn_t wcd934x_slim_irq_handler(int irq, void *data)
 {
 	struct wcd934x_codec *wcd = data;
 	unsigned long status = 0;
-	int i, j, port_id;
+	unsigned int i, j, port_id;
 	unsigned int val, int_val = 0;
 	irqreturn_t ret = IRQ_NONE;
 	bool tx;
Signature · 1 line
2.48.1
merge bot Applied
#47 lore raw

[tip: objtool/urgent] objtool, ASoC: codecs: wcd934x: Remove potential undefined behavior in wcd934x_slim_irq_handler()

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     46b70c569b774ea2c671bb3aff2a50d29760b7c3
Gitweb:        https://git.kernel.org/tip/46b70c569b774ea2c671bb3aff2a50d29760b7c3
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:09 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:29 +01:00

objtool, ASoC: codecs: wcd934x: Remove potential undefined behavior in wcd934x_slim_irq_handler()

If 'port_id' is negative, the shift counts in wcd934x_slim_irq_handler() also become negative, resulting in undefined behavior due to shift out of bounds.

If I'm reading the code correctly, that appears to be not possible, but with KCOV enabled, Clang's range analysis isn't always able to determine that and generates undefined behavior.

As a result the code generation isn't optimal, and undefined behavior should be avoided regardless. Improve code generation and remove the undefined behavior by converting the signed variables to unsigned.

Fixes the following warning with UBSAN:

sound/soc/codecs/snd-soc-wcd934x.o: warning: objtool: .text.wcd934x_slim_irq_handler: unexpected end of section

---
 sound/soc/codecs/wcd934x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/codecs/wcd934x.c +1 -1
index 910852e..c7f1b28 100644
@@ -2273,7 +2273,7 @@ static irqreturn_t wcd934x_slim_irq_handler(int irq, void *data)
 {
 	struct wcd934x_codec *wcd = data;
 	unsigned long status = 0;
-	int i, j, port_id;
+	unsigned int i, j, port_id;
 	unsigned int val, int_val = 0;
 	irqreturn_t ret = IRQ_NONE;
 	bool tx;
maintainer
#48 lore raw

Re: [tip: objtool/urgent] objtool, ASoC: codecs: wcd934x: Remove potential undefined behavior in wcd934x_slim_irq_handler()

Quoted 10 lines · On Tue, Mar 25, 2025 at 08:34:53AM -0000, tip-bot2 for Josh Poimboeuf wrote:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     46b70c569b774ea2c671bb3aff2a50d29760b7c3
Gitweb:        https://git.kernel.org/tip/46b70c569b774ea2c671bb3aff2a50d29760b7c3
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:09 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:29 +01:00

objtool, ASoC: codecs: wcd934x: Remove potential undefined behavior in wcd934x_slim_irq_handler()

FWIW the original patch doesn't seem to have made it into my inbox (the regmap one did), not that it makes a *huge* difference but might be some infra/script issue which crops up on some other more urgent occasion.

#49 lore raw

Re: [tip: objtool/urgent] objtool, ASoC: codecs: wcd934x: Remove potential undefined behavior in wcd934x_slim_irq_handler()

Quoted 10 lines · Mark Brown
Quoted 9 lines · On Tue, Mar 25, 2025 at 08:34:53AM -0000, tip-bot2 for Josh Poimboeuf wrote:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     46b70c569b774ea2c671bb3aff2a50d29760b7c3
Gitweb:        https://git.kernel.org/tip/46b70c569b774ea2c671bb3aff2a50d29760b7c3
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:09 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:29 +01:00
Quoted 6 lines
Quoted 1 line

objtool, ASoC: codecs: wcd934x: Remove potential undefined behavior in wcd934x_slim_irq_handler()

FWIW the original patch doesn't seem to have made it into my inbox (the regmap one did), not that it makes a *huge* difference but might be some infra/script issue which crops up on some other more urgent occasion.

Yeah, I noticed that the Cc: lines were incomplete for some of the patches, so before applying them I went over the series and regenerated the Cc: lines so everyone is informed and can review/object/ack if necessary.

If there's any serious mistake we'll rebase the branch!

Thanks,

Ingo

maintainer Acked-by
#50 lore raw

Re: [tip: objtool/urgent] objtool, ASoC: codecs: wcd934x: Remove potential undefined behavior in wcd934x_slim_irq_handler()

Quoted 11 lines · Ingo Molnar
Quoted 4 lines

FWIW the original patch doesn't seem to have made it into my inbox (the regmap one did), not that it makes a *huge* difference but might be some infra/script issue which crops up on some other more urgent occasion.

Yeah, I noticed that the Cc: lines were incomplete for some of the patches, so before applying them I went over the series and regenerated the Cc: lines so everyone is informed and can review/object/ack if necessary.

If there's any serious mistake we'll rebase the branch!

No problem here, just wanted to flag the potential infra thing. I didn't bother acking since the patches were applied:

merge bot Applied
#51 lore raw

[tip: objtool/urgent] objtool, ASoC: codecs: wcd934x: Remove potential undefined behavior in wcd934x_slim_irq_handler()

In reply to Josh Poimboeuf
The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     060aed9c0093b341480770457093449771cf1496
Gitweb:        https://git.kernel.org/tip/060aed9c0093b341480770457093449771cf1496
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:09 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 23:00:16 +01:00

objtool, ASoC: codecs: wcd934x: Remove potential undefined behavior in wcd934x_slim_irq_handler()

If 'port_id' is negative, the shift counts in wcd934x_slim_irq_handler() also become negative, resulting in undefined behavior due to shift out of bounds.

If I'm reading the code correctly, that appears to be not possible, but with KCOV enabled, Clang's range analysis isn't always able to determine that and generates undefined behavior.

As a result the code generation isn't optimal, and undefined behavior should be avoided regardless. Improve code generation and remove the undefined behavior by converting the signed variables to unsigned.

Fixes the following warning with UBSAN:

sound/soc/codecs/snd-soc-wcd934x.o: warning: objtool: .text.wcd934x_slim_irq_handler: unexpected end of section

---
 sound/soc/codecs/wcd934x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/codecs/wcd934x.c +1 -1
index 910852e..c7f1b28 100644
@@ -2273,7 +2273,7 @@ static irqreturn_t wcd934x_slim_irq_handler(int irq, void *data)
 {
 	struct wcd934x_codec *wcd = data;
 	unsigned long status = 0;
-	int i, j, port_id;
+	unsigned int i, j, port_id;
 	unsigned int val, int_val = 0;
 	irqreturn_t ret = IRQ_NONE;
 	bool tx;
author maintainer
#52 lore raw

PATCH 20/22 regulator: rk808: Remove undefined behavior in rk806_set_mode_dcdc()

In reply to Josh Poimboeuf

If ctr_bit is negative, the shift counts become negative, causing a shift of bounds and undefined behavior.

Presumably that's not possible in normal operation, but the code generation isn't optimal. And undefined behavior should be avoided regardless.

Improve code generation and remove the undefined behavior by converting the signed variables to unsigned.

Fixes the following warning with an UBSAN kernel:

vmlinux.o: warning: objtool: rk806_set_mode_dcdc() falls through to next function rk806_get_mode_dcdc() vmlinux.o: warning: objtool: .text.rk806_set_mode_dcdc: unexpected end of section

---
 drivers/regulator/rk808-regulator.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/regulator/rk808-regulator.c +2 -2
index 7d82bd1b36df..1e8142479656 100644
@@ -270,8 +270,8 @@ static const unsigned int rk817_buck1_4_ramp_table[] = {
 
 static int rk806_set_mode_dcdc(struct regulator_dev *rdev, unsigned int mode)
 {
-	int rid = rdev_get_id(rdev);
-	int ctr_bit, reg;
+	unsigned int rid = rdev_get_id(rdev);
+	unsigned int ctr_bit, reg;
 
 	reg = RK806_POWER_FPWM_EN0 + rid / 8;
 	ctr_bit = rid % 8;
Signature · 1 line
2.48.1
merge bot Applied
#53 lore raw

[tip: objtool/urgent] objtool, regulator: rk808: Remove potential undefined behavior in rk806_set_mode_dcdc()

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     4d014a6b8e4dba75eb0b99e6ed990bb7e38da294
Gitweb:        https://git.kernel.org/tip/4d014a6b8e4dba75eb0b99e6ed990bb7e38da294
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:10 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:29 +01:00

objtool, regulator: rk808: Remove potential undefined behavior in rk806_set_mode_dcdc()

If 'ctr_bit' is negative, the shift counts become negative, causing a shift of bounds and undefined behavior.

Presumably that's not possible in normal operation, but the code generation isn't optimal. And undefined behavior should be avoided regardless.

Improve code generation and remove the undefined behavior by converting the signed variables to unsigned.

Fixes the following warning with an UBSAN kernel:

vmlinux.o: warning: objtool: rk806_set_mode_dcdc() falls through to next function rk806_get_mode_dcdc() vmlinux.o: warning: objtool: .text.rk806_set_mode_dcdc: unexpected end of section

---
 drivers/regulator/rk808-regulator.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/regulator/rk808-regulator.c +2 -2
index 7d82bd1..1e81424 100644
@@ -270,8 +270,8 @@ static const unsigned int rk817_buck1_4_ramp_table[] = {
 
 static int rk806_set_mode_dcdc(struct regulator_dev *rdev, unsigned int mode)
 {
-	int rid = rdev_get_id(rdev);
-	int ctr_bit, reg;
+	unsigned int rid = rdev_get_id(rdev);
+	unsigned int ctr_bit, reg;
 
 	reg = RK806_POWER_FPWM_EN0 + rid / 8;
 	ctr_bit = rid % 8;
maintainer Acked-by
#54 lore raw

Re: [tip: objtool/urgent] objtool, regulator: rk808: Remove potential undefined behavior in rk806_set_mode_dcdc()

Quoted 8 lines · On Tue, Mar 25, 2025 at 08:34:52AM -0000, tip-bot2 for Josh Poimboeuf wrote:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     4d014a6b8e4dba75eb0b99e6ed990bb7e38da294
Gitweb:        https://git.kernel.org/tip/4d014a6b8e4dba75eb0b99e6ed990bb7e38da294
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:10 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:29 +01:00
merge bot Applied
#55 lore raw

[tip: objtool/urgent] objtool, regulator: rk808: Remove potential undefined behavior in rk806_set_mode_dcdc()

In reply to Josh Poimboeuf
The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     29c578c848402a34e8c8e115bf66cb6008b77062
Gitweb:        https://git.kernel.org/tip/29c578c848402a34e8c8e115bf66cb6008b77062
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:10 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 23:00:29 +01:00

objtool, regulator: rk808: Remove potential undefined behavior in rk806_set_mode_dcdc()

If 'ctr_bit' is negative, the shift counts become negative, causing a shift of bounds and undefined behavior.

Presumably that's not possible in normal operation, but the code generation isn't optimal. And undefined behavior should be avoided regardless.

Improve code generation and remove the undefined behavior by converting the signed variables to unsigned.

Fixes the following warning with an UBSAN kernel:

vmlinux.o: warning: objtool: rk806_set_mode_dcdc() falls through to next function rk806_get_mode_dcdc() vmlinux.o: warning: objtool: .text.rk806_set_mode_dcdc: unexpected end of section

---
 drivers/regulator/rk808-regulator.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/regulator/rk808-regulator.c +2 -2
index 7d82bd1..1e81424 100644
@@ -270,8 +270,8 @@ static const unsigned int rk817_buck1_4_ramp_table[] = {
 
 static int rk806_set_mode_dcdc(struct regulator_dev *rdev, unsigned int mode)
 {
-	int rid = rdev_get_id(rdev);
-	int ctr_bit, reg;
+	unsigned int rid = rdev_get_id(rdev);
+	unsigned int ctr_bit, reg;
 
 	reg = RK806_POWER_FPWM_EN0 + rid / 8;
 	ctr_bit = rid % 8;
author maintainer
#56 lore raw

PATCH 21/22 pwm: mediatek: Prevent divide-by-zero in pwm_mediatek_config()

In reply to Josh Poimboeuf

With CONFIG_COMPILE_TEST && !CONFIG_CLK, pwm_mediatek_config() has a divide-by-zero in the following line:

	do_div(resolution, clk_get_rate(pc->clk_pwms[pwm->hwpwm]));

due to the fact that the !CONFIG_CLK version of clk_get_rate() returns zero.

This is presumably just a theoretical problem: COMPILE_TEST overrides the dependency on RALINK which would select COMMON_CLK. Regardless it's a good idea to check for the error explicitly to avoid divide-by-zero.

Fixes the following warning:

drivers/pwm/pwm-mediatek.o: warning: objtool: .text: unexpected end of section

Cc Uwe Kleine-König <[email protected]>
---
 drivers/pwm/pwm-mediatek.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/pwm/pwm-mediatek.c +6 -2
index 01dfa0fab80a..7eaab5831499 100644
@@ -121,21 +121,25 @@ static int pwm_mediatek_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct pwm_mediatek_chip *pc = to_pwm_mediatek_chip(chip);
 	u32 clkdiv = 0, cnt_period, cnt_duty, reg_width = PWMDWIDTH,
 	    reg_thres = PWMTHRES;
+	unsigned long clk_rate;
 	u64 resolution;
 	int ret;
 
 	ret = pwm_mediatek_clk_enable(chip, pwm);
-
 	if (ret < 0)
 		return ret;
 
+	clk_rate = clk_get_rate(pc->clk_pwms[pwm->hwpwm]);
+	if (!clk_rate)
+		return -EINVAL;
+
 	/* Make sure we use the bus clock and not the 26MHz clock */
 	if (pc->soc->has_ck_26m_sel)
 		writel(0, pc->regs + PWM_CK_26M_SEL);
 
 	/* Using resolution in picosecond gets accuracy higher */
 	resolution = (u64)NSEC_PER_SEC * 1000;
-	do_div(resolution, clk_get_rate(pc->clk_pwms[pwm->hwpwm]));
+	do_div(resolution, clk_rate);
 
 	cnt_period = DIV_ROUND_CLOSEST_ULL((u64)period_ns * 1000, resolution);
 	while (cnt_period > 8191) {
Signature · 1 line
2.48.1
merge bot Applied
#57 lore raw

[tip: objtool/urgent] objtool, pwm: mediatek: Prevent theoretical divide-by-zero in pwm_mediatek_config()

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     2becbc66a151500636046503f541255af6acf4fa
Gitweb:        https://git.kernel.org/tip/2becbc66a151500636046503f541255af6acf4fa
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:11 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:32 +01:00

objtool, pwm: mediatek: Prevent theoretical divide-by-zero in pwm_mediatek_config()

With CONFIG_COMPILE_TEST && !CONFIG_CLK, pwm_mediatek_config() has a divide-by-zero in the following line:

	do_div(resolution, clk_get_rate(pc->clk_pwms[pwm->hwpwm]));

due to the fact that the !CONFIG_CLK version of clk_get_rate() returns zero.

This is presumably just a theoretical problem: COMPILE_TEST overrides the dependency on RALINK which would select COMMON_CLK. Regardless it's a good idea to check for the error explicitly to avoid divide-by-zero.

Fixes the following warning:

drivers/pwm/pwm-mediatek.o: warning: objtool: .text: unexpected end of section

---
 drivers/pwm/pwm-mediatek.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/pwm/pwm-mediatek.c +6 -2
index 01dfa0f..7eaab58 100644
@@ -121,21 +121,25 @@ static int pwm_mediatek_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct pwm_mediatek_chip *pc = to_pwm_mediatek_chip(chip);
 	u32 clkdiv = 0, cnt_period, cnt_duty, reg_width = PWMDWIDTH,
 	    reg_thres = PWMTHRES;
+	unsigned long clk_rate;
 	u64 resolution;
 	int ret;
 
 	ret = pwm_mediatek_clk_enable(chip, pwm);
-
 	if (ret < 0)
 		return ret;
 
+	clk_rate = clk_get_rate(pc->clk_pwms[pwm->hwpwm]);
+	if (!clk_rate)
+		return -EINVAL;
+
 	/* Make sure we use the bus clock and not the 26MHz clock */
 	if (pc->soc->has_ck_26m_sel)
 		writel(0, pc->regs + PWM_CK_26M_SEL);
 
 	/* Using resolution in picosecond gets accuracy higher */
 	resolution = (u64)NSEC_PER_SEC * 1000;
-	do_div(resolution, clk_get_rate(pc->clk_pwms[pwm->hwpwm]));
+	do_div(resolution, clk_rate);
 
 	cnt_period = DIV_ROUND_CLOSEST_ULL((u64)period_ns * 1000, resolution);
 	while (cnt_period > 8191) {
#58 lore raw

Re: [tip: objtool/urgent] objtool, pwm: mediatek: Prevent theoretical divide-by-zero in pwm_mediatek_config()

[I forgot a comma in the original post and so the list of Cc: was broken. Here comes a resend, sorry to everyone who get the mail twice now. If you reply, please do so on this copy to not continue using the broken address.]

Hello,

Quoted 8 lines · On Tue, Mar 25, 2025 at 08:34:51AM +0000, tip-bot2 for Josh Poimboeuf wrote:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     2becbc66a151500636046503f541255af6acf4fa
Gitweb:        https://git.kernel.org/tip/2becbc66a151500636046503f541255af6acf4fa
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:11 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:32 +01:00

I wonder a bit about procedures here. While I like that warnings that pop up in drivers/pwm (and elsewhere) are cared for, I think that the sensible way to change warning related settings is to make it hard to enable them first (harder than "depends on !COMPILE_TEST" "To avoid breaking bots too badly") and then work on the identified problems before warning broadly. The way chosen here instead seems to be enabling the warning immediately and then post fixes to the warnings and merge them without respective maintainer feedback in less than 12 hours.

Quoted 5 lines

objtool, pwm: mediatek: Prevent theoretical divide-by-zero in pwm_mediatek_config()

With CONFIG_COMPILE_TEST && !CONFIG_CLK, pwm_mediatek_config() has a divide-by-zero in the following line:

Quoted 7 lines
      do_div(resolution, clk_get_rate(pc->clk_pwms[pwm->hwpwm]));

due to the fact that the !CONFIG_CLK version of clk_get_rate() returns zero.

This is presumably just a theoretical problem: COMPILE_TEST overrides the dependency on RALINK which would select COMMON_CLK. Regardless it's

RALINK and ARCH_MEDIATEK. The latter enables COMMON_CLK as it depends on ARM or ARM64 which both select COMMON_CLK.

Quoted 8 lines

a good idea to check for the error explicitly to avoid divide-by-zero.

Fixes the following warning:

drivers/pwm/pwm-mediatek.o: warning: objtool: .text: unexpected end of section

Fixes caf065f8fd58 ("pwm: Add MediaTek PWM support")

would be nice.

Quoted 3 lines

I fail to reproduce the warning here for an x86_64 build on 1e26c5e28ca5. I have:

        $ grep -E 'CONFIG_(CLK|PWM_MEDIATEK|OBJTOOL_WERROR)\>' .config
        CONFIG_PWM_MEDIATEK=m
        CONFIG_OBJTOOL_WERROR=y

and the build works fine for me and there is no warning about drivers/pwm/pwm-mediatek.o. What am I missing?

Best regards
Uwe

maintainer
#59 lore raw

Re: [tip: objtool/urgent] objtool, pwm: mediatek: Prevent theoretical divide-by-zero in pwm_mediatek_config()

Quoted 2 lines · Uwe Kleine-König

and the build works fine for me and there is no warning about drivers/pwm/pwm-mediatek.o. What am I missing?

Could be compiler related; IIRC it is mostly clang that does this. When it finds /0 it simply stops code-gen.

I *really* dislike this behaviour, but since C declares this UB, they're basically free to do whatever.

IMO it should just emit the code; kernel has exceptions to deal with this and userspace gets signals.

author maintainer
#60 lore raw

Re: [tip: objtool/urgent] objtool, pwm: mediatek: Prevent theoretical divide-by-zero in pwm_mediatek_config()

In reply to Uwe Kleine-König
Quoted 8 lines · Uwe Kleine-König

I wonder a bit about procedures here. While I like that warnings that pop up in drivers/pwm (and elsewhere) are cared for, I think that the sensible way to change warning related settings is to make it hard to enable them first (harder than "depends on !COMPILE_TEST" "To avoid breaking bots too badly") and then work on the identified problems before warning broadly. The way chosen here instead seems to be enabling the warning immediately and then post fixes to the warnings and merge them without respective maintainer feedback in less than 12 hours.

Actually, this type of warning has existed for years. Nothing in the recent objtool patches enabled it.

I only discovered this particular one a few days ago. I suspect it only exists with newer compilers.

Quoted 9 lines

I fail to reproduce the warning here for an x86_64 build on 1e26c5e28ca5. I have:

        $ grep -E 'CONFIG_(CLK|PWM_MEDIATEK|OBJTOOL_WERROR)\>' .config
        CONFIG_PWM_MEDIATEK=m
        CONFIG_OBJTOOL_WERROR=y

and the build works fine for me and there is no warning about drivers/pwm/pwm-mediatek.o. What am I missing?

Sorry, I should have given more details about that. It was likely something with KCOV and/or UBSAN, though I can't seem to recreate it at the moment either :-/

Signature · 1 line
Josh
#61 lore raw

Re: [tip: objtool/urgent] objtool, pwm: mediatek: Prevent theoretical divide-by-zero in pwm_mediatek_config()

Hello Josh,

Quoted 10 lines · Josh Poimboeuf
Quoted 8 lines · Uwe Kleine-König

I wonder a bit about procedures here. While I like that warnings that pop up in drivers/pwm (and elsewhere) are cared for, I think that the sensible way to change warning related settings is to make it hard to enable them first (harder than "depends on !COMPILE_TEST" "To avoid breaking bots too badly") and then work on the identified problems before warning broadly. The way chosen here instead seems to be enabling the warning immediately and then post fixes to the warnings and merge them without respective maintainer feedback in less than 12 hours.

Quoted 2 lines

Actually, this type of warning has existed for years. Nothing in the recent objtool patches enabled it.

Yes, I understood that the recent patch only made the warning fatal.

Quoted 10 lines

I only discovered this particular one a few days ago. I suspect it only exists with newer compilers.

Quoted 7 lines

I fail to reproduce the warning here for an x86_64 build on 1e26c5e28ca5. I have:

        $ grep -E 'CONFIG_(CLK|PWM_MEDIATEK|OBJTOOL_WERROR)\>' .config
        CONFIG_PWM_MEDIATEK=m
        CONFIG_OBJTOOL_WERROR=y
Quoted 6 lines
Quoted 2 lines

and the build works fine for me and there is no warning about drivers/pwm/pwm-mediatek.o. What am I missing?

Sorry, I should have given more details about that. It was likely something with KCOV and/or UBSAN, though I can't seem to recreate it at the moment either :-/

The combination "existed for years" + "discovered a few days ago" + needs particular combination of .config and toolchain makes me believe the right way to fix is a medium priority patch via the maintainer of the affected driver. I don't see the urgency to justify the current procedure.

I don't know what the current merge plan is, but if you want to merge it through the pwm tree, I'm willing to take it. I wouldn't create a pull request just for that before 6.15, but send it along if something more urgent pops up.

Best regards
Uwe

A message that is not in the archive (sent off-list or not yet received): m7pgkp3ueo7iqgqf74upjrihr3mpmb3sqhwegnjxxwsrgx2jsw@dnec5iqiyobh

A message that is not in the archive (sent off-list or not yet received): [email protected]

#64 lore raw

Re: [tip: objtool/urgent] objtool, pwm: mediatek: Prevent theoretical divide-by-zero in pwm_mediatek_config()

Hello Ingo,

Quoted 12 lines · Ingo Molnar
Quoted 9 lines · Uwe Kleine-König

I wonder a bit about procedures here. While I like that warnings that pop up in drivers/pwm (and elsewhere) are cared for, I think that the sensible way to change warning related settings is to make it hard to enable them first (harder than "depends on !COMPILE_TEST" "To avoid breaking bots too badly") and then work on the identified problems before warning broadly. The way chosen here instead seems to be enabling the warning immediately and then post fixes to the warnings and merge them without respective maintainer feedback in less than 12 hours.

Quoted 1 line

As I indicated elsewhere in this thread, it's a WIP branch, so we'll

That sounds as if I should know that. But it's neither in the part of the thread that I was Cc:d, nor in the cover letter.

Quoted 4 lines

rebase it if/as we get feedback from maintainers: fix or skip the patch on negative feedback, adding in tags on positive feedback.

Does this particular patch look good to you?

I fail to see an urgency and so think this patch should better go via the pwm tree. Do you consider it urgent (as the branch name suggests)? Or is this v6.16 material?

Quoted 5 lines
Quoted 3 lines
Quoted 3 lines

I've also tentatively added your Acked-by, if that's OK with you.

The patch is OK. Iff you can convince me that it should go via tip, it's fine for me.

Best regards
Uwe

#65 lore raw

Re: [tip: objtool/urgent] objtool, pwm: mediatek: Prevent theoretical divide-by-zero in pwm_mediatek_config()

Quoted 7 lines · Uwe Kleine-König
Quoted 5 lines
Quoted 3 lines
Quoted 3 lines

I've also tentatively added your Acked-by, if that's OK with you.

The patch is OK.

Thanks!

Quoted 2 lines

[...] Iff you can convince me that it should go via tip, it's fine for me.

I wanted to collect all the objtool fixes for CONFIG_OBJTOOL_WERROR=y failures in a single place to not inconvenience randconfig CI testing efforts, so in that sense it would be nice to send this via the objtool/urgent tree, but I'll remove it if you insist.

Thanks,

Ingo

#66 lore raw

Re: [tip: objtool/urgent] objtool, pwm: mediatek: Prevent theoretical divide-by-zero in pwm_mediatek_config()

Hello Ingo,

Quoted 9 lines · Ingo Molnar
Quoted 6 lines · Uwe Kleine-König
Quoted 5 lines
Quoted 3 lines
Quoted 3 lines

I've also tentatively added your Acked-by, if that's OK with you.

Quoted 8 lines
Quoted 1 line

The patch is OK.

Thanks!

Quoted 2 lines

[...] Iff you can convince me that it should go via tip, it's fine for me.

I wanted to collect all the objtool fixes for CONFIG_OBJTOOL_WERROR=y

Regarding "all": Taking a quick look, I wonder if drivers/pwm/pwm-fsl-ftm.c is also affected. Given I cannot reproduce the warning for the pwm-mediatek driver, I can only guess. This driver does:

	static unsigned int fsl_pwm_ticks_to_ns(struct fsl_pwm_chip *fpc,
						  unsigned int ticks)
	{
		unsigned long rate;
		unsigned long long exval;

		rate = clk_get_rate(fpc->clk[fpc->period.clk_select]);
		exval = ticks;
		exval *= 1000000000UL;
		do_div(exval, rate >> fpc->period.clk_ps);
		return exval;
	}

and doesn't depend on HAVE_CLK. So similar to the pwm-mediatek driver there are configurations where clk_get_rate() returns 0.

Assuming this is indeed a problem, I wonder how many more offenders there are and if CONFIG_OBJTOOL_WERROR is ready for prime time already.

Quoted 3 lines

failures in a single place to not inconvenience randconfig CI testing efforts, so in that sense it would be nice to send this via the objtool/urgent tree, but I'll remove it if you insist.

I'm still in the process to determine my opinion on that.

Best regards
Uwe

#67 lore raw

Re: [tip: objtool/urgent] objtool, pwm: mediatek: Prevent theoretical divide-by-zero in pwm_mediatek_config()

Quoted 5 lines · Uwe Kleine-König
Quoted 3 lines

failures in a single place to not inconvenience randconfig CI testing efforts, so in that sense it would be nice to send this via the objtool/urgent tree, but I'll remove it if you insist.

I'm still in the process to determine my opinion on that.

Although Josh's fix looks obviously correct to me, I've removed the commit from objtool/urgent, because your indecision about it is blocking other fixes. Feel free to pick up the fix in your tree.

Thanks,

Ingo

#68 lore raw

Re: [tip: objtool/urgent] objtool, pwm: mediatek: Prevent theoretical divide-by-zero in pwm_mediatek_config()

Hello Ingo,

Quoted 11 lines · Ingo Molnar
Quoted 5 lines · Uwe Kleine-König
Quoted 3 lines

failures in a single place to not inconvenience randconfig CI testing efforts, so in that sense it would be nice to send this via the objtool/urgent tree, but I'll remove it if you insist.

I'm still in the process to determine my opinion on that.

Although Josh's fix looks obviously correct to me, I've removed the commit from objtool/urgent, because your indecision about it is blocking other fixes.

This isn't exactly the reaction that I expected, but of course you're entitled to do that.

Quoted 1 line

Feel free to pick up the fix in your tree.

I'd like to understand if there are still more fixes needed similar to the pwm-mediatek one. I managed to reproduce that issue and will play around with that a bit to check if some more fixes are needed.

Best regards
Uwe

merge bot Applied
#69 lore raw

[tip: objtool/urgent] objtool, pwm: mediatek: Prevent theoretical divide-by-zero in pwm_mediatek_config()

In reply to Josh Poimboeuf
The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     2dad2ba8f2487de932d503a6cd1095c31b2261c9
Gitweb:        https://git.kernel.org/tip/2dad2ba8f2487de932d503a6cd1095c31b2261c9
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:11 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 23:00:44 +01:00

objtool, pwm: mediatek: Prevent theoretical divide-by-zero in pwm_mediatek_config()

With CONFIG_COMPILE_TEST && !CONFIG_CLK, pwm_mediatek_config() has a divide-by-zero in the following line:

	do_div(resolution, clk_get_rate(pc->clk_pwms[pwm->hwpwm]));

due to the fact that the !CONFIG_CLK version of clk_get_rate() returns zero.

This is presumably just a theoretical problem: COMPILE_TEST overrides the dependency on RALINK which would select COMMON_CLK. Regardless it's a good idea to check for the error explicitly to avoid divide-by-zero.

Fixes the following warning:

drivers/pwm/pwm-mediatek.o: warning: objtool: .text: unexpected end of section

---
 drivers/pwm/pwm-mediatek.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/pwm/pwm-mediatek.c +6 -2
index 01dfa0f..7eaab58 100644
@@ -121,21 +121,25 @@ static int pwm_mediatek_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct pwm_mediatek_chip *pc = to_pwm_mediatek_chip(chip);
 	u32 clkdiv = 0, cnt_period, cnt_duty, reg_width = PWMDWIDTH,
 	    reg_thres = PWMTHRES;
+	unsigned long clk_rate;
 	u64 resolution;
 	int ret;
 
 	ret = pwm_mediatek_clk_enable(chip, pwm);
-
 	if (ret < 0)
 		return ret;
 
+	clk_rate = clk_get_rate(pc->clk_pwms[pwm->hwpwm]);
+	if (!clk_rate)
+		return -EINVAL;
+
 	/* Make sure we use the bus clock and not the 26MHz clock */
 	if (pc->soc->has_ck_26m_sel)
 		writel(0, pc->regs + PWM_CK_26M_SEL);
 
 	/* Using resolution in picosecond gets accuracy higher */
 	resolution = (u64)NSEC_PER_SEC * 1000;
-	do_div(resolution, clk_get_rate(pc->clk_pwms[pwm->hwpwm]));
+	do_div(resolution, clk_rate);
 
 	cnt_period = DIV_ROUND_CLOSEST_ULL((u64)period_ns * 1000, resolution);
 	while (cnt_period > 8191) {
merge bot Applied
#70 lore raw

[tip: objtool/urgent] objtool, pwm: mediatek: Prevent theoretical divide-by-zero in pwm_mediatek_config()

In reply to Josh Poimboeuf
The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     4e8af314386ec3e0b842f47705321e074b1dd556
Gitweb:        https://git.kernel.org/tip/4e8af314386ec3e0b842f47705321e074b1dd556
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:11 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Thu, 27 Mar 2025 11:57:45 +01:00

objtool, pwm: mediatek: Prevent theoretical divide-by-zero in pwm_mediatek_config()

With CONFIG_COMPILE_TEST && !CONFIG_CLK, pwm_mediatek_config() has a divide-by-zero in the following line:

	do_div(resolution, clk_get_rate(pc->clk_pwms[pwm->hwpwm]));

due to the fact that the !CONFIG_CLK version of clk_get_rate() returns zero.

This is presumably just a theoretical problem: COMPILE_TEST overrides the dependency on RALINK which would select COMMON_CLK. Regardless it's a good idea to check for the error explicitly to avoid divide-by-zero.

Fixes the following warning:

drivers/pwm/pwm-mediatek.o: warning: objtool: .text: unexpected end of section

---
 drivers/pwm/pwm-mediatek.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/pwm/pwm-mediatek.c +6 -2
index 01dfa0f..7eaab58 100644
@@ -121,21 +121,25 @@ static int pwm_mediatek_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct pwm_mediatek_chip *pc = to_pwm_mediatek_chip(chip);
 	u32 clkdiv = 0, cnt_period, cnt_duty, reg_width = PWMDWIDTH,
 	    reg_thres = PWMTHRES;
+	unsigned long clk_rate;
 	u64 resolution;
 	int ret;
 
 	ret = pwm_mediatek_clk_enable(chip, pwm);
-
 	if (ret < 0)
 		return ret;
 
+	clk_rate = clk_get_rate(pc->clk_pwms[pwm->hwpwm]);
+	if (!clk_rate)
+		return -EINVAL;
+
 	/* Make sure we use the bus clock and not the 26MHz clock */
 	if (pc->soc->has_ck_26m_sel)
 		writel(0, pc->regs + PWM_CK_26M_SEL);
 
 	/* Using resolution in picosecond gets accuracy higher */
 	resolution = (u64)NSEC_PER_SEC * 1000;
-	do_div(resolution, clk_get_rate(pc->clk_pwms[pwm->hwpwm]));
+	do_div(resolution, clk_rate);
 
 	cnt_period = DIV_ROUND_CLOSEST_ULL((u64)period_ns * 1000, resolution);
 	while (cnt_period > 8191) {
author maintainer
#71 lore raw

PATCH 22/22 lkdtm: Obfuscate do_nothing() pointer

In reply to Josh Poimboeuf

If execute_location()'s memcpy of do_nothing() gets inlined and unrolled by the compiler, it copies one word at a time:

    mov    0x0(%rip),%rax    R_X86_64_PC32    .text+0x1374
    mov    %rax,0x38(%rbx)
    mov    0x0(%rip),%rax    R_X86_64_PC32    .text+0x136c
    mov    %rax,0x30(%rbx)
    ...

Those .text references point to the middle of the function, causing objtool to complain about their lack of ENDBR.

Prevent that by resolving the function pointer at runtime rather than build time. This fixes the following warning:

drivers/misc/lkdtm/lkdtm.o: warning: objtool: execute_location+0x23: relocation to !ENDBR: .text+0x1378

---
 drivers/misc/lkdtm/perms.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
drivers/misc/lkdtm/perms.c +11 -3
index 5b861dbff27e..6c24426104ba 100644
@@ -28,6 +28,13 @@ static const unsigned long rodata = 0xAA55AA55;
 /* This is marked __ro_after_init, so it should ultimately be .rodata. */
 static unsigned long ro_after_init __ro_after_init = 0x55AA5500;
 
+/*
+ * This is a pointer to do_nothing() which is initialized at runtime rather
+ * than build time to avoid objtool IBT validation warnings caused by an
+ * inlined unrolled memcpy() in execute_location().
+ */
+static void __ro_after_init *do_nothing_ptr;
+
 /*
  * This just returns to the caller. It is designed to be copied into
  * non-executable memory regions.
@@ -65,13 +72,12 @@ static noinline __nocfi void execute_location(void *dst, bool write)
 {
 	void (*func)(void);
 	func_desc_t fdesc;
-	void *do_nothing_text = dereference_function_descriptor(do_nothing);
 
-	pr_info("attempting ok execution at %px\n", do_nothing_text);
+	pr_info("attempting ok execution at %px\n", do_nothing_ptr);
 	do_nothing();
 
 	if (write == CODE_WRITE) {
-		memcpy(dst, do_nothing_text, EXEC_SIZE);
+		memcpy(dst, do_nothing_ptr, EXEC_SIZE);
 		flush_icache_range((unsigned long)dst,
 				   (unsigned long)dst + EXEC_SIZE);
 	}
@@ -267,6 +273,8 @@ static void lkdtm_ACCESS_NULL(void)
 
 void __init lkdtm_perms_init(void)
 {
+	do_nothing_ptr = dereference_function_descriptor(do_nothing);
+
 	/* Make sure we can write to __ro_after_init values during __init */
 	ro_after_init |= 0xAA;
 }
Signature · 1 line
2.48.1
merge bot Applied
#72 lore raw

[tip: objtool/urgent] objtool, lkdtm: Obfuscate the do_nothing() pointer

The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     33fea486a42d952a40eb110a9b7885a7ac8d2778
Gitweb:        https://git.kernel.org/tip/33fea486a42d952a40eb110a9b7885a7ac8d2778
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:12 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 09:20:33 +01:00

objtool, lkdtm: Obfuscate the do_nothing() pointer

If execute_location()'s memcpy of do_nothing() gets inlined and unrolled by the compiler, it copies one word at a time:

    mov    0x0(%rip),%rax    R_X86_64_PC32    .text+0x1374
    mov    %rax,0x38(%rbx)
    mov    0x0(%rip),%rax    R_X86_64_PC32    .text+0x136c
    mov    %rax,0x30(%rbx)
    ...

Those .text references point to the middle of the function, causing objtool to complain about their lack of ENDBR.

Prevent that by resolving the function pointer at runtime rather than build time. This fixes the following warning:

drivers/misc/lkdtm/lkdtm.o: warning: objtool: execute_location+0x23: relocation to !ENDBR: .text+0x1378

---
 drivers/misc/lkdtm/perms.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
drivers/misc/lkdtm/perms.c +11 -3
index 5b861db..6c24426 100644
@@ -29,6 +29,13 @@ static const unsigned long rodata = 0xAA55AA55;
 static unsigned long ro_after_init __ro_after_init = 0x55AA5500;
 
 /*
+ * This is a pointer to do_nothing() which is initialized at runtime rather
+ * than build time to avoid objtool IBT validation warnings caused by an
+ * inlined unrolled memcpy() in execute_location().
+ */
+static void __ro_after_init *do_nothing_ptr;
+
+/*
  * This just returns to the caller. It is designed to be copied into
  * non-executable memory regions.
  */
@@ -65,13 +72,12 @@ static noinline __nocfi void execute_location(void *dst, bool write)
 {
 	void (*func)(void);
 	func_desc_t fdesc;
-	void *do_nothing_text = dereference_function_descriptor(do_nothing);
 
-	pr_info("attempting ok execution at %px\n", do_nothing_text);
+	pr_info("attempting ok execution at %px\n", do_nothing_ptr);
 	do_nothing();
 
 	if (write == CODE_WRITE) {
-		memcpy(dst, do_nothing_text, EXEC_SIZE);
+		memcpy(dst, do_nothing_ptr, EXEC_SIZE);
 		flush_icache_range((unsigned long)dst,
 				   (unsigned long)dst + EXEC_SIZE);
 	}
@@ -267,6 +273,8 @@ static void lkdtm_ACCESS_NULL(void)
 
 void __init lkdtm_perms_init(void)
 {
+	do_nothing_ptr = dereference_function_descriptor(do_nothing);
+
 	/* Make sure we can write to __ro_after_init values during __init */
 	ro_after_init |= 0xAA;
 }
maintainer Reviewed-by on v1 22/22
#73 lore raw

Re: [PATCH 22/22] lkdtm: Obfuscate do_nothing() pointer

In reply to Josh Poimboeuf
Quoted 15 lines · Josh Poimboeuf

If execute_location()'s memcpy of do_nothing() gets inlined and unrolled by the compiler, it copies one word at a time:

    mov    0x0(%rip),%rax    R_X86_64_PC32    .text+0x1374
    mov    %rax,0x38(%rbx)
    mov    0x0(%rip),%rax    R_X86_64_PC32    .text+0x136c
    mov    %rax,0x30(%rbx)
    ...

Those .text references point to the middle of the function, causing objtool to complain about their lack of ENDBR.

Prevent that by resolving the function pointer at runtime rather than build time. This fixes the following warning:

Quoted 6 lines

drivers/misc/lkdtm/lkdtm.o: warning: objtool: execute_location+0x23: relocation to !ENDBR: .text+0x1378

Thanks!

Signature · 1 line
Kees Cook
merge bot Applied
#74 lore raw

[tip: objtool/urgent] objtool, lkdtm: Obfuscate the do_nothing() pointer

In reply to Josh Poimboeuf
The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     26d2ab43acdd1fd5fab3c3abb0b79d7ff14fedaa
Gitweb:        https://git.kernel.org/tip/26d2ab43acdd1fd5fab3c3abb0b79d7ff14fedaa
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:12 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Tue, 25 Mar 2025 23:00:45 +01:00

objtool, lkdtm: Obfuscate the do_nothing() pointer

If execute_location()'s memcpy of do_nothing() gets inlined and unrolled by the compiler, it copies one word at a time:

    mov    0x0(%rip),%rax    R_X86_64_PC32    .text+0x1374
    mov    %rax,0x38(%rbx)
    mov    0x0(%rip),%rax    R_X86_64_PC32    .text+0x136c
    mov    %rax,0x30(%rbx)
    ...

Those .text references point to the middle of the function, causing objtool to complain about their lack of ENDBR.

Prevent that by resolving the function pointer at runtime rather than build time. This fixes the following warning:

drivers/misc/lkdtm/lkdtm.o: warning: objtool: execute_location+0x23: relocation to !ENDBR: .text+0x1378

---
 drivers/misc/lkdtm/perms.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
drivers/misc/lkdtm/perms.c +11 -3
index 5b861db..6c24426 100644
@@ -29,6 +29,13 @@ static const unsigned long rodata = 0xAA55AA55;
 static unsigned long ro_after_init __ro_after_init = 0x55AA5500;
 
 /*
+ * This is a pointer to do_nothing() which is initialized at runtime rather
+ * than build time to avoid objtool IBT validation warnings caused by an
+ * inlined unrolled memcpy() in execute_location().
+ */
+static void __ro_after_init *do_nothing_ptr;
+
+/*
  * This just returns to the caller. It is designed to be copied into
  * non-executable memory regions.
  */
@@ -65,13 +72,12 @@ static noinline __nocfi void execute_location(void *dst, bool write)
 {
 	void (*func)(void);
 	func_desc_t fdesc;
-	void *do_nothing_text = dereference_function_descriptor(do_nothing);
 
-	pr_info("attempting ok execution at %px\n", do_nothing_text);
+	pr_info("attempting ok execution at %px\n", do_nothing_ptr);
 	do_nothing();
 
 	if (write == CODE_WRITE) {
-		memcpy(dst, do_nothing_text, EXEC_SIZE);
+		memcpy(dst, do_nothing_ptr, EXEC_SIZE);
 		flush_icache_range((unsigned long)dst,
 				   (unsigned long)dst + EXEC_SIZE);
 	}
@@ -267,6 +273,8 @@ static void lkdtm_ACCESS_NULL(void)
 
 void __init lkdtm_perms_init(void)
 {
+	do_nothing_ptr = dereference_function_descriptor(do_nothing);
+
 	/* Make sure we can write to __ro_after_init values during __init */
 	ro_after_init |= 0xAA;
 }
merge bot Applied
#75 lore raw

[tip: objtool/urgent] objtool, lkdtm: Obfuscate the do_nothing() pointer

In reply to Josh Poimboeuf
The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     42e5650944b8cd4361385cc9414f03a6d6748989
Gitweb:        https://git.kernel.org/tip/42e5650944b8cd4361385cc9414f03a6d6748989
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:12 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Thu, 27 Mar 2025 11:59:11 +01:00

objtool, lkdtm: Obfuscate the do_nothing() pointer

If execute_location()'s memcpy of do_nothing() gets inlined and unrolled by the compiler, it copies one word at a time:

    mov    0x0(%rip),%rax    R_X86_64_PC32    .text+0x1374
    mov    %rax,0x38(%rbx)
    mov    0x0(%rip),%rax    R_X86_64_PC32    .text+0x136c
    mov    %rax,0x30(%rbx)
    ...

Those .text references point to the middle of the function, causing objtool to complain about their lack of ENDBR.

Prevent that by resolving the function pointer at runtime rather than build time. This fixes the following warning:

drivers/misc/lkdtm/lkdtm.o: warning: objtool: execute_location+0x23: relocation to !ENDBR: .text+0x1378

---
 drivers/misc/lkdtm/perms.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
drivers/misc/lkdtm/perms.c +11 -3
index 5b861db..6c24426 100644
@@ -29,6 +29,13 @@ static const unsigned long rodata = 0xAA55AA55;
 static unsigned long ro_after_init __ro_after_init = 0x55AA5500;
 
 /*
+ * This is a pointer to do_nothing() which is initialized at runtime rather
+ * than build time to avoid objtool IBT validation warnings caused by an
+ * inlined unrolled memcpy() in execute_location().
+ */
+static void __ro_after_init *do_nothing_ptr;
+
+/*
  * This just returns to the caller. It is designed to be copied into
  * non-executable memory regions.
  */
@@ -65,13 +72,12 @@ static noinline __nocfi void execute_location(void *dst, bool write)
 {
 	void (*func)(void);
 	func_desc_t fdesc;
-	void *do_nothing_text = dereference_function_descriptor(do_nothing);
 
-	pr_info("attempting ok execution at %px\n", do_nothing_text);
+	pr_info("attempting ok execution at %px\n", do_nothing_ptr);
 	do_nothing();
 
 	if (write == CODE_WRITE) {
-		memcpy(dst, do_nothing_text, EXEC_SIZE);
+		memcpy(dst, do_nothing_ptr, EXEC_SIZE);
 		flush_icache_range((unsigned long)dst,
 				   (unsigned long)dst + EXEC_SIZE);
 	}
@@ -267,6 +273,8 @@ static void lkdtm_ACCESS_NULL(void)
 
 void __init lkdtm_perms_init(void)
 {
+	do_nothing_ptr = dereference_function_descriptor(do_nothing);
+
 	/* Make sure we can write to __ro_after_init values during __init */
 	ro_after_init |= 0xAA;
 }
merge bot Applied
#76 lore raw

[tip: objtool/urgent] objtool, lkdtm: Obfuscate the do_nothing() pointer

In reply to Josh Poimboeuf
The following commit has been merged into the objtool/urgent branch of tip:

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     05026ea01e95ffdeb0e5ac8fb7fb1b551e3a8726
Gitweb:        https://git.kernel.org/tip/05026ea01e95ffdeb0e5ac8fb7fb1b551e3a8726
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 24 Mar 2025 14:56:12 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Fri, 28 Mar 2025 14:38:09 +01:00

objtool, lkdtm: Obfuscate the do_nothing() pointer

If execute_location()'s memcpy of do_nothing() gets inlined and unrolled by the compiler, it copies one word at a time:

    mov    0x0(%rip),%rax    R_X86_64_PC32    .text+0x1374
    mov    %rax,0x38(%rbx)
    mov    0x0(%rip),%rax    R_X86_64_PC32    .text+0x136c
    mov    %rax,0x30(%rbx)
    ...

Those .text references point to the middle of the function, causing objtool to complain about their lack of ENDBR.

Prevent that by resolving the function pointer at runtime rather than build time. This fixes the following warning:

drivers/misc/lkdtm/lkdtm.o: warning: objtool: execute_location+0x23: relocation to !ENDBR: .text+0x1378

---
 drivers/misc/lkdtm/perms.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
drivers/misc/lkdtm/perms.c +11 -3
index 5b861db..6c24426 100644
@@ -29,6 +29,13 @@ static const unsigned long rodata = 0xAA55AA55;
 static unsigned long ro_after_init __ro_after_init = 0x55AA5500;
 
 /*
+ * This is a pointer to do_nothing() which is initialized at runtime rather
+ * than build time to avoid objtool IBT validation warnings caused by an
+ * inlined unrolled memcpy() in execute_location().
+ */
+static void __ro_after_init *do_nothing_ptr;
+
+/*
  * This just returns to the caller. It is designed to be copied into
  * non-executable memory regions.
  */
@@ -65,13 +72,12 @@ static noinline __nocfi void execute_location(void *dst, bool write)
 {
 	void (*func)(void);
 	func_desc_t fdesc;
-	void *do_nothing_text = dereference_function_descriptor(do_nothing);
 
-	pr_info("attempting ok execution at %px\n", do_nothing_text);
+	pr_info("attempting ok execution at %px\n", do_nothing_ptr);
 	do_nothing();
 
 	if (write == CODE_WRITE) {
-		memcpy(dst, do_nothing_text, EXEC_SIZE);
+		memcpy(dst, do_nothing_ptr, EXEC_SIZE);
 		flush_icache_range((unsigned long)dst,
 				   (unsigned long)dst + EXEC_SIZE);
 	}
@@ -267,6 +273,8 @@ static void lkdtm_ACCESS_NULL(void)
 
 void __init lkdtm_perms_init(void)
 {
+	do_nothing_ptr = dereference_function_descriptor(do_nothing);
+
 	/* Make sure we can write to __ro_after_init values during __init */
 	ro_after_init |= 0xAA;
 }