Re: [Xen-devel] [PATCH v2] interrupts: allow guest to set and clear MSI-X mask bit

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.08.13 at 13:01, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> On 05/08/13 11:44, Jan Beulich wrote:
>>>>> On 23.07.13 at 19:59, Joby Poriyath <joby.poriyath@xxxxxxxxxx> wrote:
>>> On Tue, Jul 23, 2013 at 09:28:42AM -0400, Konrad Rzeszutek Wilk wrote:
>>>> On Tue, Jul 23, 2013 at 11:54:41AM +0100, Joby Poriyath wrote:
>>>>> Ping...
>>>>>
>>>>> Could you kindly review this?
>>>> I believe Malcom reviewed and sent you an email with his response.
>>> Andrew and Malcolm has reviewed the code.
>>>
>>>> Are you waiting for Andrew to look at it as well? Or Jan? Jan is
>>>> out on vacation for a couple of weeks.
>>> I was hoping that Jan would also review this, since this patch 
>>> partly reverts his code. 
>>>
>>> This patch makes an assumption that, for a passed through device, 
>>> xen needs to mask the MSI-X control bit only when it does vcpu 
>>> migration. Otherwise xen leaves it in the state guest has set it.
>> Exactly. And hence the patch is wrong, 
> 
> Unfortunately it is not.
> 
> The whole problem is that the VF can (and does) request that the PF
> resets itself (using an out-of-band mailbox system in the hardware
> itself), causing the mask bit to change (i.e. get set) without Xen's
> knowledge.

The at the very least such a code change should be done for VFs
only. But I'd object to that too.

> At this point, it is only the guest which can rectify the situation and
> clear the mask bit.
> 
> I would agree that the solution is not perfect, but SRIOV virtual
> functions rely on the ability to clear the mask bit, whether or not it
> is a security issue.  This means that your patch to disable writing to
> the mask bit has caused a functional regression in all SRIOV operations
> in Xen.

Only in the special case that was described in the patch header
afaict (or else I'm sure we would have known much earlier).

> I would say that the less bad alternative is to reintroduce the ability
> to change the mask bit, and fix the regression, then work out how to fix
> the security aspect.

No, my position is pretty clear here: No re-introduction of security
issues when fixing secondary problems.

The more that the immediate (but only partial) solution would be quite
clear: If the guest wants to clear the flag and Xen thinks the flag is
clear, it could certainly allow the write. The security aspect here is
when the guest wants to clear the flag while Xen expects it to be set.

But a proper solution would of course be preferred. That may involve
notifying Xen of the reset from the PF driver.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel