Re: [Xen-devel] [PATCH 3/3] x86/mm-locks: apply a bias to lock levels for current domain

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 12/20/18 9:05 AM, Roger Pau Monné wrote:
> On Wed, Dec 19, 2018 at 06:10:04PM +0000, George Dunlap wrote:
>> On 12/19/18 2:59 PM, Roger Pau Monné wrote:
>>>> Using 'current' means that potential deadlocks which would now cause a
>>>> BUG() won't anymore.  I'm fine with not adding extra protections that
>>>> aren't there now; but I don't want to remove protections that are.
>>>
>>> The lock ordering enforcement is still kept as-is, but Xen is allowed
>>> to lock the caller mm locks in the right order (be it privileged or
>>> not) after having locked a subject domain ones also in the correct
>>> order.
>>
>> Suppose we have mm_lock_x, with value 8, and mm_lock_y with value 16.
>>
>> Suppose we have H1, which grabs current->mm_lock_x and tgt->mm_lock_y.
> 
> With my proposal H1 won't be a valid lock sequence, since
> current->mm_lock_x level > tgt->mm_lock_y level.

I made a mistake; here's a corrected and expanded example.

Suppose we have mm_lock_x, with value 8, and mm_lock_y with value 16.

Suppose we have H1, which grabs tgt->mm_lock_x and current->mm_lock_y.

And suppose we have H2, which grabs tgt->mm_lock_y, and current->mm_lock_x.

And suppose domA calls H1 on domB at the same time that domB calls H2 on
domA.  We could have the following sequence:

1. H1: grab B->mm_lock_x
2. H2: grab A->mm_lock_y
3. H1: wait on A->mm_lock_y
4. H2: wait on B->mm_lock_x #DEADLOCK

With the current mm lock checking:
* H1 grabs a lock of level 8, then a lock of level 16, so no BUG check.
* H2 grabs a lock of level 16 and then a lock of level 8, and so BUG
checks, catching this potential deadlock.

With your "current bias" patch:
* H1 grabs a lock of level 8, then a lock of level 80 (16 + 64), so no
BUG check.
* H2 grabs a lock of level 16, then a lock of level 72 (8 + 64), so no
BUG check, missing the potential deadlock.

With "dom0 bias", you basically have to choose one of the hypercalls to
be privileged only.  If we choose H2 to be privileged, then B must be
dom0 and A must be a domU.  So we get:
* H1 grabs lock level 72 (16 + 64), then lock level 16, causing a BUG check.
* H2 grabs lock level 16, then lock level 80, no bug check.

The practical implication of dom0 bias is that any hypercall which grabs
two mm locks, one of two things must be true:
* When it is called from one domU to another domU, locks must follow the
"normal" locking order listed in mm-locks.h
* When it is called between a dom0 and a domU, it must be consistenly
called either one way or the other; i.e., it must always be dom0 calling
it with a domU target, or domU calling it with a dom0 target (otherwise
the dom0 / domU locking order is violated).

Now that I state it that way, it's not immediately obvious that's a
property we have or want.

> I'm fine with this proposal, it's just that if it's safe for Dom0 to
> pick any other domain mm lock and then any Dom0 mm lock it should also
> be safe for any domain and not Dom0 only.

If this were true, I'd be happy with your proposal; but I'm pretty sure
it's not true.  If you can give me a proof that no two hypercalls H1 and
H2 can exist under your proposal, then we can consider it.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel