[PATCH v2 3/5] objtool: Add support for annotated jump tables

Ard Biesheuvel posted 5 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v2 3/5] objtool: Add support for annotated jump tables
Posted by Ard Biesheuvel 1 month, 2 weeks ago
From: Ard Biesheuvel <ardb@kernel.org>

Add logic to follow R_X86_64_NONE relocations attached to indirect
jumps, which are emitted to annotate jump tables, which are otherwise
difficult to spot reliably.

If an ELF symbol is associated with the jump table, its size is taken as
the size of the jump table, and subsequently used to limit the traversal
of the table and validate its jump destinations.

One complicating factor is that indirect jumps may actually be direct
jumps to retpoline thunks, and therefore already have a relocation
associated with it. Accommodate these by ignoring R_*_NONE relocations
in insn_reloc(), so that the existing code does not get confused by
them.

E.g.,

  8c:   48 63 7c 85 00          movslq 0x0(%rbp,%rax,4),%rdi
  91:   48 01 ef                add    %rbp,%rdi
  94:   e9 00 00 00 00          jmp    99 <crc_pcl+0x89>
            94: R_X86_64_NONE       .rodata+0x400
            95: R_X86_64_PLT32      __x86_indirect_thunk_rdi-0x4

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 tools/objtool/arch/x86/special.c | 33 ++++++++++++++++----
 tools/objtool/check.c            | 10 ++++--
 2 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/tools/objtool/arch/x86/special.c b/tools/objtool/arch/x86/special.c
