linux-kernel - Re: [PATCH v3 6/7] kbuild: use obj-y instead extra-y for objects placed at the head

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]

Date:   Tue, 25 Oct 2022 12:26:35 +0000 (UTC)
From:   Michael Matz <matz@...e.de>
To:     Jiri Slaby <jirislaby@...nel.org>
cc:     Masahiro Yamada <masahiroy@...nel.org>,
        linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arch@...r.kernel.org,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Nathan Chancellor <nathan@...nel.org>,
        Martin Liška <mliska@...e.cz>,
        Borislav Petkov <bpetkov@...e.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Ard Biesheuvel <ardb@...nel.org>
Subject: Re: [PATCH v3 6/7] kbuild: use obj-y instead extra-y for objects
 placed at the head

Hello,

On Mon, 24 Oct 2022, Jiri Slaby wrote:

> > Create vmlinux.a to collect all the objects that are unconditionally
> > linked to vmlinux. The objects listed in head-y are moved to the head
> > of vmlinux.a by using 'ar m'.
... 
> > --- a/scripts/Makefile.vmlinux_o
> > +++ b/scripts/Makefile.vmlinux_o
> > @@ -18,7 +18,7 @@ quiet_cmd_gen_initcalls_lds = GEN     $@
> >   	$(PERL) $(real-prereqs) > $@
> >     .tmp_initcalls.lds: $(srctree)/scripts/generate_initcall_order.pl \
> > -		$(KBUILD_VMLINUX_OBJS) $(KBUILD_VMLINUX_LIBS) FORCE
> > +		vmlinux.a $(KBUILD_VMLINUX_LIBS) FORCE
> 
> There is a slight problem with this. The kernel built with gcc-LTO does not
> boot. But as I understand it, it's not limited to gcc-LTO only.
> 
> On x86, startup_64() is supposed to be at offset >zero< of the image (see
> .Lrelocated()). It was ensured by putting head64.o to the beginning of vmlinux
> (by KBUILD_VMLINUX_OBJS on the LD command-line above). The patch above instead
> packs head64.o into vmlinux.a and then moves it using "ar -m" to the beginning
> (it's in 7/7 of the series IIRC).
> 
> The problem is that .o files listed on the LD command line explicitly are
> taken as spelled. But unpacking .a inside LD gives no guarantees on the order
> of packed objects. To quote: "that it happens to work sometimes is pure luck."
> (Correct me guys, if I misunderstood you.)

To be precise: I know of no linker (outside LTO-like modes) that processes 
archives in a different order than first-to-last-member (under 
whole-archive), but that's not guaranteed anywhere.  So relying on 
member-order within archives is always brittle.

It will completely break down with LTO modes: the granularity for that is 
functions, and they are placed in some unknown (from the outside, but 
usually related to call-graph locality) order into several partitions, 
with non-LTO-able parts (like asm code) being placed randomly between 
them.  The order of these blobs can not be defined in relation to the 
input order of object files: with cross-file dependencies such order might 
not even exist.  Those whole sequence of blobs then takes the place of the 
input archive (which, as there was only one, has no particular order from 
the linker command lines perspective).

There are only two ways of guaranteeing an ordering: put non-LTO-.o files 
at certain places of the link command, or, better, use a linker script to 
specify an order.

> For x86, the most ideal fix seems to be to fix it in the linker script. By
> putting startup_64() to a different section and handle it in the ld script
> specially -- see the attachment. It should always have been put this way, the
> command line order is only a workaround. But this might need more fixes on
> other archs too -- I haven't take a look.
> 
> Ideas, comments? I'll send the attachment as a PATCH later (if there are 
> no better suggestions).

This will work.  An alternative way would be to explicitely name the input 
file in the section commands, without renaming the section:

@@ -126,6 +126,7 @@ SECTIONS
                _text = .;
                _stext = .;
                /* bootstrapping code */
+               KEEP(vmlinux.a:head64.o(.head.text))
                HEAD_TEXT
                TEXT_TEXT

But I guess not all arch's name their must-be-first file head64.o (or even 
have such requirement), so that's probably still arch-dependend and hence 
not inherently better than your way.

(syntax for the section selector in linkerscripts is:

  {archive-glob:}filename-glob (sectionname-glob)

Ciao,
Michael.