[PATCH v5 15/22] objtool: Add action to check for absence of absolute relocations

Ard Biesheuvel posted 22 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH v5 15/22] objtool: Add action to check for absence of absolute relocations
Posted by Ard Biesheuvel 1 month, 3 weeks ago
From: Ard Biesheuvel <ardb@kernel.org>

The x86 startup code must not use absolute references to code or data,
as it executes before the kernel virtual mapping is up.

Add an action to objtool to check all allocatable sections (with the
exception of __patchable_function_entries, which uses absolute
references for nebulous reasons) and raise an error if any absolute
references are found.

Note that debug sections typically contain lots of absolute references
too, but those are not allocatable so they will be ignored.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 tools/objtool/builtin-check.c           |  2 ++
 tools/objtool/check.c                   | 36 ++++++++++++++++++++
 tools/objtool/include/objtool/builtin.h |  1 +
 3 files changed, 39 insertions(+)

diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 80239843e9f0..0f6b197cfcb0 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -87,6 +87,7 @@ static const struct option check_options[] = {
 	OPT_BOOLEAN('t', "static-call", &opts.static_call, "annotate static calls"),
 	OPT_BOOLEAN('u', "uaccess", &opts.uaccess, "validate uaccess rules for SMAP"),
 	OPT_BOOLEAN(0  , "cfi", &opts.cfi, "annotate kernel control flow integrity (kCFI) function preambles"),
+	OPT_BOOLEAN(0  , "noabs", &opts.noabs, "reject absolute references in allocatable sections"),
 	OPT_CALLBACK_OPTARG(0, "dump", NULL, NULL, "orc", "dump metadata", parse_dump),
 
 	OPT_GROUP("Options:"),
@@ -162,6 +163,7 @@ static bool opts_valid(void)
 	    opts.hack_noinstr		||
 	    opts.ibt			||
 	    opts.mcount			||
+	    opts.noabs			||
 	    opts.noinstr		||
 	    opts.orc			||
 	    opts.retpoline		||
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index d967ac001498..5d1d38404892 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4643,6 +4643,39 @@ static void disas_warned_funcs(struct objtool_file *file)
 		disas_funcs(funcs);
 }
 
+static int check_abs_references(struct objtool_file *file)
+{
+	struct section *sec;
+	struct reloc *reloc;
+	int ret = 0;
+
+	for_each_sec(file, sec) {
+		/* absolute references in non-loadable sections are fine */
+		if (!(sec->sh.sh_flags & SHF_ALLOC))
+			continue;
+
+		/* section must have an associated .rela section */
+		if (!sec->rsec)
+			continue;
+
+		/*
+		 * Special case for compiler generated metadata that is not
+		 * consumed until after boot.
+		 */
+		if (!strcmp(sec->name, "__patchable_function_entries"))
+			continue;
+
+		for_each_reloc(sec->rsec, reloc) {
+			if (reloc_type(reloc) == R_ABS64) {
+				WARN("section %s has absolute relocation at offset 0x%lx",
+				     sec->name, reloc_offset(reloc));
+				ret++;
+			}
+		}
+	}
+	return ret;
+}
+
 struct insn_chunk {
 	void *addr;
 	struct insn_chunk *next;
@@ -4776,6 +4809,9 @@ int check(struct objtool_file *file)
 			goto out;
 	}
 
