[PATCH 0/3] objtool: Fix undefined behaviour reported by UBSan on a defconfig build

[PATCH 0/3] objtool: Fix undefined behaviour reported by UBSan on a defconfig build

[Leizhen Zhang]

An x86_64 defconfig build of v7.4-rc2 with objtool built with
-fsanitize=address,undefined (GCC 12) gives two UBSan reports from
objtool, both on the normal vmlinux.o pass, no unusual input involved:

  arch/x86/orc.c:11:18: runtime error: member access within null pointer of type 'struct cfi_state'
  special.c:84:18: runtime error: load of misaligned address ... for type 'unsigned int', which requires 4 byte alignment

1/3 fixes the first one: init_orc_entry() takes &cfi->regs[CFI_BP]
before its own "if (!cfi)" check.  2/3 is the same pattern in the
LoongArch copy of the function; I compile-tested objtool for LoongArch
but have no LoongArch object to run it on.  3/3 fixes the second one:
the feature word of a 14-byte packed struct alt_instr is read through
an 'unsigned int *', so every odd-numbered entry is misaligned.

Testing: I relinked vmlinux.o without objtool and ran the old and the
new objtool binary on two copies of it, with the arguments Kbuild uses.
The old one prints both reports, the new one prints none, and the two
resulting vmlinux.o files are byte-identical.  A full rebuild with the
patched objtool is also clean for objtool.

None of this is a security issue: objtool only processes objects from
the build itself.

The two reports were found with UBSan, and the analysis and fixes were
prepared with the help of an AI coding assistant; I reviewed and tested
the result.

Leizhen Zhang (3):
  objtool/x86: Move the cfi->regs access after the NULL check
  objtool/LoongArch: Move the cfi->regs access after the NULL check
  objtool: Fix misaligned load of the alternative feature word

 tools/objtool/arch/loongarch/orc.c | 6 ++++--
 tools/objtool/arch/x86/orc.c       | 4 +++-
 tools/objtool/special.c            | 4 ++--
 3 files changed, 9 insertions(+), 5 deletions(-)


base-commit: 7475ea8b58616b33c04afb976cf0fbaad9dd63fb
-- 
2.34.1

[PATCH 1/3] objtool/x86: Move the cfi->regs access after the NULL check

[Leizhen Zhang]

init_orc_entry() takes &cfi->regs[CFI_BP] before it checks whether cfi
is NULL, and a NULL cfi is an expected case: the comment right below
says it happens for unreachable nops/traps and for
STACK_FRAME_NON_STANDARD functions.

Forming a member address through a NULL pointer is undefined behaviour,
and since the dereference comes first the compiler is free to assume
cfi != NULL and drop the check that follows.  An x86_64 defconfig build
of v7.4-rc2 with objtool built with -fsanitize=undefined hits it in
every objtool run, on vmlinux.o and on each module, all through
orc_create():

  arch/x86/orc.c:11:18: runtime error: member access within null pointer of type 'struct cfi_state'
    #0 init_orc_entry arch/x86/orc.c:11
    #1 orc_create tools/objtool/orc_gen.c:72
    #2 check tools/objtool/check.c:4939

Take the address only after the NULL check.  Running objtool with and
without this patch on the same defconfig vmlinux.o gives byte-identical
output.

The report was found with UBSan, and the analysis and fix were
prepared with the help of an AI coding assistant; I reviewed and
tested the result.

Fixes: 8b946cc38e06 ("objtool: Introduce CFI hash")
Signed-off-by: Leizhen Zhang <lzsx618@gmail.com>
---
 tools/objtool/arch/x86/orc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/objtool/arch/x86/orc.c b/tools/objtool/arch/x86/orc.c
