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

[Frediano Ziglio <frediano.ziglio@xxxxxxxxx>]

(added again missing recipients)

On Thu, Aug 8, 2024 at 9:54 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 08.08.2024 10:00, Frediano Ziglio wrote:
> > On Thu, Aug 8, 2024 at 8:34 AM Jan Beulich <jbeulich@xxxxxxxx> 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.
>
> Jan

Not sure if I should be flattered or scared with that.
You are surely putting quite some trust on me. On the other hand you
are putting also some effort.
The change I proposed is just one line, you are asking to check all
reference, change them correctly and possibly testing them all.
I tried to do a small test (removing symbols from the trampoline) and
for instance head.S is writing to that section. That would require all
references to be made after the copy and to the copy. At the moment
the copy is done differently for different paths so it's less
straightforward than it seems.

Frediano