+	if (opts.noabs)
+		warnings += check_abs_references(file);
+
 	if (opts.orc && nr_insns) {
 		ret = orc_create(file);
 		if (ret)
diff --git a/tools/objtool/include/objtool/builtin.h b/tools/objtool/include/objtool/builtin.h
index 6b08666fa69d..ab22673862e1 100644
--- a/tools/objtool/include/objtool/builtin.h
+++ b/tools/objtool/include/objtool/builtin.h
@@ -26,6 +26,7 @@ struct opts {
 	bool uaccess;
 	int prefix;
 	bool cfi;
+	bool noabs;
 
 	/* options: */
 	bool backtrace;
-- 
2.50.0.727.gbf7dc18ff4-goog
Re: [PATCH v5 15/22] objtool: Add action to check for absence of absolute relocations
Posted by Peter Zijlstra 1 month, 2 weeks ago
On Wed, Jul 16, 2025 at 05:18:30AM +0200, Ard Biesheuvel wrote:
> index d967ac001498..5d1d38404892 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -4643,6 +4643,39 @@ static void disas_warned_funcs(struct objtool_file *file)
>  		disas_funcs(funcs);
>  }
>  
> +static int check_abs_references(struct objtool_file *file)
> +{
> +	struct section *sec;
> +	struct reloc *reloc;
> +	int ret = 0;
> +
> +	for_each_sec(file, sec) {
> +		/* absolute references in non-loadable sections are fine */
> +		if (!(sec->sh.sh_flags & SHF_ALLOC))
> +			continue;
> +
> +		/* section must have an associated .rela section */
> +		if (!sec->rsec)
> +			continue;
> +
> +		/*
> +		 * Special case for compiler generated metadata that is not
> +		 * consumed until after boot.
> +		 */
> +		if (!strcmp(sec->name, "__patchable_function_entries"))
> +			continue;
> +
> +		for_each_reloc(sec->rsec, reloc) {
> +			if (reloc_type(reloc) == R_ABS64) {

This should probably also check R_ABS32. Yes, your current only user is
x86_64 so R_ABS64 covers things, but we're getting more and more archs
using objtool. No reason this check shouldn't also work on PPC32 for
example.

> +				WARN("section %s has absolute relocation at offset 0x%lx",
> +				     sec->name, reloc_offset(reloc));
> +				ret++;
> +			}
> +		}
> +	}
> +	return ret;
> +}
Re: [PATCH v5 15/22] objtool: Add action to check for absence of absolute relocations
Posted by Ard Biesheuvel 1 month, 2 weeks ago
On Wed, 16 Jul 2025 at 19:54, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, Jul 16, 2025 at 05:18:30AM +0200, Ard Biesheuvel wrote:
> > index d967ac001498..5d1d38404892 100644
> > --- a/tools/objtool/check.c
> > +++ b/tools/objtool/check.c
> > @@ -4643,6 +4643,39 @@ static void disas_warned_funcs(struct objtool_file *file)
> >               disas_funcs(funcs);
> >  }
> >
> > +static int check_abs_references(struct objtool_file *file)
> > +{
> > +     struct section *sec;
> > +     struct reloc *reloc;
> > +     int ret = 0;
> > +
> > +     for_each_sec(file, sec) {
> > +             /* absolute references in non-loadable sections are fine */
> > +             if (!(sec->sh.sh_flags & SHF_ALLOC))
> > +                     continue;
> > +
> > +             /* section must have an associated .rela section */
> > +             if (!sec->rsec)
> > +                     continue;
> > +
> > +             /*
> > +              * Special case for compiler generated metadata that is not
> > +              * consumed until after boot.
> > +              */
> > +             if (!strcmp(sec->name, "__patchable_function_entries"))
> > +                     continue;
> > +
> > +             for_each_reloc(sec->rsec, reloc) {
> > +                     if (reloc_type(reloc) == R_ABS64) {
>
> This should probably also check R_ABS32. Yes, your current only user is
> x86_64 so R_ABS64 covers things, but we're getting more and more archs
> using objtool. No reason this check shouldn't also work on PPC32 for
> example.
>

Yeah, I was unsure about this.

This check is sufficient to ensure that PIC code is not emitted with,
e.g., global variables with absolute addresses, etc. So the R_ABS64
check here is only a check whether any relocations of the native
pointer size are present (but no R_ABS_NATIVE exists at this point)

For robustness, we should actually check for all absolute relocations
here, including R_X86_64_32S, which is not abstracted into a R_ABSxx
type for objtool.

So perhaps this needs an arch hook where x86_64 can implement it as

bool arch_is_abs_reloc(reloc)
{
   switch (reloc_type(reloc)) {
   case R_X86_64_32:
   case R_X86_64_32S:
   case R_X86_64_64:
      return true;
   }
   return false;
}

and the default just compares against R_ABS32 / R_ABS64 depending on
the word size?
Re: [PATCH v5 15/22] objtool: Add action to check for absence of absolute relocations
Posted by Peter Zijlstra 1 month, 2 weeks ago
On Wed, Jul 16, 2025 at 08:26:55PM +1000, Ard Biesheuvel wrote:

> For robustness, we should actually check for all absolute relocations
> here, including R_X86_64_32S, which is not abstracted into a R_ABSxx
> type for objtool.
> 
> So perhaps this needs an arch hook where x86_64 can implement it as
> 
> bool arch_is_abs_reloc(reloc)
> {
>    switch (reloc_type(reloc)) {
>    case R_X86_64_32:
>    case R_X86_64_32S:
>    case R_X86_64_64:
>       return true;
>    }
>    return false;
> }
> 
> and the default just compares against R_ABS32 / R_ABS64 depending on
> the word size?

Yes, an arch hook like that makes sense. Perhaps make the signature:

bool arch_is_abs_reloc(struct elf *, struct reloc *);

Because the word size comes from elf_addr_size().
Re: [PATCH v5 15/22] objtool: Add action to check for absence of absolute relocations
Posted by Josh Poimboeuf 1 month, 2 weeks ago
On Wed, Jul 16, 2025 at 01:32:43PM +0200, Peter Zijlstra wrote:
> On Wed, Jul 16, 2025 at 08:26:55PM +1000, Ard Biesheuvel wrote:
> 
> > For robustness, we should actually check for all absolute relocations
> > here, including R_X86_64_32S, which is not abstracted into a R_ABSxx
> > type for objtool.
> > 
> > So perhaps this needs an arch hook where x86_64 can implement it as
> > 
> > bool arch_is_abs_reloc(reloc)
> > {
> >    switch (reloc_type(reloc)) {
> >    case R_X86_64_32:
> >    case R_X86_64_32S:
> >    case R_X86_64_64:
> >       return true;
> >    }
> >    return false;
> > }
> > 
> > and the default just compares against R_ABS32 / R_ABS64 depending on
> > the word size?
> 
> Yes, an arch hook like that makes sense. Perhaps make the signature:
> 
> bool arch_is_abs_reloc(struct elf *, struct reloc *);
> 
> Because the word size comes from elf_addr_size().

We already have an arch_pc_relative_reloc(), please try to keep the
naming consistent.

-- 
Josh