Re: [Xen-devel] [PATCH 2/2] xen/livepatch: Don't crash on encountering STN_UNDEF relocations

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.06.17 at 01:05, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 19/06/2017 19:30, Konrad Rzeszutek Wilk wrote:
>> On Wed, Jun 14, 2017 at 12:49:21PM -0600, Jan Beulich wrote:
>>>>>> Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 06/14/17 8:34 PM >>>
>>>> Well - I've got a livepatch with such a relocation.  It is probably a
>>>> livepatch build tools issue, but the question is whether Xen should ever
>>>> accept such a livepatch or not (irrespective of whether this exact
>>>> relocation is permitted within the ELF spec).
>>> Since the spec explicitly mentions that case, I think we'd better support 
> it.
>>> But it wouldn't be the end of the world if we didn't, as presumably there
>>> aren't that many use cases for it.
>> OK. In that case:
>>
>> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>

I have to admit that I'm surprised by that, not only because of
what Andrew says below, but also because imo the patch would
imo need to be done somewhat differently, as outlined earlier
(making STN_UNDEF less of a special case).

>> Do you think it would be possible to generate an test-case for this
>> in arch/test/livepatch?
> 
> I can trivially cause this situation to occur with the current build
> tools, but we are currently presuming a build tools bug to be the
> underlying issue behind getting a STN_UNDEF relocation in the livepatch.
> 
> Given that a STN_UNDEF relocation (appears to) mean a NULL dereference
> once the relocations are evaluated, I am not happy with supporting such
> a case.

Well, quite clearly this can be of use only to produce constants,
not to produce pointers (unless chained ["expression"] relocations
are being used, where the result of one element in the chain is the
addend of the next one, albeit even then this would effectively be
a NOP relocation, so may be useful only when post-editing binaries
where the tool doesn't want to change [relocation] section sizes).

> Therefore, I'm going to insist that we take a concrete decision as to
> what to do in the hypervisor code, before adding a test case, and
> advocate for excluding it outright rather than tolerating it in the
> (certain?) knowledge that Xen will subsequently crash.

As per the explanation above, we can't tell whether Xen will
subsequently crash, as we don't know what it is that is being
relocated by such an relocation. While, as indicated before, I'd like
to see us support everything the standard mandates, I wouldn't
view it as a big problem to simply return -EOPNOTSUPP for this case
for the time being.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel