Re: [PATCH] xen/livepatch: fixup relocations to replaced symbols

["Roger Pau Monné" <roger.pau@citrix.com>]

On Mon, Jul 21, 2025 at 04:51:33PM +0100, Ross Lagerwall wrote:
> On Wed, Jul 16, 2025 at 5:00 PM Roger Pau Monne <roger.pau@citrix.com> wrote:
> >
> > In a livepatch payload relocations will refer to included functions.  If
> > that function happens to be a replacement for an existing Xen function, the
> > relocations on the livepatch payload will use the newly introduced symbol,
> > rather than the old one.  This is usually fine, but if the result of the
> > relocation is stored for later use (for example in the domain struct),
> > usages of this address will lead to a page-fault once the livepatch is
> > reverted.
> >
> > Implement a second pass over relocations once the list of replaced
> > functions has been loaded, and fixup any references to replaced functions
> > to use the old symbol address instead of the new one.  There are some
> > sections that must be special cased to continue using the new symbol
> > address, as those instances must reference the newly added livepatch
> > content (for example the alternative patch sites).
> >
> 
> Would it be possible to fix this at build time instead by generating the
> payload such that relocations to changed functions always point at the
> Xen function rather than the included function?

Hm, it's kind of complicated (I'm not saying it's impossible).  I
think it will require renaming the replaced function symbols, and then
introducing new unresolved symbols that reference the existing
functions, and adjusting the relocations to point to those new
symbols.

Then Xen would resolve those symbols and the relocations would be
calculated using the old addresses.

You can not unconditionally replace all the relocations that point to
replacement symbols, as some of the relocations will need to point to
new vs old instances (just as this patch does, where not all
relocations are adjusted to point to the old addresses).

I can give it a try, but looks more complicated than the approach
here, as it requires crafting extra symbols to add to the payload, and
adjust relocations.

> This seems preferable to me rather than making the runtime code more
> complicated.
> 
> Of course, neither approach would fix all cases - if the function is
> completely new calling it would still page-fault after reverting the
> live patch - so the usual care needs to be taken when creating live
> patches.

Indeed, storing pointers to newly introduced functions won't work,
that needs care, but it should be easier to spot by code review of the
livepatch.  Would be nice to get an automated way to prevent such
pointers from getting stored, but I can't think of a way to do it.

Thanks, Roger.