diff mbox series

[v2] scripts: Do not strip .rela.dyn section

Message ID 20250403134200.385077-1-alexghiti@rivosinc.com (mailing list archive)
State Superseded
Headers show
Series [v2] scripts: Do not strip .rela.dyn section | expand

Checks

Context Check Description
bjorn/pre-ci_am success Success
bjorn/build-rv32-defconfig success build-rv32-defconfig
bjorn/build-rv64-clang-allmodconfig success build-rv64-clang-allmodconfig
bjorn/build-rv64-gcc-allmodconfig success build-rv64-gcc-allmodconfig
bjorn/build-rv64-nommu-k210-defconfig success build-rv64-nommu-k210-defconfig
bjorn/build-rv64-nommu-k210-virt success build-rv64-nommu-k210-virt
bjorn/checkpatch success checkpatch
bjorn/dtb-warn-rv64 success dtb-warn-rv64
bjorn/header-inline success header-inline
bjorn/kdoc success kdoc
bjorn/module-param success module-param
bjorn/verify-fixes success verify-fixes
bjorn/verify-signedoff success verify-signedoff

Commit Message

Alexandre Ghiti April 3, 2025, 1:42 p.m. UTC
riscv uses the .rela.dyn section to relocate the kernel at runtime but
that section is stripped from vmlinux. That prevents kexec to
successfully load vmlinux since it does not contain the relocations info
needed.

Fixes: 71d815bf5dfd ("kbuild: Strip runtime const RELA sections correctly")
Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
---
 scripts/Makefile.lib | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ard Biesheuvel April 3, 2025, 3:11 p.m. UTC | #1
On Thu, 3 Apr 2025 at 16:42, Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
>
> riscv uses the .rela.dyn section to relocate the kernel at runtime but
> that section is stripped from vmlinux. That prevents kexec to
> successfully load vmlinux since it does not contain the relocations info
> needed.
>

Maybe explain that .rela.dyn contains runtime relocations, which are
only emitted if they are actually needed - as opposed to the static
relocations that are not emitted as SHF_ALLOC sections, and are not
considered to be part of the runtime image in the first place. It
would be nice if we could use --remove-relocations= here, which only
removes static relocations, but unfortunately, llvm-objcopy does not
support this.

Also, I wonder if this should apply to all of .rel.dyn, .rela.dyn and
.relr.dyn, as they all carry runtime relocations.

Finally, I'd be curious to know why RISC-V relies on --emit-relocs in
the first place? Is the relocs check really needed? If not, it would
be a nice opportunity to get rid of Makefile.postlink entirely.

In any case, for this change, or a variation along the lines of what I
wrote above,

Acked-by: Ard Biesheuvel <ardb@kernel.org>


> Fixes: 71d815bf5dfd ("kbuild: Strip runtime const RELA sections correctly")
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> ---
>  scripts/Makefile.lib | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index cad20f0e66ee..0a1f1e67a0ed 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -377,7 +377,7 @@ quiet_cmd_objcopy = OBJCOPY $@
>  cmd_objcopy = $(OBJCOPY) $(OBJCOPYFLAGS) $(OBJCOPYFLAGS_$(@F)) $< $@
>
>  quiet_cmd_strip_relocs = RSTRIP  $@
> -cmd_strip_relocs = $(OBJCOPY) --remove-section='.rel*' $@
> +cmd_strip_relocs = $(OBJCOPY) --remove-section='.rel*' --remove-section=!.rela.dyn $@
>
>  # Gzip
>  # ---------------------------------------------------------------------------
> --
> 2.39.2
>
Alexandre Ghiti April 3, 2025, 3:45 p.m. UTC | #2
Hi Ard,

On 03/04/2025 17:11, Ard Biesheuvel wrote:
> On Thu, 3 Apr 2025 at 16:42, Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
>> riscv uses the .rela.dyn section to relocate the kernel at runtime but
>> that section is stripped from vmlinux. That prevents kexec to
>> successfully load vmlinux since it does not contain the relocations info
>> needed.
>>
> Maybe explain that .rela.dyn contains runtime relocations, which are
> only emitted if they are actually needed - as opposed to the static
> relocations that are not emitted as SHF_ALLOC sections, and are not
> considered to be part of the runtime image in the first place.


Ok I'll do.


> It
> would be nice if we could use --remove-relocations= here, which only
> removes static relocations, but unfortunately, llvm-objcopy does not
> support this.
>
> Also, I wonder if this should apply to all of .rel.dyn, .rela.dyn and
> .relr.dyn, as they all carry runtime relocations.


Ok, I'll add them to the next version.


>
> Finally, I'd be curious to know why RISC-V relies on --emit-relocs in
> the first place? Is the relocs check really needed? If not, it would
> be a nice opportunity to get rid of Makefile.postlink entirely.


