[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] Re: Comments on Xen bug 1732


  • To: Jan Beulich <JBeulich@xxxxxxxxxx>
  • From: Haitao Shan <maillists.shan@xxxxxxxxx>
  • Date: Mon, 31 Jan 2011 16:52:42 +0800
  • Cc: Yunhong Jiang <yunhong.jiang@xxxxxxxxx>, Keir Fraser <keir@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxx
  • Delivery-date: Mon, 31 Jan 2011 00:53:35 -0800
  • Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; b=FqiLykcosYw4sIkz94owp9l0nj99vbvWAfyVA4hfu5gipRhJI4HpnmY/dBQ6MvQjqg i9rsljvn3rS+qF6hweGsGRUZGrv/ugeY/A/2nm6rMSV3CgWpGZdAG3OXTwnxI4+RGHCc vW1SK2sEf4+nHdaMh+K2NYJ21kEycPQv92Bv8=
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>



2011/1/31 Jan Beulich <JBeulich@xxxxxxxxxx>
>>> On 31.01.11 at 05:54, Haitao Shan <maillists.shan@xxxxxxxxx> wrote:
> Hi, Jan,
>
> As you may already notice the bug 1732, (
> http://bugzilla.xensource.com/bugzilla/show_bug.cgi?id=1732), the culprit is
> c/s 22182.
>
> I see the following attached code in your patch. It is pointless to check
> msi->table_base against the value read from physical device if this function
> is a virtual function of SR-IOV device. VFs are required to have BARs zeroed
> by specifications. And for VFs, unless you can read these values from
> corresponding PF, you will have to trust the "table_base" passed from dom0
> via hypercall. Actually, this parameter is specifically introduced for
> enabling SR-IOV.

Quite possible, which would perhaps just require removing some
or all of the warnings. In doing so, it must however be avoided to
introduce new ways for things to go bad silently.

> I am not familiar with this patch and hence its story. But I think it would
> be very simple for you to fix this up?

Not really, no. I had posted this patch as a draft after there was
no reaction on the part of the original implementers of the MSI and
pass-through code to address the security problem we're dealing
with here (and afaict the issue still wasn't completely addressed,
as I don't recall having seen corresponding adjustments to qemu).
I never had hardware to test this with, and hence had to rely on
Yunhong's testing and ack-ing of the patch.

> BTW: I vaguely recall that MSI-X table base might not be the first page of
> the corresponding BAR register.

While I agree that the code is lacking the use of
msix_table_offset_reg(), I would question what else would be
in the range supplied by the BAR, as the specification allows
only MSI-X table and PBA to share a BAR.

This is what I copied from PCI spec 3.0. I don't see that the spec only allows the two to be shared.
-----------------------------PCI----------
To enable system software to map MSI-X structures onto different processor pages for
improved access control, it is recommended that a function dedicate separate Base Address
registers for the MSI-X Table and MSI-X PBA, or else provide more than the minimum
required isolation with address ranges.
If dedicated separate Base Address registers is not feasible, it is recommended that a
function dedicate a single Base Address register for the MSI-X Table and MSI-X PBA.
If a dedicated Base Address register is not feasible, it is recommended that a function isolate
the MSI-X structures from the non-MSI-X structures with aligned 8 KB ranges rather than
the mandatory aligned 4 KB ranges. 
--------------------------spec---------------

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.