[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z5dd5TSUuTT_qNrf@gmail.com>
Date: Mon, 27 Jan 2025 11:20:21 +0100
From: Ingo Molnar <mingo@...nel.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Ard Biesheuvel <ardb@...nel.org>, linux-kernel@...r.kernel.org,
the arch/x86 maintainers <x86@...nel.org>,
David Woodhouse <dwmw2@...radead.org>,
Peter Zijlstra <peterz@...radead.org>,
"H. Peter Anvin" <hpa@...or.com>
Subject: [PATCH] Revert "x86/boot: Reject absolute references in .head.text
* Linus Torvalds <torvalds@...ux-foundation.org> wrote:
> On Tue, 21 Jan 2025 at 13:29, Ingo Molnar <mingo@...nel.org> wrote:
> >
> > - A series to remove the last remaining absolute symbol references from
> > .head.text, and enforce this at build time, by Ard Biesheuvel:
> > [...]
> > - Which build-time enforcement uncovered a handful of bugs of essentially
> > non-working code, and a wrokaround for a toolchain bug, fixed by
> > Ard Biesheuvel as well:
> >
> > - Fix spurious undefined reference when CONFIG_X86_5LEVEL=n, on GCC-12
> > - Disable UBSAN on SEV code that may execute very early
> > - Disable ftrace branch profiling in SEV startup code
>
> Bah. I only noticed this today, because I was on the road part of the
> week and didn't do my usual "build with clang".
>
> But this is broken with my normal clang config, and I get a very
> unhelpful error message:
>
> Absolute reference to symbol '.rodata' not permitted in .head.text
>
> and I have no idea why, or where it comes from. And that error
> message doesn't specify enough information for me to even *guess* at
> what's going on.
>
> And bisecting it obviously just points at faf0ed487415 ("x86/boot:
> Reject absolute references in .head.text"), since it's just a random
> new check for an old issue.
>
> New random rule, with new random error as a result, and totally opaque
> to anybody else than Ard.
>
> Useless crap, in other words.
>
> Why isn't the fix a revert? Because that error message is really bad.
> It needs to tell me where things went wrong, not just a "You're effed.
> Ha! Ha!".
>
> And to add insult to injury, all of this is done in-place on the
> vmlinux file, so when it all fails, make does
>
> make[2]: *** Deleting file 'vmlinux'
>
> and doesn't even leave behind anything to look at.
Sorry about that - I was really hoping we'd never see this arguably
maximally passive-aggressive message and build failure, as the patch
has lived in -next for 1.5 months, but here we go ...
> Anyway, that check needs to either
>
> (a) die a painful death very quickly
>
> (b) be made to actually print out useful information of WHERE the
> relocation comes from and WHERE it points to
I'd go for (a)+(b): I've queued up a revert which I'll send to you in a
few hours, so (b) can be done properly. Also feel free to queue up the
revert directly:
Acked-by: Ingo Molnar <mingo@...nel.org>
I'd even argue that (b) should first be a non-fatal and 100% helpful
message in a standard build warning format so that CI bots pick it up,
and then in a kernel release or two we can do (c) that does the hard
build failure ...
Thanks,
Ingo
===========================>
From: Ingo Molnar <mingo@...nel.org>
Date: Mon, 27 Jan 2025 11:08:14 +0100
Subject: [PATCH] Revert "x86/boot: Reject absolute references in .head.text"
This reverts commit faf0ed487415f76fe4acf7980ce360901f5e1698.
As Linus reported, the hard build failure is entirely unhelpful
in tracking down the bug:
Absolute reference to symbol '.rodata' not permitted in .head.text
... and to add insult to injury, the offending vmlinux gets deleted,
making it hard to figure out what's going on ...
So revert this until a (much) more developer-friendly version
is merged.
Reported-by: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Ard Biesheuvel <ardb@...nel.org>
Signed-off-by: Ingo Molnar <mingo@...nel.org>
Link: https://lore.kernel.org/r/CAHk-=wj7k9nvJn6cpa3-5Ciwn2RGyE605BMkjWE4MqnvC9E92A@mail.gmail.com
---
arch/x86/tools/relocs.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index e937be979ec8..27441e5863b2 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -841,10 +841,10 @@ static int is_percpu_sym(ElfW(Sym) *sym, const char *symname)
static int do_reloc64(struct section *sec, Elf_Rel *rel, ElfW(Sym) *sym,
const char *symname)
{
- int headtext = !strcmp(sec_name(sec->shdr.sh_info), ".head.text");
unsigned r_type = ELF64_R_TYPE(rel->r_info);
ElfW(Addr) offset = rel->r_offset;
int shn_abs = (sym->st_shndx == SHN_ABS) && !is_reloc(S_REL, symname);
+
if (sym->st_shndx == SHN_UNDEF)
return 0;
@@ -900,12 +900,6 @@ static int do_reloc64(struct section *sec, Elf_Rel *rel, ElfW(Sym) *sym,
break;
}
- if (headtext) {
- die("Absolute reference to symbol '%s' not permitted in .head.text\n",
- symname);
- break;
- }
-
/*
* Relocation offsets for 64 bit kernels are output
* as 32 bits and sign extended back to 64 bits when
Powered by blists - more mailing lists