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

[Damien Le Moal <damien.lemoal@opensource.wdc.com>]

On 4/15/22 11:11, Niklas Cassel wrote:
> 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.

OK. All good then.
I maintain my reviewed-by tag :)



-- 
Damien Le Moal
Western Digital Research