Re: [RFC PATCH 11/28] x86/pvh: Avoid absolute symbol references in .head.text

[Jason Andryuk <jason.andryuk@amd.com>]

On 2024-09-25 17:50, Ard Biesheuvel wrote:
> On Wed, 25 Sept 2024 at 23:11, Jason Andryuk <jason.andryuk@amd.com> wrote:
>>
>> Hi Ard,
>>
>> On 2024-09-25 11:01, Ard Biesheuvel wrote:
>>> From: Ard Biesheuvel <ardb@kernel.org>
>>>
>>> The .head.text section contains code that may execute from a different
>>> address than it was linked at. This is fragile, given that the x86 ABI
>>> can refer to global symbols via absolute or relative references, and the
>>> toolchain assumes that these are interchangeable, which they are not in
>>> this particular case.
>>>
>>> In the case of the PVH code, there are some additional complications:
>>> - the absolute references are in 32-bit code, which get emitted with
>>>     R_X86_64_32 relocations, and these are not permitted in PIE code;
>>> - the code in question is not actually relocatable: it can only run
>>>     correctly from the physical load address specified in the ELF note.
>>>
>>> So rewrite the code to only rely on relative symbol references: these
>>> are always 32-bits wide, even in 64-bit code, and are resolved by the
>>> linker at build time.
>>>
>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>
>> Juergen queued up my patches to make the PVH entry point position
>> independent (5 commits):
>> https://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git/log/?h=linux-next
>>
>> My commit that corresponds to this patch of yours is:
>> https://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git/commit/?h=linux-next&id=1db29f99edb056d8445876292f53a63459142309
>>
>> (There are more changes to handle adjusting the page tables.)
>>
> 
> Thanks for the head's up. Those changes look quite similar, so I guess
> I should just rebase my stuff onto the xen tree.
> 
> The only thing that I would like to keep from my version is
> 
> + lea (gdt - pvh_start_xen)(%ebp), %eax

If you rebase on top of the xen tree, using rva() would match the rest 
of the code:

	lea rva(gdt)(%ebp), %eax

> + add %eax, 2(%eax)
> + lgdt (%eax)
> 
> and
> 
> - .word gdt_end - gdt_start
> - .long _pa(gdt_start)
> + .word gdt_end - gdt_start - 1
> + .long gdt_start - gdt
> 
> The first line is a bugfix, btw, so perhaps I should send that out
> separately. But my series relies on all 32-bit absolute symbol
> references being removed, since the linker rejects those when running
> in PIE mode, and so the second line is needed to get rid of the _pa()
> there.

Sounds good.

Regards,
Jason