Re: [PATCH][4.17] x86/shadow: drop (replace) bogus assertions

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.10.2022 12:30, Roger Pau Monné wrote:
> On Fri, Oct 14, 2022 at 10:49:55AM +0200, Jan Beulich wrote:
>> The addition of a call to shadow_blow_tables() from shadow_teardown()
>> has resulted in the "no vcpus" related assertion becoming triggerable:
>> If domain_create() fails with at least one page successfully allocated
>> in the course of shadow_enable(), or if domain_create() succeeds and
>> the domain is then killed without ever invoking XEN_DOMCTL_max_vcpus.
>>
>> The assertion's comment was bogus anyway: Shadow mode has been getting
>> enabled before allocation of vCPU-s for quite some time. Convert the
>> assertion to a conditional: As long as there are no vCPU-s, there's
>> nothing to blow away.
>>
>> Fixes: e7aa55c0aab3 ("x86/p2m: free the paging memory pool preemptively")
>> Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>
>> A similar assertion/comment pair exists in _shadow_prealloc(); the
>> comment is similarly bogus, and the assertion could in principle trigger
>> e.g. when shadow_alloc_p2m_page() is called early enough. Replace those
>> at the same time by a similar early return, here indicating failure to
>> the caller (which will generally lead to the domain being crashed in
>> shadow_prealloc()).
> 
> It's my understanding we do care about this because a control domain
> could try to populate the p2m before calling XEN_DOMCTL_max_vcpus, and
> hence could trigger the ASSERT, as otherwise asserting would be fine.

Yes, that's the scenario I had in mind.

>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks, but Andrew and I will need to reach agreement before I can put
this (or whatever alternative) in.

Jan