Re: [PATCH 1/5] x86: Put trampoline in .init.data section

[Jan Beulich <jbeulich@suse.com>]

On 19.08.2024 17:30, Frediano Ziglio wrote:
> On Mon, Aug 19, 2024 at 3:30 PM Jan Beulich <jbeulich@suse.com> wrote:
>> On 19.08.2024 16:16, Frediano Ziglio wrote:
>>> On Thu, Aug 8, 2024 at 9:54 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>> On 08.08.2024 10:00, Frediano Ziglio wrote:
>>>>> On Thu, Aug 8, 2024 at 8:34 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>> On 07.08.2024 15:48, Alejandro Vallejo wrote:
>>>>>>> This change allows to put the trampoline in a separate, not executable
>>>>>>> section. The trampoline contains a mix of code and data (data which
>>>>>>> is modified from C code during early start so must be writable).
>>>>>>> This is in preparation for W^X patch in order to satisfy UEFI CA
>>>>>>> memory mitigation requirements.
>>>>>>
>>>>>> Which, aiui, has the downside of disassembly of the section no longer
>>>>>> happening by default, when using objdump or similar tools, which go from
>>>>>> section attributes. Why is it being in .init.text (and hence RX) not
>>>>>> appropriate? It should - in principle at least - be possible to avoid
>>>>>> all in-place writing to it, but instead only ever write to its relocated
>>>>>> copy. Quite a bit more code churn of course.
>>>>>>
>>>>>> I wonder if we shouldn't put the trampoline in its own section, RWX in
>>>>>> the object file, and switched to whatever appropriate in the binary
>>>>>> (which really may be RX, not RW).
>>>>>
>>>>> We cannot have RWX to satisfy UEFI CA memory mitigation, that's why I
>>>>> had to move it, code sections should not be writeable. We can mark
>>>>> either RX or RW but we use the data very early so we are not able to
>>>>> change the permissions (we can try with all complications that this
>>>>> could bring like how to report an error at so early stages).
>>>>
>>>> The early writing could be done away with, as indicated. There's not
>>>> really any strict requirement to write to the trampoline region within
>>>> the Xen image. All updates to it could in principle be done after it
>>>> was copied into low memory. Then (and of course only then) could it be
>>>> part of an RX section in the image, maybe .init.text, maybe a separate
>>>> .trampoline section.
>>>
>>>    how strong are you on this? Is this "objdump" thing such a big
>>> issue? The code contains a lot of 16 bit code which would require
>>> additional options anyway. Won't be an assembly listing output more
>>> helpful instead?
>>
>> Well. Whether a listing can serve as a stand-in depends on the situation.
>> Not being able to disassemble code (e.g. also in the final executable)
>> can be pretty limiting. The need to pass extra options is related, but
>> not really an argument against.
> 
> If some code is inside some data section (in the final binary) you can
> use -D option to disassemble everything, even data. For instance a
> "objdump -D xen-syms -m i8086" and look for some "trampoline" symbols.
> Yes, the output of -D is surely longer than -d.

Well, no, I surely don't want to disassemble all data. Based on what I've
observed in the past, I also wouldn't be surprised if objdump didn't get
utterly confused by disassembling data, perhaps to the point of crashing.

>>> Could I ouput the trampoline in a code section ("ax" instead of "aw")
>>> and then later move it into .init.data section assuring .init.data is
>>> writeable but not executable?
>>
>> Could you go into a little more detail on what you mean here? At the
>> first glance my reaction is "yes, sure, why not", but much depends on
>> what exactly is meant.
> 
> For instance you could put the trampoline into a
>     .section .init.trampoline, "awx", @progbits
> section (having the "x" will be disassembled by objdump -d head.o).
> Then in xen/arch/x86/xen.lds.S in the .init.data section having something like
> ...
>   DECL_SECTION(.init.data) {
>        *(.init.bss.stack_aligned)
>       (.init.trampoline)
>    ...
> this will put the trampoline in .init.data section of the final
> object. At this point the .init.data containing code will have execute
> permission that you would have to fix using objcopy command.
> The final trampoline will be in a data section not executable so to
> use objdump you will need the -D option, but not disassembling head.o.
> In theory we could keep the temporary object file before the objcopy
> adjustment to avoid the -D but I don't think it would save a lot of
> burdain.

Part of my "want to be able to disassemble" also applies to the final
binaries. Since iirc one can disassemble individual sections, an option
may be to have .trampoline be its own section even in the final (PE)
binary?

In any event, especially as long as there is no really good option, I
think I'd like to have input from Andrew and/or Roger as well.

Jan