Date:   Tue, 25 Feb 2020 17:56:06 -0800
From:   Fangrui Song <maskray@...gle.com>
To:     Kees Cook <keescook@...omium.org>
Cc:     Nick Desaulniers <ndesaulniers@...gle.com>,
        Arvind Sankar <nivedita@...m.mit.edu>,
        Borislav Petkov <bp@...en8.de>,
        Nathan Chancellor <natechancellor@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        clang-built-linux <clang-built-linux@...glegroups.com>,
        Michael Matz <matz@...e.de>,
        Kristen Carlson Accardi <kristen@...ux.intel.com>
Subject: Re: --orphan-handling=warn

> > Kees is working on a series to just be explicit about what sections
> > are ordered where, and what's discarded, which should better handle
> > incompatibilities between linkers in regards to orphan section
> > placement and "what does `*` mean."  Kees, that series can't come soon
> 
> So, with my series[1] applied, ld.bfd builds clean. With ld.lld, I get a
> TON of warnings, such as:
> 
> (.bss.rel.ro) is being placed in '.bss.rel.ro'

.bss.rel.ro (SHT_NOBITS) is lld specific. GNU ld does not have it. It is
currently used for copy relocations of symbols in read-only PT_LOAD
segments. If a relro section's statically relocated data is all zeros,
we can move the section to .bss.rel.ro

> (.iplt) is being placed in '.iplt'
> (.plt) is being placed in '.plt'
> (.rela.altinstr_aux) is being placed in '.rela.altinstr_aux'
> (.rela.altinstr_replacement) is being placed in
> '.rela.altinstr_replacement'
> (.rela.altinstructions) is being placed in '.rela.altinstructions'
> (.rela.apicdrivers) is being placed in '.rela.apicdrivers'
> (.rela__bug_table) is being placed in '.rela__bug_table'
> (.rela.con_initcall.init) is being placed in '.rela.init.data'
> (.rela.cpuidle.text) is being placed in '.rela.text'
> (.rela.data..cacheline_aligned) is being placed in '.rela.data'
> (.rela.data) is being placed in '.rela.data'
> (.rela.data..percpu) is being placed in '.rela.data..percpu'
> (.rela.data..percpu..page_aligned) is being placed in '.rela.data..percpu'
> ...

I need to figure out the exact GNU ld rule for input SHT_REL[A] retained
by --emit-relocs.

   ld.bfd: warning: orphan section `.rela.meminit.text' from `arch/x86/kernel/head_64.o' being placed in section `.rela.dyn'
   ld.bfd: warning: orphan section `.rela___ksymtab+__ctzsi2' from `arch/x86/kernel/head_64.o' being placed in section `.rela.dyn'
   ld.bfd: warning: orphan section `.rela___ksymtab+__clzsi2' from `arch/x86/kernel/head_64.o' being placed in section `.rela.dyn'
   ld.bfd: warning: orphan section `.rela___ksymtab+__clzdi2' from `arch/x86/kernel/head_64.o' being placed in section `.rela.dyn'
   ld.bfd: warning: orphan section `.rela___ksymtab+__ctzdi2' from `arch/x86/kernel/head_64.o' being placed in section `.rela.dyn'

lld simply ignores such SHT_REL[A] when checking input section descriptions.
A .rela.foo relocating .foo will be named .rela.foobar if .foo is placed in .foobar

It makes sense for --orphan-handling= not to warn/error.
https://reviews.llvm.org/D75151

> But as you can see in the /DISCARD/, these (and all the others), should
> be getting caught:
> 
>         /DISCARD/ : {
>                 *(.eh_frame)
> +               *(.rela.*) *(.rela_*)
> +               *(.rel.*) *(.rel_*)
> +               *(.got) *(.got.*)
> +               *(.igot.*) *(.iplt)
>         }
> 
> I don't understand what's happening here. I haven't minimized this case
> nor opened an lld bug yet.

