* [PATCH] kbuild: Merge module sections if and only if CONFIG_LTO_CLANG is enabled
@ 2021-03-22 23:44 Sean Christopherson
2021-03-23 16:19 ` Sami Tolvanen
2021-03-31 19:56 ` Kees Cook
0 siblings, 2 replies; 9+ messages in thread
From: Sean Christopherson @ 2021-03-22 23:44 UTC (permalink / raw)
To: Masahiro Yamada, Michal Marek, Nathan Chancellor, Nick Desaulniers
Cc: linux-kbuild, linux-kernel, clang-built-linux, Sami Tolvanen,
Kees Cook, Sean Christopherson
Merge module sections only when using Clang LTO. With gcc-10, merging
sections does not appear to update the symbol tables for the module,
e.g. 'readelf -s' shows the value that a symbol would have had, if
sections were not merged.
The stale symbol table breaks gdb's function disassambler, and presumably
other things, e.g.
gdb -batch -ex "file arch/x86/kvm/kvm.ko" -ex "disassemble kvm_init"
reads the wrong bytes and dumps garbage.
Fixes: dd2776222abb ("kbuild: lto: merge module sections")
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
This is obviously the quick and dirty approach to fixing the problem,
presumably there is a way to actually update the symbols, but that is far,
far outside my area of expertise.
IIUC how the disassemblers work, the section headers are correctly updated,
e.g. objdump displays the correct info, and even gdb's disassembler shows
the "correct" offset, it's just the symbol tables that are out of whack.
An earlier version of the LTO series did have exactly this #ifdef, but
it was dropped when no one objected to Kees' suggestion to unconditionally
merge sections. https://lkml.kernel.org/r/202010141548.47CB1BC@keescook
scripts/module.lds.S | 2 ++
1 file changed, 2 insertions(+)
diff --git a/scripts/module.lds.S b/scripts/module.lds.S
index 168cd27e6122..3580c6d02957 100644
--- a/scripts/module.lds.S
+++ b/scripts/module.lds.S
@@ -25,6 +25,7 @@ SECTIONS {
* -ffunction-sections, which increases the size of the final module.
* Merge the split sections in the final binary.
*/
+#ifdef CONFIG_LTO_CLANG
.bss : {
*(.bss .bss.[0-9a-zA-Z_]*)
*(.bss..L*)
@@ -41,6 +42,7 @@ SECTIONS {
}
.text : { *(.text .text.[0-9a-zA-Z_]*) }
+#endif
}
/* bring in arch-specific sections */
--
2.31.0.rc2.261.g7f71774620-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] kbuild: Merge module sections if and only if CONFIG_LTO_CLANG is enabled
2021-03-22 23:44 [PATCH] kbuild: Merge module sections if and only if CONFIG_LTO_CLANG is enabled Sean Christopherson
@ 2021-03-23 16:19 ` Sami Tolvanen
2021-03-23 16:36 ` Sean Christopherson
2021-03-31 19:56 ` Kees Cook
1 sibling, 1 reply; 9+ messages in thread
From: Sami Tolvanen @ 2021-03-23 16:19 UTC (permalink / raw)
To: Sean Christopherson
Cc: Masahiro Yamada, Michal Marek, Nathan Chancellor,
Nick Desaulniers, linux-kbuild, LKML, clang-built-linux,
Kees Cook
On Mon, Mar 22, 2021 at 4:44 PM Sean Christopherson <seanjc@google.com> wrote:
>
> Merge module sections only when using Clang LTO. With gcc-10, merging
> sections does not appear to update the symbol tables for the module,
> e.g. 'readelf -s' shows the value that a symbol would have had, if
> sections were not merged.
I'm fine with limiting this to LTO only, but it would be helpful to
understand which sections are actually getting merged here. Are you
compiling the kernel with -ffunction-sections and/or -fdata-sections?
Does this issue only happen with gcc 10?
Sami
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] kbuild: Merge module sections if and only if CONFIG_LTO_CLANG is enabled
2021-03-23 16:19 ` Sami Tolvanen
@ 2021-03-23 16:36 ` Sean Christopherson
2021-03-23 18:14 ` Sami Tolvanen
0 siblings, 1 reply; 9+ messages in thread
From: Sean Christopherson @ 2021-03-23 16:36 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Masahiro Yamada, Michal Marek, Nathan Chancellor,
Nick Desaulniers, linux-kbuild, LKML, clang-built-linux,
Kees Cook
On Tue, Mar 23, 2021, Sami Tolvanen wrote:
> On Mon, Mar 22, 2021 at 4:44 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > Merge module sections only when using Clang LTO. With gcc-10, merging
> > sections does not appear to update the symbol tables for the module,
> > e.g. 'readelf -s' shows the value that a symbol would have had, if
> > sections were not merged.
>
> I'm fine with limiting this to LTO only, but it would be helpful to
> understand which sections are actually getting merged here.
It doesn't appear to matter which sections get merged, the tables only show the
correct data if there is no merging whatsoever, e.g. allowing merging for any
one of the four types (.bss, .data, .rodata and .text) results in breakage.
AFAICT, merging any sections causes the layout to change and throw off the
symbol tables.
> Are you compiling the kernel with -ffunction-sections and/or -fdata-sections?
I tried both. Default off, and forcing those flags by hacking the Makefile had
no effect.
> Does this issue only happen with gcc 10?
gcc-7 shows the same behavior, I haven't checked anything older or anything in
between.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] kbuild: Merge module sections if and only if CONFIG_LTO_CLANG is enabled
2021-03-23 16:36 ` Sean Christopherson
@ 2021-03-23 18:14 ` Sami Tolvanen
2021-03-24 22:45 ` Sean Christopherson
0 siblings, 1 reply; 9+ messages in thread
From: Sami Tolvanen @ 2021-03-23 18:14 UTC (permalink / raw)
To: Sean Christopherson
Cc: Masahiro Yamada, Michal Marek, Nathan Chancellor,
Nick Desaulniers, linux-kbuild, LKML, clang-built-linux,
Kees Cook
On Tue, Mar 23, 2021 at 9:36 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Tue, Mar 23, 2021, Sami Tolvanen wrote:
> > On Mon, Mar 22, 2021 at 4:44 PM Sean Christopherson <seanjc@google.com> wrote:
> > >
> > > Merge module sections only when using Clang LTO. With gcc-10, merging
> > > sections does not appear to update the symbol tables for the module,
> > > e.g. 'readelf -s' shows the value that a symbol would have had, if
> > > sections were not merged.
> >
> > I'm fine with limiting this to LTO only, but it would be helpful to
> > understand which sections are actually getting merged here.
>
> It doesn't appear to matter which sections get merged, the tables only show the
> correct data if there is no merging whatsoever, e.g. allowing merging for any
> one of the four types (.bss, .data, .rodata and .text) results in breakage.
> AFAICT, merging any sections causes the layout to change and throw off the
> symbol tables.
Thanks for the clarification. I can reproduce this issue with gcc +
bfd if any of the sections are merged, but gcc + lld produces valid
symbol tables.
Perhaps someone more familiar with bfd can comment on whether this is
a bug or a feature, and if there's a flag we can pass to bfd that
would fix the issue. In the meanwhile, this patch looks like a
reasonable workaround to me.
Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
Tested-by: Sami Tolvanen <samitolvanen@google.com>
Sami
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] kbuild: Merge module sections if and only if CONFIG_LTO_CLANG is enabled
2021-03-23 18:14 ` Sami Tolvanen
@ 2021-03-24 22:45 ` Sean Christopherson
2021-03-31 19:30 ` Kees Cook
0 siblings, 1 reply; 9+ messages in thread
From: Sean Christopherson @ 2021-03-24 22:45 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Masahiro Yamada, Michal Marek, Nathan Chancellor,
Nick Desaulniers, linux-kbuild, LKML, clang-built-linux,
Kees Cook
On Tue, Mar 23, 2021, Sami Tolvanen wrote:
> On Tue, Mar 23, 2021 at 9:36 AM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Tue, Mar 23, 2021, Sami Tolvanen wrote:
> > > On Mon, Mar 22, 2021 at 4:44 PM Sean Christopherson <seanjc@google.com> wrote:
> > > >
> > > > Merge module sections only when using Clang LTO. With gcc-10, merging
> > > > sections does not appear to update the symbol tables for the module,
> > > > e.g. 'readelf -s' shows the value that a symbol would have had, if
> > > > sections were not merged.
> > >
> > > I'm fine with limiting this to LTO only, but it would be helpful to
> > > understand which sections are actually getting merged here.
> >
> > It doesn't appear to matter which sections get merged, the tables only show the
> > correct data if there is no merging whatsoever, e.g. allowing merging for any
> > one of the four types (.bss, .data, .rodata and .text) results in breakage.
> > AFAICT, merging any sections causes the layout to change and throw off the
> > symbol tables.
>
> Thanks for the clarification. I can reproduce this issue with gcc +
> bfd if any of the sections are merged, but gcc + lld produces valid
> symbol tables.
FWIW, clang + bfd also produces mangled tables, so it does appear to be bfd
specific.
> Perhaps someone more familiar with bfd can comment on whether this is
> a bug or a feature, and if there's a flag we can pass to bfd that
> would fix the issue. In the meanwhile, this patch looks like a
> reasonable workaround to me.
>
> Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
> Tested-by: Sami Tolvanen <samitolvanen@google.com>
>
> Sami
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] kbuild: Merge module sections if and only if CONFIG_LTO_CLANG is enabled
2021-03-24 22:45 ` Sean Christopherson
@ 2021-03-31 19:30 ` Kees Cook
2021-03-31 20:07 ` Sean Christopherson
0 siblings, 1 reply; 9+ messages in thread
From: Kees Cook @ 2021-03-31 19:30 UTC (permalink / raw)
To: Sean Christopherson
Cc: Sami Tolvanen, Fangrui Song, Masahiro Yamada, Michal Marek,
Nathan Chancellor, Nick Desaulniers, linux-kbuild, LKML,
clang-built-linux
On Wed, Mar 24, 2021 at 10:45:36PM +0000, Sean Christopherson wrote:
> On Tue, Mar 23, 2021, Sami Tolvanen wrote:
> > On Tue, Mar 23, 2021 at 9:36 AM Sean Christopherson <seanjc@google.com> wrote:
> > >
> > > On Tue, Mar 23, 2021, Sami Tolvanen wrote:
> > > > On Mon, Mar 22, 2021 at 4:44 PM Sean Christopherson <seanjc@google.com> wrote:
> > > > >
> > > > > Merge module sections only when using Clang LTO. With gcc-10, merging
> > > > > sections does not appear to update the symbol tables for the module,
> > > > > e.g. 'readelf -s' shows the value that a symbol would have had, if
> > > > > sections were not merged.
> > > >
> > > > I'm fine with limiting this to LTO only, but it would be helpful to
> > > > understand which sections are actually getting merged here.
> > >
> > > It doesn't appear to matter which sections get merged, the tables only show the
> > > correct data if there is no merging whatsoever, e.g. allowing merging for any
> > > one of the four types (.bss, .data, .rodata and .text) results in breakage.
> > > AFAICT, merging any sections causes the layout to change and throw off the
> > > symbol tables.
> >
> > Thanks for the clarification. I can reproduce this issue with gcc +
> > bfd if any of the sections are merged, but gcc + lld produces valid
> > symbol tables.
>
> FWIW, clang + bfd also produces mangled tables, so it does appear to be bfd
> specific.
Are you able to open a bug against bfd for this?
> > Perhaps someone more familiar with bfd can comment on whether this is
> > a bug or a feature, and if there's a flag we can pass to bfd that
> > would fix the issue. In the meanwhile, this patch looks like a
> > reasonable workaround to me.
> >
> > Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
> > Tested-by: Sami Tolvanen <samitolvanen@google.com>
Thanks, I'll get this sent to Linus.
--
Kees Cook
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] kbuild: Merge module sections if and only if CONFIG_LTO_CLANG is enabled
2021-03-31 19:30 ` Kees Cook
@ 2021-03-31 20:07 ` Sean Christopherson
2021-04-01 21:33 ` Kees Cook
0 siblings, 1 reply; 9+ messages in thread
From: Sean Christopherson @ 2021-03-31 20:07 UTC (permalink / raw)
To: Kees Cook
Cc: Sami Tolvanen, Fangrui Song, Masahiro Yamada, Michal Marek,
Nathan Chancellor, Nick Desaulniers, linux-kbuild, LKML,
clang-built-linux
On Wed, Mar 31, 2021, Kees Cook wrote:
> On Wed, Mar 24, 2021 at 10:45:36PM +0000, Sean Christopherson wrote:
> > On Tue, Mar 23, 2021, Sami Tolvanen wrote:
> > > On Tue, Mar 23, 2021 at 9:36 AM Sean Christopherson <seanjc@google.com> wrote:
> > > >
> > > > On Tue, Mar 23, 2021, Sami Tolvanen wrote:
> > > > > On Mon, Mar 22, 2021 at 4:44 PM Sean Christopherson <seanjc@google.com> wrote:
> > > > > >
> > > > > > Merge module sections only when using Clang LTO. With gcc-10, merging
> > > > > > sections does not appear to update the symbol tables for the module,
> > > > > > e.g. 'readelf -s' shows the value that a symbol would have had, if
> > > > > > sections were not merged.
> > > > >
> > > > > I'm fine with limiting this to LTO only, but it would be helpful to
> > > > > understand which sections are actually getting merged here.
> > > >
> > > > It doesn't appear to matter which sections get merged, the tables only show the
> > > > correct data if there is no merging whatsoever, e.g. allowing merging for any
> > > > one of the four types (.bss, .data, .rodata and .text) results in breakage.
> > > > AFAICT, merging any sections causes the layout to change and throw off the
> > > > symbol tables.
> > >
> > > Thanks for the clarification. I can reproduce this issue with gcc +
> > > bfd if any of the sections are merged, but gcc + lld produces valid
> > > symbol tables.
> >
> > FWIW, clang + bfd also produces mangled tables, so it does appear to be bfd
> > specific.
>
> Are you able to open a bug against bfd for this?
Yes? I'll ping you when I run into trouble ;-)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] kbuild: Merge module sections if and only if CONFIG_LTO_CLANG is enabled
2021-03-31 20:07 ` Sean Christopherson
@ 2021-04-01 21:33 ` Kees Cook
0 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2021-04-01 21:33 UTC (permalink / raw)
To: Sean Christopherson
Cc: Sami Tolvanen, Fangrui Song, Masahiro Yamada, Michal Marek,
Nathan Chancellor, Nick Desaulniers, linux-kbuild, LKML,
clang-built-linux
On Wed, Mar 31, 2021 at 08:07:18PM +0000, Sean Christopherson wrote:
> On Wed, Mar 31, 2021, Kees Cook wrote:
> > On Wed, Mar 24, 2021 at 10:45:36PM +0000, Sean Christopherson wrote:
> > > On Tue, Mar 23, 2021, Sami Tolvanen wrote:
> > > > On Tue, Mar 23, 2021 at 9:36 AM Sean Christopherson <seanjc@google.com> wrote:
> > > > >
> > > > > On Tue, Mar 23, 2021, Sami Tolvanen wrote:
> > > > > > On Mon, Mar 22, 2021 at 4:44 PM Sean Christopherson <seanjc@google.com> wrote:
> > > > > > >
> > > > > > > Merge module sections only when using Clang LTO. With gcc-10, merging
> > > > > > > sections does not appear to update the symbol tables for the module,
> > > > > > > e.g. 'readelf -s' shows the value that a symbol would have had, if
> > > > > > > sections were not merged.
> > > > > >
> > > > > > I'm fine with limiting this to LTO only, but it would be helpful to
> > > > > > understand which sections are actually getting merged here.
> > > > >
> > > > > It doesn't appear to matter which sections get merged, the tables only show the
> > > > > correct data if there is no merging whatsoever, e.g. allowing merging for any
> > > > > one of the four types (.bss, .data, .rodata and .text) results in breakage.
> > > > > AFAICT, merging any sections causes the layout to change and throw off the
> > > > > symbol tables.
> > > >
> > > > Thanks for the clarification. I can reproduce this issue with gcc +
> > > > bfd if any of the sections are merged, but gcc + lld produces valid
> > > > symbol tables.
> > >
> > > FWIW, clang + bfd also produces mangled tables, so it does appear to be bfd
> > > specific.
> >
> > Are you able to open a bug against bfd for this?
>
> Yes? I'll ping you when I run into trouble ;-)
Hm, I can't tell if any of these are duplicates:
https://sourceware.org/bugzilla/buglist.cgi?bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=SUSPENDED&bug_status=WAITING&bug_status=REOPENED&component=ld&list_id=59962&product=binutils&query_format=advanced&short_desc=symbol&short_desc_type=allwordssubstr
--
Kees Cook
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] kbuild: Merge module sections if and only if CONFIG_LTO_CLANG is enabled
2021-03-22 23:44 [PATCH] kbuild: Merge module sections if and only if CONFIG_LTO_CLANG is enabled Sean Christopherson
2021-03-23 16:19 ` Sami Tolvanen
@ 2021-03-31 19:56 ` Kees Cook
1 sibling, 0 replies; 9+ messages in thread
From: Kees Cook @ 2021-03-31 19:56 UTC (permalink / raw)
To: Sean Christopherson, Michal Marek, Masahiro Yamada,
Nathan Chancellor, Nick Desaulniers
Cc: Kees Cook, Sami Tolvanen, clang-built-linux, linux-kbuild, linux-kernel
On Mon, 22 Mar 2021 16:44:38 -0700, Sean Christopherson wrote:
> Merge module sections only when using Clang LTO. With gcc-10, merging
> sections does not appear to update the symbol tables for the module,
> e.g. 'readelf -s' shows the value that a symbol would have had, if
> sections were not merged.
>
> The stale symbol table breaks gdb's function disassambler, and presumably
> other things, e.g.
>
> [...]
Applied to for-linus/lto, thanks!
[1/1] kbuild: lto: Merge module sections if and only if CONFIG_LTO_CLANG is enabled
https://git.kernel.org/kees/c/8b382ebc86a8
--
Kees Cook
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-04-01 21:33 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-22 23:44 [PATCH] kbuild: Merge module sections if and only if CONFIG_LTO_CLANG is enabled Sean Christopherson
2021-03-23 16:19 ` Sami Tolvanen
2021-03-23 16:36 ` Sean Christopherson
2021-03-23 18:14 ` Sami Tolvanen
2021-03-24 22:45 ` Sean Christopherson
2021-03-31 19:30 ` Kees Cook
2021-03-31 20:07 ` Sean Christopherson
2021-04-01 21:33 ` Kees Cook
2021-03-31 19:56 ` Kees Cook
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).