lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