--orphan-handling was implemented per
https://bugs.llvm.org/show_bug.cgi?id=34946
It seems the reporter did not follow up after the feature was implemented.
Now we have the Linux kernel case...
Last December I encountered another case in my company.

It is pretty clear that this feature is useful and we should fix it :)

https://reviews.llvm.org/D75149

> enough. ;) (I think it's intended to help "fine grain" (per function)
> KASLR).  More comments in the other thread.

> Actually, it's rather opposed to the FGKASLR series, as for that, I need
> some kind of linker script directive like this:
> 
> 	/PASSTHRU/ : {
> 		*(.text.*)
> 	}
> 
> Where "PASSTHRU" would create a 1-to-1 input-section to output-section
> with the same name, flags, etc.

/PASSTHRU/ sections are still handled as orphan sections?
Do you restrict { } to input section descriptions, not output section
data (https://sourceware.org/binutils/docs/ld/Output-Section-Data.html#Output-Section-Data)?
or symbol assignments?

You can ask https://sourceware.org/ml/binutils/2020-02/ whether
they'd like to accept the feature request:)

(My personal feeling is that I want to see more use cases to add the new
feature...)

> ld.bfd's handling of orphan sections named .text.* is to put them each
> as a separate output section, after the existing .text output section.
> 
> ld.lld's handling of orphan sections named .text.* is to put them into
> the .text output section.

Confirmed. lld can adapt. I need to do some homework...

> For FGKASLR (as it is currently implemented[2]), the sections need to be
> individually named output sections (as bfd does it). *However*, with the
> "warn on orphans" patch, FGKASLR's intentional orphaning will backfire
> (I guess the warning could be turned off, but I'd like lld to handle
> FGKASLR at some point.)
> 
> Note that cheating and doing the 1-to-1 mapping by handy with a 40,000
> entry linker script ... made ld.lld take about 15 minutes to do the
> final link. :(

Placing N orphan sections requires O(N^2) time (in both GNU ld and lld) :(

> > Taken from the Zen of Python, but in regards to sections in linker
> > scripts, "explicit is better than implicit."
> 
> Totally agreed. I just hope there's a good solution for this PASSTHRU
> idea...
> 
> -Kees
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=linker/orphans/x86-arm
> [2]
> https://github.com/kaccardi/linux/commit/127111e8c6170a130d8d12d73728e74acbe05e13

On 2020-02-25, Kees Cook wrote:
>On Tue, Feb 25, 2020 at 12:37:26PM -0800, Nick Desaulniers wrote:
>> On Tue, Feb 25, 2020 at 11:43 AM Kees Cook <keescook@...omium.org> wrote:
>> >
>> > On Tue, Feb 25, 2020 at 01:29:51PM -0500, Arvind Sankar wrote:
>> > > On Mon, Feb 24, 2020 at 09:35:04PM -0800, Kees Cook wrote:
>> > > > Note that cheating and doing the 1-to-1 mapping by handy with a 40,000
>> > > > entry linker script ... made ld.lld take about 15 minutes to do the
>> > > > final link. :(
>> > >
>> > > Out of curiosity, how long does ld.bfd take on that linker script :)
>> >
>> > A single CPU at 100% for 15 minutes. :)
>>
>> I can see the implementers of linker script handling thinking "surely
>> no one would ever have >10k entries." Then we invented things like
>> -ffunction-sections, -fdata-sections, (per basic block equivalents:
>> https://reviews.llvm.org/D68049) and then finally FGKASLR. "640k ought
>> to be enough for anybody" and such.
>
>Heh, yeah. I had no expectation that it would work _well_; I just
>wanted to test if it _could_ work. And it did: FGKASLR up and running
>on Clang+LLD. I stopped there before attempting the next step:
>FGKASLR+LTO+CFI, which I assume would be hilariously slow linking.

Now I learned the term FGKASLR... I need to do some homework.

Powered by blists - more mailing lists