Re: [PATCH v2] binfmt_flat: do not stop relocating GOT entries prematurely on riscv

Niklas Cassel
2022-04-15T02:11:45Z

On Fri, Apr 15, 2022 at 10:13:31AM +0900, Damien Le Moal wrote:
> On 4/15/22 10:08, Niklas Cassel wrote:
> > On Fri, Apr 15, 2022 at 09:56:38AM +0900, Damien Le Moal wrote:
> >> On 4/15/22 09:30, Niklas Cassel wrote:
> >>> On Fri, Apr 15, 2022 at 08:51:27AM +0900, Damien Le Moal wrote:
> >>>> On 4/14/22 18:10, Niklas Cassel wrote:
> >
> > (snip)
> >
> >> So if we are sure that we can just skip the first 16B/8B for riscv, I
> >> would not bother checking the header content. But as mentioned, the
> >> current code is fine too.
> >
> > That was my point, I'm not sure that we can be sure that we can always
> > skip it in the future. E.g. if the elf2flt linker script decides to swap
> > the order of .got and .got.plt for some random reason in the future,
> > we would skip data that really should have been relocated.
>
> Good point. Your current patch is indeed better then. BUT that would also
> mean that the skip header function needs to be called inside the loop
> then, no ? If the section orders are reversed, we would still need to skip
> that header in the middle of the relocation loop...

So this is theoretical, but if the sections were swapped in the linker
script, and we have the patch in $subject applied, we will not skip data
that needs to be relocated. But after relocating all the entries in the
.got section we will still break too early, if we actually had any
.got.plt entries after the .got.plt header. The .got.plt entries would
not get relocated.

However, the elf2flt maintainer explicitly asked ut to fix the kernel or
binutils, so that they can continue using the exact same linker script
that it has been using forever. (And we shouldn't need to change binutils
just for the bFLT format.)

So the chance that the linker script changes in practice is really small.
(This .got.plt vs .got hasn't changed in 19 years.)

But if it does, we will just have one problem instead of two :)
However, I think that applying this patch is sufficient for now,
since it makes the code work with the existing elf2flt linker script.

Adapting the code to also handle this theoretical layout of the linker
script would just complicate things even more. I'm not even sure if we
would be able to handle this case, since the information about the .got
and .got.plt section sizes is lost once the ELF has been converted to
bFLT.

Kind regards,
Niklas