So I had to check and it happens that this was an issue with the 
toolchain, I should check if that still happens with newer ones.

commit 559d1e45a16dcf1542e430ea3dce9ab625be98d0
Author: Alexandre Ghiti <alexghiti@rivosinc.com>
Date:   Wed Mar 29 06:53:29 2023 +0200

     riscv: Use --emit-relocs in order to move .rela.dyn in init

     To circumvent an issue where placing the relocations inside the init
     sections produces empty relocations, use --emit-relocs. But to avoid
     carrying those relocations in vmlinux, use an intermediate
     vmlinux.relocs file which is a copy of vmlinux *before* stripping its
     relocations.

     Suggested-by: Björn Töpel <bjorn@kernel.org>
     Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
     Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
     Link: 
https://lore.kernel.org/r/20230329045329.64565-7-alexghiti@rivosinc.com
     Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>


>
> In any case, for this change, or a variation along the lines of what I
> wrote above,
>
> Acked-by: Ard Biesheuvel <ardb@kernel.org>


Thanks,

Alex


>
>
>> Fixes: 71d815bf5dfd ("kbuild: Strip runtime const RELA sections correctly")
>> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>> ---
>>   scripts/Makefile.lib | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>> index cad20f0e66ee..0a1f1e67a0ed 100644
>> --- a/scripts/Makefile.lib
>> +++ b/scripts/Makefile.lib
>> @@ -377,7 +377,7 @@ quiet_cmd_objcopy = OBJCOPY $@
>>   cmd_objcopy = $(OBJCOPY) $(OBJCOPYFLAGS) $(OBJCOPYFLAGS_$(@F)) $< $@
>>
>>   quiet_cmd_strip_relocs = RSTRIP  $@
>> -cmd_strip_relocs = $(OBJCOPY) --remove-section='.rel*' $@
>> +cmd_strip_relocs = $(OBJCOPY) --remove-section='.rel*' --remove-section=!.rela.dyn $@
>>
>>   # Gzip
>>   # ---------------------------------------------------------------------------
>> --
>> 2.39.2
>>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
Masahiro Yamada April 4, 2025, 3:24 p.m. UTC | #3
On Fri, Apr 4, 2025 at 12:45 AM Alexandre Ghiti <alex@ghiti.fr> wrote:
>
> Hi Ard,
>
> On 03/04/2025 17:11, Ard Biesheuvel wrote:
> > On Thu, 3 Apr 2025 at 16:42, Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> >> riscv uses the .rela.dyn section to relocate the kernel at runtime but
> >> that section is stripped from vmlinux. That prevents kexec to
> >> successfully load vmlinux since it does not contain the relocations info
> >> needed.
> >>
> > Maybe explain that .rela.dyn contains runtime relocations, which are
> > only emitted if they are actually needed - as opposed to the static
> > relocations that are not emitted as SHF_ALLOC sections, and are not
> > considered to be part of the runtime image in the first place.
>
>
> Ok I'll do.
>
>
> > It
> > would be nice if we could use --remove-relocations= here, which only
> > removes static relocations, but unfortunately, llvm-objcopy does not
> > support this.
> >
> > Also, I wonder if this should apply to all of .rel.dyn, .rela.dyn and
> > .relr.dyn, as they all carry runtime relocations.
>
>
> Ok, I'll add them to the next version.
>
>
> >
> > Finally, I'd be curious to know why RISC-V relies on --emit-relocs in
> > the first place? Is the relocs check really needed? If not, it would
> > be a nice opportunity to get rid of Makefile.postlink entirely.
>
>
> So I had to check and it happens that this was an issue with the
> toolchain, I should check if that still happens with newer ones.
>
> commit 559d1e45a16dcf1542e430ea3dce9ab625be98d0
> Author: Alexandre Ghiti <alexghiti@rivosinc.com>
> Date:   Wed Mar 29 06:53:29 2023 +0200
>
>      riscv: Use --emit-relocs in order to move .rela.dyn in init




So,

Fixes: 559d1e45a16d ("riscv: Use --emit-relocs in order to move
.rela.dyn in init")

Is this the correct tag?
Alexandre Ghiti April 7, 2025, 8:15 a.m. UTC | #4
Hi Masahiro,

On Fri, Apr 4, 2025 at 5:25 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Fri, Apr 4, 2025 at 12:45 AM Alexandre Ghiti <alex@ghiti.fr> wrote:
> >
> > Hi Ard,
> >
> > On 03/04/2025 17:11, Ard Biesheuvel wrote:
> > > On Thu, 3 Apr 2025 at 16:42, Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> > >> riscv uses the .rela.dyn section to relocate the kernel at runtime but
> > >> that section is stripped from vmlinux. That prevents kexec to
> > >> successfully load vmlinux since it does not contain the relocations info
> > >> needed.
> > >>
> > > Maybe explain that .rela.dyn contains runtime relocations, which are
> > > only emitted if they are actually needed - as opposed to the static
> > > relocations that are not emitted as SHF_ALLOC sections, and are not
> > > considered to be part of the runtime image in the first place.
> >
> >
> > Ok I'll do.
> >
> >
> > > It
> > > would be nice if we could use --remove-relocations= here, which only
> > > removes static relocations, but unfortunately, llvm-objcopy does not
> > > support this.
> > >
> > > Also, I wonder if this should apply to all of .rel.dyn, .rela.dyn and
> > > .relr.dyn, as they all carry runtime relocations.
> >
> >
> > Ok, I'll add them to the next version.
> >
> >
> > >
> > > Finally, I'd be curious to know why RISC-V relies on --emit-relocs in
> > > the first place? Is the relocs check really needed? If not, it would
> > > be a nice opportunity to get rid of Makefile.postlink entirely.
> >
> >
> > So I had to check and it happens that this was an issue with the
> > toolchain, I should check if that still happens with newer ones.
> >
> > commit 559d1e45a16dcf1542e430ea3dce9ab625be98d0
> > Author: Alexandre Ghiti <alexghiti@rivosinc.com>
> > Date:   Wed Mar 29 06:53:29 2023 +0200
> >
> >      riscv: Use --emit-relocs in order to move .rela.dyn in init
>
>
>
>
> So,
>
> Fixes: 559d1e45a16d ("riscv: Use --emit-relocs in order to move
> .rela.dyn in init")
>
> Is this the correct tag?

This is the initial culprit yes, but if we use this tag, the fix won't
apply. So I decided to pick Ard's patch so that this fix can be easily
backported to 6.14, and I'll come up with a new version for previous
releases. Is that ok with you?

Thanks,

Alex

>
>
>
>
>
> --
> Best Regards
> Masahiro Yamada
Alexandre Ghiti April 7, 2025, 8:42 a.m. UTC | #5
On 07/04/2025 10:15, Alexandre Ghiti wrote:
> Hi Masahiro,
>
> On Fri, Apr 4, 2025 at 5:25 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>> On Fri, Apr 4, 2025 at 12:45 AM Alexandre Ghiti <alex@ghiti.fr> wrote:
>>> Hi Ard,
>>>
>>> On 03/04/2025 17:11, Ard Biesheuvel wrote:
>>>> On Thu, 3 Apr 2025 at 16:42, Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
>>>>> riscv uses the .rela.dyn section to relocate the kernel at runtime but
>>>>> that section is stripped from vmlinux. That prevents kexec to
>>>>> successfully load vmlinux since it does not contain the relocations info
>>>>> needed.
>>>>>
>>>> Maybe explain that .rela.dyn contains runtime relocations, which are
>>>> only emitted if they are actually needed - as opposed to the static
>>>> relocations that are not emitted as SHF_ALLOC sections, and are not
>>>> considered to be part of the runtime image in the first place.
>>>
>>> Ok I'll do.
>>>
>>>
>>>> It
>>>> would be nice if we could use --remove-relocations= here, which only
>>>> removes static relocations, but unfortunately, llvm-objcopy does not
>>>> support this.
>>>>
>>>> Also, I wonder if this should apply to all of .rel.dyn, .rela.dyn and
>>>> .relr.dyn, as they all carry runtime relocations.
>>>
>>> Ok, I'll add them to the next version.
>>>
>>>
>>>> Finally, I'd be curious to know why RISC-V relies on --emit-relocs in
>>>> the first place? Is the relocs check really needed? If not, it would
>>>> be a nice opportunity to get rid of Makefile.postlink entirely.
>>>
>>> So I had to check and it happens that this was an issue with the
>>> toolchain, I should check if that still happens with newer ones.
>>>
>>> commit 559d1e45a16dcf1542e430ea3dce9ab625be98d0
>>> Author: Alexandre Ghiti <alexghiti@rivosinc.com>
>>> Date:   Wed Mar 29 06:53:29 2023 +0200
>>>
>>>       riscv: Use --emit-relocs in order to move .rela.dyn in init
>>
>>
>>
>> So,
>>
>> Fixes: 559d1e45a16d ("riscv: Use --emit-relocs in order to move
>> .rela.dyn in init")
>>
>> Is this the correct tag?
> This is the initial culprit yes, but if we use this tag, the fix won't
> apply. So I decided to pick Ard's patch so that this fix can be easily
> backported to 6.14, and I'll come up with a new version for previous
> releases. Is that ok with you?


And I have just looked at 6.15-rc1 and noticed that the relocation 
stripping was moved to Makefile.vmlinux, so this fix won't apply to 6.14 
neither.

I'll then use the initial culprit, which is Fixes: 559d1e45a16d ("riscv: 
Use --emit-relocs in order to move.rela.dyn in init").

Sorry for the noise,

Alex


>
> Thanks,
>
> Alex
>
>>
>>
>>
>>
>> --
>> Best Regards
>> Masahiro Yamada
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
Masahiro Yamada April 10, 2025, 6:31 p.m. UTC | #6
On Mon, Apr 7, 2025 at 5:43 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
>
>
> On 07/04/2025 10:15, Alexandre Ghiti wrote:
> > Hi Masahiro,
> >
> > On Fri, Apr 4, 2025 at 5:25 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >> On Fri, Apr 4, 2025 at 12:45 AM Alexandre Ghiti <alex@ghiti.fr> wrote:
> >>> Hi Ard,
> >>>
> >>> On 03/04/2025 17:11, Ard Biesheuvel wrote:
> >>>> On Thu, 3 Apr 2025 at 16:42, Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> >>>>> riscv uses the .rela.dyn section to relocate the kernel at runtime but
> >>>>> that section is stripped from vmlinux. That prevents kexec to
> >>>>> successfully load vmlinux since it does not contain the relocations info
> >>>>> needed.
> >>>>>
> >>>> Maybe explain that .rela.dyn contains runtime relocations, which are
> >>>> only emitted if they are actually needed - as opposed to the static
> >>>> relocations that are not emitted as SHF_ALLOC sections, and are not
> >>>> considered to be part of the runtime image in the first place.
> >>>
> >>> Ok I'll do.
> >>>
> >>>
> >>>> It
> >>>> would be nice if we could use --remove-relocations= here, which only
> >>>> removes static relocations, but unfortunately, llvm-objcopy does not
> >>>> support this.
> >>>>
> >>>> Also, I wonder if this should apply to all of .rel.dyn, .rela.dyn and
> >>>> .relr.dyn, as they all carry runtime relocations.
> >>>
> >>> Ok, I'll add them to the next version.
> >>>
> >>>
> >>>> Finally, I'd be curious to know why RISC-V relies on --emit-relocs in
> >>>> the first place? Is the relocs check really needed? If not, it would
> >>>> be a nice opportunity to get rid of Makefile.postlink entirely.
> >>>
> >>> So I had to check and it happens that this was an issue with the
> >>> toolchain, I should check if that still happens with newer ones.
> >>>
> >>> commit 559d1e45a16dcf1542e430ea3dce9ab625be98d0
> >>> Author: Alexandre Ghiti <alexghiti@rivosinc.com>
> >>> Date:   Wed Mar 29 06:53:29 2023 +0200
> >>>
> >>>       riscv: Use --emit-relocs in order to move .rela.dyn in init
> >>
> >>
> >>
> >> So,
> >>
> >> Fixes: 559d1e45a16d ("riscv: Use --emit-relocs in order to move
> >> .rela.dyn in init")
> >>
> >> Is this the correct tag?
> > This is the initial culprit yes, but if we use this tag, the fix won't
> > apply. So I decided to pick Ard's patch so that this fix can be easily
> > backported to 6.14, and I'll come up with a new version for previous
> > releases. Is that ok with you?
>
>
> And I have just looked at 6.15-rc1 and noticed that the relocation
> stripping was moved to Makefile.vmlinux, so this fix won't apply to 6.14
> neither.
>
> I'll then use the initial culprit, which is Fixes: 559d1e45a16d ("riscv:
> Use --emit-relocs in order to move.rela.dyn in init").
>
> Sorry for the noise,

You do not need to worry about whether this commit can be cleanly
back-ported to the stable kernels or not.

If the fix cleanly applies, the process should be easy (and almost automatic).

If not, we can modify the patch context and submit to the stable ML.
This needs manual modification, but is still possible.
diff mbox series

Patch

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index cad20f0e66ee..0a1f1e67a0ed 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -377,7 +377,7 @@  quiet_cmd_objcopy = OBJCOPY $@
 cmd_objcopy = $(OBJCOPY) $(OBJCOPYFLAGS) $(OBJCOPYFLAGS_$(@F)) $< $@
 
 quiet_cmd_strip_relocs = RSTRIP  $@
-cmd_strip_relocs = $(OBJCOPY) --remove-section='.rel*' $@
+cmd_strip_relocs = $(OBJCOPY) --remove-section='.rel*' --remove-section=!.rela.dyn $@
 
 # Gzip
 # ---------------------------------------------------------------------------