On 08.03.2022 17:26, Roger Pau Monné wrote: > On Tue, Mar 08, 2022 at 05:15:33PM +0100, Roger Pau Monné wrote: >> On Tue, Mar 08, 2022 at 04:45:34PM +0100, Jan Beulich wrote: >>> On 08.03.2022 16:36, Bjoern Doebel wrote: >>>> --- a/xen/arch/x86/livepatch.c >>>> +++ b/xen/arch/x86/livepatch.c >>>> @@ -339,7 +339,7 @@ int arch_livepatch_perform_rela(struct livepatch_elf >>>> *elf, >>>> >>>> val -= (uint64_t)dest; >>>> *(int32_t *)dest = val; >>> >>> Afaict after this assignment ... >>> >>>> - if ( (int64_t)val != *(int32_t *)dest ) >>>> + if ( (int32_t)val != *(int32_t *)dest ) >>> >>> ... this condition can never be false. The cast really wants to be >>> to int64_t, and the overflow you saw being reported is quite likely >>> for a different reason. But from the sole message you did quote >>> it's not really possible to figure what else is wrong. >> >> It seems Linux has that check ifdef'ed [0], but there's no reference >> as to why it's that way (I've tracked it back to the x86-64 import on >> the historical tree, f3081f5b66a06175ff2dabfe885a53fb04e71076). >> >> It's a 64bit relocation using a 32bit value, but it's unclear to me >> that modifying the top 32bits is not allowed/intended. > > Urg, I've worded this very badly. It's a 64bit relocation using a > 32bit value, but it's unclear to me that modifying the top 32bits is > not allowed/intended and fine to be dropped.
I'm still confused: Afaics this is in the handling of R_X86_64_PC32, which is a 32-bit relocation. Only a 32-bit field in memory is to be modified, and the resulting value needs to fit such that when the CPU fetches it and sign-extends it to 64 bits, the original value is re-established. Hence the check, aiui. Jan
