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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.07.2025 17:02, Roger Pau Monné wrote:
> On Wed, Jul 16, 2025 at 06:31:03PM +0200, Jan Beulich wrote:
>> On 16.07.2025 18:00, Roger Pau Monne 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).
>>
>> This is what I was fearing, when you first mentioned the problem (and the
>> plan) to me. What I don't see is why you do your fixing up regardless of
>> relocation type. Relative relocations within the payload ought to be fine
>> to not override? At which point some of the special casing may already no
>> longer be necessary.
>>
>> (Later) Except that if code uses PC-relative addressing to determine a
>> pointer to store into some struct, that'll appear as a relative relocation
>> type, too. But then you may have a bigger problem: When referencing and
>> referenced code are in the same section and in the same translation unit,
>> the assembler could avoid emitting a relocation altogether. You would see
>> nothing to fix up ...
> 
> The only way for the referencing and referenced code to be in the same
> function would be for the function to reference itself, which should
> be quite rare?  I don't recall seeing any code in Xen where a function
> stores a pointer to itself.
> 
> Otherwise each function is in a separate section, and hence references
> to functions should always use a relocation.

Oh, right, I forgot about -ffunction-sections being in use.

Jan