index eff078ecc945..b688afcc4ffc 100644
--- a/tools/objtool/arch/x86/orc.c
+++ b/tools/objtool/arch/x86/orc.c
@@ -8,7 +8,7 @@
 
 int init_orc_entry(struct orc_entry *orc, struct cfi_state *cfi, struct instruction *insn)
 {
-	struct cfi_reg *bp = &cfi->regs[CFI_BP];
+	struct cfi_reg *bp;
 
 	memset(orc, 0, sizeof(*orc));
 
@@ -22,6 +22,8 @@ int init_orc_entry(struct orc_entry *orc, struct cfi_state *cfi, struct instruct
 		return 0;
 	}
 
+	bp = &cfi->regs[CFI_BP];
+
 	switch (cfi->type) {
 	case UNWIND_HINT_TYPE_UNDEFINED:
 		orc->type = ORC_TYPE_UNDEFINED;
-- 
2.34.1

[PATCH 2/3] objtool/LoongArch: Move the cfi->regs access after the NULL check

[Leizhen Zhang]

init_orc_entry() takes &cfi->regs[CFI_FP] and &cfi->regs[CFI_RA] before
it checks whether cfi is NULL, although a NULL cfi is an expected case
(unreachable nops/traps and STACK_FRAME_NON_STANDARD functions, as the
comment right below says).

Forming a member address through a NULL pointer is undefined behaviour,
and it lets the compiler assume cfi != NULL and drop the check that
follows.  The x86 copy of this function has the same problem, and UBSan
reports it there on every defconfig build.

Take the addresses only after the NULL check.

The report was found with UBSan, and the analysis and fix were
prepared with the help of an AI coding assistant; I reviewed and
tested the result.

Fixes: 3c7266cd7bc5 ("objtool/LoongArch: Enable orc to be built")
Signed-off-by: Leizhen Zhang <lzsx618@gmail.com>
---
 tools/objtool/arch/loongarch/orc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/objtool/arch/loongarch/orc.c b/tools/objtool/arch/loongarch/orc.c
index ffd3a3c858ae..960b59d5e566 100644
--- a/tools/objtool/arch/loongarch/orc.c
+++ b/tools/objtool/arch/loongarch/orc.c
@@ -8,8 +8,7 @@
 
 int init_orc_entry(struct orc_entry *orc, struct cfi_state *cfi, struct instruction *insn)
 {
-	struct cfi_reg *fp = &cfi->regs[CFI_FP];
-	struct cfi_reg *ra = &cfi->regs[CFI_RA];
+	struct cfi_reg *fp, *ra;
 
 	memset(orc, 0, sizeof(*orc));
 
@@ -23,6 +22,9 @@ int init_orc_entry(struct orc_entry *orc, struct cfi_state *cfi, struct instruct
 		return 0;
 	}
 
+	fp = &cfi->regs[CFI_FP];
+	ra = &cfi->regs[CFI_RA];
+
 	switch (cfi->type) {
 	case UNWIND_HINT_TYPE_UNDEFINED:
 		orc->type = ORC_TYPE_UNDEFINED;
-- 
2.34.1

[PATCH 3/3] objtool: Fix misaligned load of the alternative feature word

[Leizhen Zhang]

get_alt_entry() reads the feature word of a group alternative through
an 'unsigned int *' at offset + entry->feature.  On x86 struct alt_instr
is packed and 14 bytes long (ALT_ENTRY_SIZE) with the feature word at
offset 8, so for every odd-numbered .altinstructions entry the load is
only 2-byte aligned.  That is undefined behaviour in C, and UBSan
reports it on an x86_64 defconfig build of v7.4-rc2:

  special.c:84:18: runtime error: load of misaligned address 0x52e000018416 for type 'unsigned int', which requires 4 byte alignment

x86 hosts tolerate the access, but objtool may run on hosts that do
not, and the compiler is allowed to assume the alignment.  Copy the
value out with memcpy() instead.  Running objtool with and without
this patch on the same defconfig vmlinux.o gives byte-identical output.

The report was found with UBSan, and the analysis and fix were
prepared with the help of an AI coding assistant; I reviewed and
tested the result.

Fixes: be5ee60ac554 ("objtool: Provide access to feature and flags of group alternatives")
Signed-off-by: Leizhen Zhang <lzsx618@gmail.com>
---
 tools/objtool/special.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/objtool/special.c b/tools/objtool/special.c
index 2a533afbc69a..5df169d0e45d 100644
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c
@@ -81,8 +81,8 @@ static int get_alt_entry(struct elf *elf, const struct special_entry *entry,
 						   entry->orig_len);
 		alt->new_len = *(unsigned char *)(sec->data->d_buf + offset +
 						  entry->new_len);
-		alt->feature = *(unsigned int *)(sec->data->d_buf + offset +
-						 entry->feature);
+		memcpy(&alt->feature, sec->data->d_buf + offset + entry->feature,
+		       sizeof(alt->feature));
 	}
 
 	orig_reloc = find_reloc_by_dest(elf, sec, offset + entry->orig);
-- 
2.34.1