index f8fb67636384..67c20623d7f7 100644
--- a/tools/objtool/arch/x86/special.c
+++ b/tools/objtool/arch/x86/special.c
@@ -115,30 +115,51 @@ struct reloc *arch_find_switch_table(struct objtool_file *file,
 	struct reloc  *text_reloc, *rodata_reloc;
 	struct section *table_sec;
 	unsigned long table_offset;
+	struct symbol *sym;
 
 	/* look for a relocation which references .rodata */
 	text_reloc = find_reloc_by_dest_range(file->elf, insn->sec,
 					      insn->offset, insn->len);
-	if (!text_reloc || text_reloc->sym->type != STT_SECTION ||
-	    !text_reloc->sym->sec->rodata)
+	if (!text_reloc || !text_reloc->sym->sec->rodata)
 		return NULL;
 
-	table_offset = reloc_addend(text_reloc);
+	/*
+	 * If the indirect jump instruction itself is annotated with a
+	 * R_X86_64_NONE relocation, it should point to the jump table
+	 * in .rodata. In this case, the ELF symbol will give us the
+	 * size of the table. Ignore other occurrences of R_X86_64_NONE.
+	 */
+	if (reloc_type(text_reloc) == R_X86_64_NONE &&
+	    insn->type != INSN_JUMP_DYNAMIC)
+		return NULL;
+
+	table_offset = text_reloc->sym->offset + reloc_addend(text_reloc);
 	table_sec = text_reloc->sym->sec;
 
 	if (reloc_type(text_reloc) == R_X86_64_PC32)
 		table_offset += 4;
 
+	switch (text_reloc->sym->type) {
+	case STT_OBJECT:
+		sym = text_reloc->sym;
+		break;
+	case  STT_SECTION:
+		sym = find_symbol_containing(table_sec, table_offset);
+		break;
+	default:
+		return NULL;
+	}
+
 	/*
 	 * Make sure the .rodata address isn't associated with a
-	 * symbol.  GCC jump tables are anonymous data.
+	 * symbol. Unannotated GCC jump tables are anonymous data.
 	 *
 	 * Also support C jump tables which are in the same format as
 	 * switch jump tables.  For objtool to recognize them, they
 	 * need to be placed in the C_JUMP_TABLE_SECTION section.  They
 	 * have symbols associated with them.
 	 */
-	if (find_symbol_containing(table_sec, table_offset) &&
+	if (reloc_type(text_reloc) != R_X86_64_NONE && sym &&
 	    strcmp(table_sec->name, C_JUMP_TABLE_SECTION))
 		return NULL;
 
@@ -151,6 +172,6 @@ struct reloc *arch_find_switch_table(struct objtool_file *file,
 	if (!rodata_reloc)
 		return NULL;
 
-	*table_size = 0;
+	*table_size = sym ? sym->len : 0;
 	return rodata_reloc;
 }
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 5f711ac5b43d..6521c82880f0 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1386,6 +1386,8 @@ __weak const char *arch_nop_fentry_call(int len)
 
 static struct reloc *insn_reloc(struct objtool_file *file, struct instruction *insn)
 {
+	unsigned long offset = insn->offset;
+	unsigned int len = insn->len;
 	struct reloc *reloc;
 
 	if (insn->no_reloc)
@@ -1394,8 +1396,12 @@ static struct reloc *insn_reloc(struct objtool_file *file, struct instruction *i
 	if (!file)
 		return NULL;
 
-	reloc = find_reloc_by_dest_range(file->elf, insn->sec,
-					 insn->offset, insn->len);
+	do {
+		/* Skip any R_*_NONE relocations */
+		reloc = find_reloc_by_dest_range(file->elf, insn->sec,
+						 offset++, len--);
+	} while (len && reloc && reloc_type(reloc) == 0);
+
 	if (!reloc) {
 		insn->no_reloc = 1;
 		return NULL;
-- 
2.47.0.rc0.187.ge670bccf7e-goog
Re: [PATCH v2 3/5] objtool: Add support for annotated jump tables
Posted by Josh Poimboeuf 1 month, 2 weeks ago
On Thu, Oct 10, 2024 at 02:28:05PM +0200, Ard Biesheuvel wrote:
> +++ b/tools/objtool/arch/x86/special.c
> @@ -115,30 +115,51 @@ struct reloc *arch_find_switch_table(struct objtool_file *file,
>  	struct reloc  *text_reloc, *rodata_reloc;
>  	struct section *table_sec;
>  	unsigned long table_offset;
> +	struct symbol *sym;
>  
>  	/* look for a relocation which references .rodata */
>  	text_reloc = find_reloc_by_dest_range(file->elf, insn->sec,
>  					      insn->offset, insn->len);

Hm, we can probably put insn_reloc() in check.h and use that here to
take advantage of its caching for the no_reloc case.

> +	switch (text_reloc->sym->type) {
> +	case STT_OBJECT:
> +		sym = text_reloc->sym;
> +		break;
> +	case  STT_SECTION:
	     ^
	     extra whitespace

-- 
Josh
Re: [PATCH v2 3/5] objtool: Add support for annotated jump tables
Posted by Ard Biesheuvel 1 month, 2 weeks ago
On Thu, 10 Oct 2024 at 22:15, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Thu, Oct 10, 2024 at 02:28:05PM +0200, Ard Biesheuvel wrote:
> > +++ b/tools/objtool/arch/x86/special.c
> > @@ -115,30 +115,51 @@ struct reloc *arch_find_switch_table(struct objtool_file *file,
> >       struct reloc  *text_reloc, *rodata_reloc;
> >       struct section *table_sec;
> >       unsigned long table_offset;
> > +     struct symbol *sym;
> >
> >       /* look for a relocation which references .rodata */
> >       text_reloc = find_reloc_by_dest_range(file->elf, insn->sec,
> >                                             insn->offset, insn->len);
>
> Hm, we can probably put insn_reloc() in check.h and use that here to
> take advantage of its caching for the no_reloc case.
>

insn_reloc() filters out R_*_NONE relocations, for the reasons pointed
out in the commit log.
Re: [PATCH v2 3/5] objtool: Add support for annotated jump tables
Posted by Josh Poimboeuf 1 month, 2 weeks ago
On Fri, Oct 11, 2024 at 08:29:48AM +0200, Ard Biesheuvel wrote:
> On Thu, 10 Oct 2024 at 22:15, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > On Thu, Oct 10, 2024 at 02:28:05PM +0200, Ard Biesheuvel wrote:
> > > +++ b/tools/objtool/arch/x86/special.c
> > > @@ -115,30 +115,51 @@ struct reloc *arch_find_switch_table(struct objtool_file *file,
> > >       struct reloc  *text_reloc, *rodata_reloc;
> > >       struct section *table_sec;
> > >       unsigned long table_offset;
> > > +     struct symbol *sym;
> > >
> > >       /* look for a relocation which references .rodata */
> > >       text_reloc = find_reloc_by_dest_range(file->elf, insn->sec,
> > >                                             insn->offset, insn->len);
> >
> > Hm, we can probably put insn_reloc() in check.h and use that here to
> > take advantage of its caching for the no_reloc case.
> >
> 
> insn_reloc() filters out R_*_NONE relocations, for the reasons pointed
> out in the commit log.

True, unless this becomes the fallback code for the generic "no
annotations" case as I suggested elsewhere.

Regardless we should be sure to write and read insn->no_reloc where we
can to minimize duplicate reloc lookups.

-- 
Josh
Re: [PATCH v2 3/5] objtool: Add support for annotated jump tables
Posted by Josh Poimboeuf 1 month, 2 weeks ago
On Thu, Oct 10, 2024 at 02:28:05PM +0200, Ard Biesheuvel wrote:
> @@ -1394,8 +1396,12 @@ static struct reloc *insn_reloc(struct objtool_file *file, struct instruction *i
>  	if (!file)
>  		return NULL;
>  
> -	reloc = find_reloc_by_dest_range(file->elf, insn->sec,
> -					 insn->offset, insn->len);
> +	do {
> +		/* Skip any R_*_NONE relocations */
> +		reloc = find_reloc_by_dest_range(file->elf, insn->sec,
> +						 offset++, len--);
> +	} while (len && reloc && reloc_type(reloc) == 0);

s/0/R_NONE/

-- 
Josh