linux-kernel - Re: [PATCH] vmlinux.lds.h: Coalesce transient LLVM dead code elimination sections

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: <7304fdf3-23d7-442b-b870-e88ae6f37004@localhost>
Date:   Thu, 02 Jul 2020 15:01:25 -0700
From:   "Danny Lin" <danny@...ag0n.dev>
To:     "Nathan Chancellor" <natechancellor@...il.com>
Cc:     "Kees Cook" <keescook@...omium.org>,
        "Arnd Bergmann" <arnd@...db.de>,
        "Nick Desaulniers" <ndesaulniers@...gle.com>,
        "Sami Tolvanen" <samitolvanen@...gle.com>,
        "Fangrui Song" <maskray@...gle.com>, linux-arch@...r.kernel.org,
        linux-kernel@...r.kernel.org, clang-built-linux@...glegroups.com,
        stable@...r.kernel.org
Subject: Re: [PATCH] vmlinux.lds.h: Coalesce transient LLVM dead code elimination sections

Jul 2, 2020 9:04:25 AM Nathan Chancellor <natechancellor@...il.com>:

> On Thu, Jul 02, 2020 at 08:54:53AM -0700, Kees Cook wrote:
>> On Thu, Jul 02, 2020 at 01:54:00AM -0700, Danny Lin wrote:
>>> A recent LLVM 11 commit [1] made LLD stop implicitly coalescing some
>>> temporary LLVM sections, namely .{data,bss}..compoundliteral.XXX:
>>>
>>> [30] .data..compoundli PROGBITS         ffffffff9ac9a000  19e9a000
>>> 000000000000cea0  0000000000000000  WA       0     0     32
>>> [31] .rela.data..compo RELA             0000000000000000  40965440
>>> 0000000000001d88  0000000000000018   I      2238    30     8
>>> [32] .data..compoundli PROGBITS         ffffffff9aca6ea0  19ea6ea0
>>> 00000000000033c0  0000000000000000  WA       0     0     32
>>> [33] .rela.data..compo RELA             0000000000000000  409671c8
>>> 0000000000000948  0000000000000018   I      2238    32     8
>>> [...]
>>> [2213] .bss..compoundlit NOBITS           ffffffffa3000000  1d85c000
>>> 00000000000000a0  0000000000000000  WA       0     0     32
>>> [2214] .bss..compoundlit NOBITS           ffffffffa30000a0  1d85c000
>>> 0000000000000040  0000000000000000  WA       0     0     32
>>> [...]
>>>
>>> While these extra sections don't typically cause any breakage, they do
>>> inflate the vmlinux size due to the overhead of storing metadata for
>>> thousands of extra sections.
>>>
>>> It's also worth noting that for some reason, some downstream Android
>>> kernels can't boot at all if these sections aren't coalesced.
>>>
>>> This issue isn't limited to any specific architecture; it affects arm64
>>> and x86 if CONFIG_LD_DEAD_CODE_DATA_ELIMINATION is forced on.
>
> It might be worth noting that this happens explicitly because of
> -fdata-sections, which is currently only used with
> CONFIG_LD_DEAD_CODE_DATA_ELIMINATION but there are other features such
> as LTO that will enable this and make this relevant in the future.
>
> https://android-review.googlesource.com/c/kernel/common/+/1329278/6#message-81b191e92ef4e98e017fa9ded5ef63ef6e60db3a
>
> It is also worth noting that those commits add .bss..L* and .data..L*
> and rodata variants. Do you know if those are relevant here?

As far as I can tell, those sections are exclusive to LTO which isn't in
mainline yet. I don't see any sections like that in my DCE-only vmlinux.

>
>>> Example on x86 allyesconfig:
>>> Before: 2241 sections, 1170972 KiB
>>> After:    56 sections, 1171169 KiB
>
> Am I reading this right that coalescing those sections increases the
> image size? Kind of interesting.

Oops, I accidentally swapped the numbers in the commit message.
Coalescing the sections makes the image smaller as expected.

>
>>> [1] https://github.com/llvm/llvm-project/commit/9e33c096476ab5e02ab1c8442cc3cb4e32e29f17
>>>
>>> Link: https://github.com/ClangBuiltLinux/linux/issues/958
>>> Cc: stable@...r.kernel.org # v4.4+
>>> Suggested-by: Fangrui Song <maskray@...gle.com>
>>> Signed-off-by: Danny Lin <danny@...ag0n.dev>
>
> Reviewed-by: Nathan Chancellor <natechancellor@...il.com>
>
>>> ---
>>> include/asm-generic/vmlinux.lds.h | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
>>> index db600ef218d7..18968cba87c7 100644
>>> --- a/include/asm-generic/vmlinux.lds.h
>>> +++ b/include/asm-generic/vmlinux.lds.h
>>> @@ -94,10 +94,10 @@
>>> */
>>> #ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
>>> #define TEXT_MAIN .text .text.[0-9a-zA-Z_]*
>>> -#define DATA_MAIN .data .data.[0-9a-zA-Z_]* .data..LPBX*
>>> +#define DATA_MAIN .data .data.[0-9a-zA-Z_]* .data..LPBX* .data..compoundliteral*
>
> I am fairly certain this will fix a PowerPC warning that we had
> recently so good!
>
> https://lore.kernel.org/lkml/202006180904.TVUXCf6H%25lkp@intel.com/
>
> Unfortunately, I forgot to reply to that thread...
>
>>> #define SDATA_MAIN .sdata .sdata.[0-9a-zA-Z_]*
>>> #define RODATA_MAIN .rodata .rodata.[0-9a-zA-Z_]*
>>> -#define BSS_MAIN .bss .bss.[0-9a-zA-Z_]*
>>> +#define BSS_MAIN .bss .bss.[0-9a-zA-Z_]* .bss..compoundliteral*
>>
>> Are there .data.. and .bss.. sections we do NOT want to collect? i.e.
>> why not include .data..* and .bss..* ?
>
> At one point Android was doing that for modules but stopped:
>
> https://android-review.googlesource.com/c/kernel/common/+/1266787
>
> I wonder if that is a problem for the main kernel image.

A comment above the code in question explicitly states that not all
.data..* sections should be coalesced. There's a .data..percpu section
in my x86 vmlinux which should probably remain separate.

>
> Cheers,
> Nathan
>