| Message ID | 20241014125703.2287936-4-ardb+git@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Use dot prefixes for section names | expand |
On Mon, 14 Oct 2024 at 05:57, Ard Biesheuvel <ardb+git@google.com> wrote: > > Pre-existing code uses a dot prefix or double underscore to prefix ELF > section names. strip_relocs on x86 relies on this, and other out of tree > tools that mangle vmlinux (kexec or live patching) may rely on this as > well. > > So let's not deviate from this and use a dot prefix for runtime-const > and alloc_tags sections. I'm not following what the actual problem is. Yes, I see that you report that it results in section names like ".relaalloc_tags", but what's the actual _issue_ with that? It seems entirely harmless. In fact, when I was going the runtime sections, I was thinking how convenient it was for the linker to generate the start/stop symbols for us, and that we should perhaps *expand* on that pattern. So this seems a step backwards to me, with no real explanation of what the actual problem is. Yes, we have (two different) pre-existing patterns, but neither pattern seems to be an actual improvement. Linus
On Mon, 14 Oct 2024 at 19:29, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Mon, 14 Oct 2024 at 05:57, Ard Biesheuvel <ardb+git@google.com> wrote: > > > > Pre-existing code uses a dot prefix or double underscore to prefix ELF > > section names. strip_relocs on x86 relies on this, and other out of tree > > tools that mangle vmlinux (kexec or live patching) may rely on this as > > well. > > > > So let's not deviate from this and use a dot prefix for runtime-const > > and alloc_tags sections. > > I'm not following what the actual problem is. Yes, I see that you > report that it results in section names like ".relaalloc_tags", but > what's the actual _issue_ with that? It seems entirely harmless. > > In fact, when I was going the runtime sections, I was thinking how > convenient it was for the linker to generate the start/stop symbols > for us, and that we should perhaps *expand* on that pattern. > > So this seems a step backwards to me, with no real explanation of what > the actual problem is. > > Yes, we have (two different) pre-existing patterns, but neither > pattern seems to be an actual improvement. > We have this code in arch/x86/Makefile.postlink: quiet_cmd_strip_relocs = RSTRIP $@ cmd_strip_relocs = \ $(OBJCOPY) --remove-section='.rel.*' --remove-section='.rel__*' \ --remove-section='.rela.*' --remove-section='.rela__*' $@ Of course, that could easily be fixed, I was just being cautious in case there is other, out-of-tree tooling for live patch or kexec etc that has similar assumptions wrt section names.
On Mon, 14 Oct 2024 at 10:44, Ard Biesheuvel <ardb@kernel.org> wrote: > > We have this code in arch/x86/Makefile.postlink: > > quiet_cmd_strip_relocs = RSTRIP $@ > cmd_strip_relocs = \ > $(OBJCOPY) --remove-section='.rel.*' --remove-section='.rel__*' \ > --remove-section='.rela.*' --remove-section='.rela__*' $@ > > Of course, that could easily be fixed, I was just being cautious in > case there is other, out-of-tree tooling for live patch or kexec etc > that has similar assumptions wrt section names. I'd actually much rather just make strip_relocs not have that "." and "__" pattern at all, and just say "we strip all sections that start with '.rel'". And then we make the rule that we do *not* create sections named ".rel*". That seems like a much simpler rule, and would seem to simplify strip_relocs too, which would just become $(OBJCOPY) --remove-section='.rel*' $@ (We seem to have three different copies of that complex pattern with .rel vs .rela and "." vs "__" - it's in s390, riscv, and x86. So we'd do that simplification in three places) IOW, I'd much rather make our section rules simpler rather than more complex. Of course, if there is some active and acute problem report with this thing, we might not have that option, but in the absence of any *known* issue with just simplifying things, I'd rather do that. I feel that our linker scripts - and linking rules in general - are already quite complicated, which is why I'd really like to take this as a time to try to simplify the rules. Linus Linus
On Mon, 14 Oct 2024 at 20:10, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Mon, 14 Oct 2024 at 10:44, Ard Biesheuvel <ardb@kernel.org> wrote: > > > > We have this code in arch/x86/Makefile.postlink: > > > > quiet_cmd_strip_relocs = RSTRIP $@ > > cmd_strip_relocs = \ > > $(OBJCOPY) --remove-section='.rel.*' --remove-section='.rel__*' \ > > --remove-section='.rela.*' --remove-section='.rela__*' $@ > > > > Of course, that could easily be fixed, I was just being cautious in > > case there is other, out-of-tree tooling for live patch or kexec etc > > that has similar assumptions wrt section names. > > I'd actually much rather just make strip_relocs not have that "." and > "__" pattern at all, and just say "we strip all sections that start > with '.rel'". > > And then we make the rule that we do *not* create sections named ".rel*". > > That seems like a much simpler rule, and would seem to simplify > strip_relocs too, which would just become > > $(OBJCOPY) --remove-section='.rel*' $@ > > (We seem to have three different copies of that complex pattern with > .rel vs .rela and "." vs "__" - it's in s390, riscv, and x86. So we'd > do that simplification in three places) > > IOW, I'd much rather make our section rules simpler rather than more complex. > > Of course, if there is some active and acute problem report with this > thing, we might not have that option, but in the absence of any > *known* issue with just simplifying things, I'd rather do that. > I don't disagree with any of this. CC'ing folks working on live patch in case they have any insights. Full thread here: https://lore.kernel.org/all/20241014125703.2287936-4-ardb+git@google.com/
On Mon 2024-10-14 20:19:32, Ard Biesheuvel wrote: > On Mon, 14 Oct 2024 at 20:10, Linus Torvalds > <torvalds@linux-foundation.org> wrote: > > > > On Mon, 14 Oct 2024 at 10:44, Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > We have this code in arch/x86/Makefile.postlink: > > > > > > quiet_cmd_strip_relocs = RSTRIP $@ > > > cmd_strip_relocs = \ > > > $(OBJCOPY) --remove-section='.rel.*' --remove-section='.rel__*' \ > > > --remove-section='.rela.*' --remove-section='.rela__*' $@ > > > > > > Of course, that could easily be fixed, I was just being cautious in > > > case there is other, out-of-tree tooling for live patch or kexec etc > > > that has similar assumptions wrt section names. > > > > I'd actually much rather just make strip_relocs not have that "." and > > "__" pattern at all, and just say "we strip all sections that start > > with '.rel'". > > > > And then we make the rule that we do *not* create sections named ".rel*". > > > > That seems like a much simpler rule, and would seem to simplify > > strip_relocs too, which would just become > > > > $(OBJCOPY) --remove-section='.rel*' $@ > > > > (We seem to have three different copies of that complex pattern with > > .rel vs .rela and "." vs "__" - it's in s390, riscv, and x86. So we'd > > do that simplification in three places) > > > > IOW, I'd much rather make our section rules simpler rather than more complex. > > > > Of course, if there is some active and acute problem report with this > > thing, we might not have that option, but in the absence of any > > *known* issue with just simplifying things, I'd rather do that. > > > > I don't disagree with any of this. CC'ing folks working on live patch > in case they have any insights. The livepatching specific sections use the name pattern: .klp.rela.objname.section_name They are generated by a post processing of the livepatch module. The code is not upstream at the moment. It is implemented by some out-of-tree tools which are used to build the livepatches. More details can be found in Documentation/livepatch/module-elf-format.rst Best Regards, Petr
From: Ard Biesheuvel <ardb@kernel.org> Pre-existing code uses a dot prefix or double underscore to prefix ELF section names. strip_relocs on x86 relies on this, and other out of tree tools that mangle vmlinux (kexec or live patching) may rely on this as well. So let's not deviate from this and use a dot prefix for runtime-const and alloc_tags sections. Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Masahiro Yamada <masahiroy@kernel.org> Cc: Nathan Chancellor <nathan@kernel.org> Cc: Suren Baghdasaryan <surenb@google.com> Cc: Kent Overstreet <kent.overstreet@linux.dev> Cc: Arnd Bergmann <arnd@arndb.de> Cc: linux-arch@vger.kernel.org Cc: linux-mm@kvack.org Cc: linux-kbuild@vger.kernel.org Ard Biesheuvel (2): codetag: Use dot prefix for section name runtime-const: Use dot prefix for section names arch/arm64/include/asm/runtime-const.h | 4 ++-- arch/s390/include/asm/runtime-const.h | 4 ++-- arch/x86/include/asm/runtime-const.h | 4 ++-- include/asm-generic/codetag.lds.h | 2 +- include/asm-generic/vmlinux.lds.h | 4 ++-- include/linux/alloc_tag.h | 4 ++-- 6 files changed, 11 insertions(+), 11 deletions(-)