Re: [PATCH v3] x86/pv: inject #UD for entirely missing SYSCALL callbacks

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.10.2020 09:53, Jan Beulich wrote:
> On 28.10.2020 22:31, Andrew Cooper wrote:
>> On 26/10/2020 09:40, Jan Beulich wrote:
>>> In the case that no 64-bit SYSCALL callback is registered, the guest
>>> will be crashed when 64-bit userspace executes a SYSCALL instruction,
>>> which would be a userspace => kernel DoS.  Similarly for 32-bit
>>> userspace when no 32-bit SYSCALL callback was registered either.
>>>
>>> This has been the case ever since the introduction of 64bit PV support,
>>> but behaves unlike all other SYSCALL/SYSENTER callbacks in Xen, which
>>> yield #GP/#UD in userspace before the callback is registered, and are
>>> therefore safe by default.
>>>
>>> This change does constitute a change in the PV ABI, for the corner case
>>> of a PV guest kernel not registering a 64-bit callback (which has to be
>>> considered a defacto requirement of the unwritten PV ABI, considering
>>> there is no PV equivalent of EFER.SCE).
>>>
>>> It brings the behaviour in line with PV32 SYSCALL/SYSENTER, and PV64
>>> SYSENTER (safe by default, until explicitly enabled).
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>> Signed-off-by: Jan Beulich <JBeulich@xxxxxxxx>
>>> ---
>>> v3:
>>>  * Split this change off of "x86/pv: Inject #UD for missing SYSCALL
>>>    callbacks", to allow the uncontroversial part of that change to go
>>>    in. Add conditional "rex64" for UREGS_rip adjustment. (Is branching
>>>    over just the REX prefix too much trickery even for an unlikely to be
>>>    taken code path?)
>>
>> I find this submission confusion seeing as my v3 is already suitably
>> acked and ready to commit.  What I haven't had yet enough free time to
>> do so.
> 
> My objection to the other half stands, and hence, I'm afraid, stands
> in the way of your patch getting committed. Aiui Roger's ack doesn't
> invalidate my objection, sorry.

Actually I realize now that earlier I said I'm okay with this going in
if Roger acks it. I take it that Roger was aware of the controversy
when providing the ack. Therefore I'd like to take back what I've said
above, and your supposed v3 ought to be okay to get committed.

As to backporting - I'd surely be taking the split off part here, but
I have to admit I'm hesitant to take the full change. IOW splitting
the two changes might still be a helpful thing.

Jan