Re: [PATCH] domain: Validate __copy_to_guest in VCPUOP_register_runstate_memory_area

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.11.2024 12:24, Michal Orzel wrote:
> On 26/11/2024 11:40, Jan Beulich wrote:
>> On 26.11.2024 11:26, Michal Orzel wrote:
>>> For guests with paging mode external, guest_handle_okay() always returns
>>> success, even if the guest handle is invalid (e.g. address not in P2M).
>>> In VCPUOP_register_runstate_memory_area, we would then blindly set
>>> runstate_guest() for a given vCPU to invalid handle. Moreover, we don't
>>> check the return value from __copy_to_guest() and return success to the
>>> guest, even in case of a failure during copy.
>>
>> I'm afraid this is all deliberate, providing best effort behavior. For a
>> paging mode external guest, the handle may become valid subsequently. If
>> any __copy_to_guest() fail here, subsequent update_runstate_area() may
>> succeed (and success of the actual copying isn't checked there either).
> Hmm, I've never looked at that this way. I always thought that mapping must 
> be in place
> before setting up handle. Is this concept really something used (e.g. in 
> Linux) or just a
> way for us to provide excuse for not hardening the code because of a fear of 
> regressions, etc.?

I'm pretty certain some common guest OS used such "early" registration at
some point. I don't recall whether that was Windows or Linux, nor - for the
latter - whether it was upstream (pv-ops) or the old forward ports from the
2.6.18-xen tree. And I can't say whether that may still be the case
somewhere. Nor that it was specifically the runstate area (it seems more
likely that it was the vCPU info area). Hence my desire for us to be
cautious.

Jan