Re: [PATCH][4.17?] x86: also zap secondary time area handles during soft reset

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.10.2022 17:23, Roger Pau Monné wrote:
> On Thu, Oct 13, 2022 at 08:48:21AM +0200, Jan Beulich wrote:
>> Just like domain_soft_reset() properly zaps runstate area handles, the
>> secondary time area ones also need discarding to prevent guest memory
>> corruption once the guest is re-started.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks.

>> ---
>> To avoid another for_each_vcpu() here, domain_soft_reset() could also
>> be made call a new arch_vcpu_soft_reset() out of its already present
>> loop. Yet that would make the change less isolated.
>>
>> In domain_soft_reset() I wonder whether, just like done here, the
>> zapping of runstate area handles and vCPU info mappings wouldn't better
>> be done after all operations which can fail. But perhaps for this to
>> matter the domain is left in too inconsistent a state anyway if the
>> function fails ...
> 
> We would need some kind of recovery anyway, so given the current code
> and lack of recovery it doesn't seem to matter much.
> 
>> However, at the very least I wonder whether x86'es
>> restriction to HVM shouldn't leave PV guests undisturbed if a soft-reset
>> was attempted on them. Right now they not only have state partially
>> clobbered, but (if the arch function is reached) they would be crashed
>> unconditionally.
> 
> It's a toolstack initiated operation by a domctl, so I'm fine with
> saying that it's up for the toolstack to prevent soft resets from
> being attempted against PV domains.  Would be nice to reject the
> operation earlier on the hypervisor, maybe by moving
> arch_domain_soft_reset() earlier in domain_soft_reset() so that we
> can return without crashing?

I wasn't sure about moving arch_domain_soft_reset() as a whole, but
yes, if that wouldn't cause other interaction issues this might be
an option.

> In any case it's unlikely for a domain that was attempting a soft
> reset to survive the hypervisor rejecting the operation, so it doesn't
> matter much whether the domain is crashed by Xen or left as-is I would
> think.

I'm sorry, I don't think I understand what you're saying here. For
PV I'd very much expect the guest to survive; it may of course then
be crashed or destroyed by a further tool stack operation.

Jan