Re: [PATCH v3] x86/mm: Short circuit damage from "fishy" ref/typecount failure

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.01.2021 14:02, Andrew Cooper wrote:
> This code has been copied in 3 places, but it is problematic.
> 
> All cases will hit a BUG() later in domain teardown, when a the missing
> type/count reference is underflowed.

I'm afraid I could use some help with this: Why would there
be a missing reference, when the getting of one failed? IOW
I'm not (yet) convinced you don't make the impact more
severe in the (supposedly) impossible case of these paths
getting hit, by converting a domain crash into a host one.
It is in particular relevant that a PV guest may be able to
cheat and "guess" an MFN to obtain references for before a
certain hypercall (or other operation) actually completed.

> v2:
>  * Reword the commit message.
>  * Switch BUG() to BUG_ON() to further reduce code volume.

Short of us explicitly agreeing that such is fine to use, I
think we ought to stick to the previously (long ago) agreed
rule that BUG_ON() controlling expressions should not have
side effects, for us not wanting to guarantee it will now
and forever _not_ behave like ASSERT() wrt to evaluating
the expression.

Jan