From: "Duyck, Alexander H" <alexander.h.duyck@intel.com>
To: "Wang, Liang-min" <liang-min.wang@intel.com>,
"alex.williamson@redhat.com" <alex.williamson@redhat.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Kirsher, Jeffrey T" <jeffrey.t.kirsher@intel.com>,
"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
"bhelgaas@google.com" <bhelgaas@google.com>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>
Subject: Re: [PATCH] Enable SR-IOV instantiation through /sys file
Date: Fri, 27 Oct 2017 23:20:41 +0000 [thread overview]
Message-ID: <1509146439.11655.60.camel@intel.com> (raw)
In-Reply-To: <20171028001907.7b8fa60d@t450s.home>
T24gU2F0LCAyMDE3LTEwLTI4IGF0IDAwOjE5ICswMjAwLCBBbGV4IFdpbGxpYW1zb24gd3JvdGU6
DQo+IE9uIEZyaSwgMjcgT2N0IDIwMTcgMjE6NTA6NDMgKzAwMDANCj4gIldhbmcsIExpYW5nLW1p
biIgPGxpYW5nLW1pbi53YW5nQGludGVsLmNvbT4gd3JvdGU6DQo+IA0KPiA+ID4gLS0tLS1Pcmln
aW5hbCBNZXNzYWdlLS0tLS0NCj4gPiA+IEZyb206IEFsZXggV2lsbGlhbXNvbiBbbWFpbHRvOmFs
ZXgud2lsbGlhbXNvbkByZWRoYXQuY29tXQ0KPiA+ID4gU2VudDogVHVlc2RheSwgT2N0b2JlciAy
NCwgMjAxNyA2OjA3IFBNDQo+ID4gPiBUbzogV2FuZywgTGlhbmctbWluIDxsaWFuZy1taW4ud2Fu
Z0BpbnRlbC5jb20+DQo+ID4gPiBDYzogS2lyc2hlciwgSmVmZnJleSBUIDxqZWZmcmV5LnQua2ly
c2hlckBpbnRlbC5jb20+OyBrdm1Admdlci5rZXJuZWwub3JnOw0KPiA+ID4gbGludXgtcGNpQHZn
ZXIua2VybmVsLm9yZzsgbGludXgta2VybmVsQHZnZXIua2VybmVsLm9yZzsNCj4gPiA+IGJoZWxn
YWFzQGdvb2dsZS5jb207IER1eWNrLCBBbGV4YW5kZXIgSCA8YWxleGFuZGVyLmguZHV5Y2tAaW50
ZWwuY29tPg0KPiA+ID4gU3ViamVjdDogUmU6IFtQQVRDSF0gRW5hYmxlIFNSLUlPViBpbnN0YW50
aWF0aW9uIHRocm91Z2ggL3N5cyBmaWxlDQo+ID4gPiANCj4gPiA+IE9uIFR1ZSwgMjQgT2N0IDIw
MTcgMjE6NDk6MTUgKzAwMDANCj4gPiA+ICJXYW5nLCBMaWFuZy1taW4iIDxsaWFuZy1taW4ud2Fu
Z0BpbnRlbC5jb20+IHdyb3RlOg0KPiA+ID4gICANCj4gPiA+ID4gSnVzdCBsaWtlIGFueSBQQ0ll
IGRldmljZXMgdGhhdCBzdXBwb3J0cyBTUi1JT1YuIFRoZXJlIGFyZSByZXN0cmljdGlvbnMgc2V0
IGZvciAgDQo+ID4gPiANCj4gPiA+IFZGLiBBbHNvLCB0aGVyZSBpcyBhIGNvbmNlcHQgb2YgdHJ1
c3QgVkYgbm93IGF2YWlsYWJsZSBmb3IgUEYgdG8gbWFuYWdlIGNlcnRhaW4NCj4gPiA+IGZlYXR1
cmVzIHRoYXQgb25seSBzZWxlY3RlZCBWRiBjb3VsZCBleGVyY2lzZS4gQXJlIHlvdSBzYXlpbmcg
YWxsIHRoZSBkZXZpY2VzDQo+ID4gPiBzdXBwb3J0aW5nIFNSLUlPViBhbGwgaGF2ZSBzZWN1cml0
eSBpc3N1ZT8NCj4gPiA+IA0KPiA+ID4gSGVyZSdzIGEgc2ltcGxlIGV4YW1wbGUsIG1vc3QgU1It
SU9WIGNhcGFibGUgTklDcywgaW5jbHVkaW5nIHRob3NlIGZyb20NCj4gPiA+IEludGVsLCByZXF1
aXJlIHRoZSBQRiBpbnRlcmZhY2UgdG8gYmUgdXAgaW4gb3JkZXIgdG8gcm91dGUgdHJhZmZpYyBm
cm9tDQo+ID4gPiB0aGUgVkYuICBJZiB0aGUgdXNlciBjb250cm9scyB0aGUgUEYgaW50ZXJmYWNl
IGFuZCBWRnMgYXJlIHVzZWQNCj4gPiA+IGVsc2V3aGVyZSBpbiB0aGUgaG9zdCwgdGhlIFBGIGRy
aXZlciBpbiB1c2Vyc3BhY2UgY2FuIGluZHVjZSBhIGRlbmlhbA0KPiA+ID4gb2Ygc2VydmljZSBv
biB0aGUgVkZzLiAgVGhhdCBkb2Vzbid0IGV2ZW4gdGFrZSBpbnRvIGFjY291bnQgdGhhdCBWRnMN
Cj4gPiA+IG1pZ2h0IGJlIGluIHNlcGFyYXRlIElPTU1VIGdyb3VwcyBmcm9tIHRoZSBQRiBhbmQg
dGhlcmVmb3JlIG5vdA0KPiA+ID4gaXNvbGF0ZWQgZnJvbSB0aGUgaG9zdCBsaWtlIHRoZSBQRiBh
bmQgdGhhdCB0aGUgUEYgZHJpdmVyIGNhbg0KPiA+ID4gcG90ZW50aWFsbHkgbWFuaXB1bGF0ZSB0
aGUgVkYsIHBvc3NpYmx5IHBlcmZvcm1pbmcgRE1BIG9uIGJlaGFsZiBvZiB0aGUNCj4gPiA+IFBG
LiAgVkZzIGFyZSBvbmx5IGNvbnNpZGVyZWQgc2VjdXJlIHRvZGF5IGJlY2F1c2UgdGhlIFBGIGlz
IG1hbmFnZWQgYnkNCj4gPiA+IGEgZHJpdmVyIGluIHRoZSBob3N0IGtlcm5lbC4gIEFsbG93aW5n
IHNpbXBsZSBlbmFibGVtZW50IG9mIFZGcyBmb3IgYQ0KPiA+ID4gdXNlciBvd25lZCBQRiBzZWVt
cyBpbmhlcmVudGx5IGluc2VjdXJlIHRvIG1lLiAgVGhhbmtzLA0KPiA+ID4gDQo+ID4gPiBBbGV4
ICANCj4gPiANCj4gPiBGaXJzdGx5LCB0aGUgY29uY2VybiBpcyBvbiB1c2VyLXNwYWNlIFBGIGRy
aXZlciBiYXNlZCB1cG9uIHZmaW8tcGNpLCB0aGlzIHBhdGNoIGRvZXNuJ3QNCj4gPiBjaGFuZ2Ug
UEYgYmVoYXZpb3Igc28gd2l0aC93aXRob3V0IHRoaXMgcGF0Y2gsIHRoZSBjb25jZXJuIHJlbWFp
bnMgdGhlIHNhbWUuDQo+IA0KPiBUaGlzIHBhdGNoIGVuYWJsZXMgU1ItSU9WIHRvIGJlIGVuYWJs
ZWQgdmlhIHRoZSBob3N0IG9uIGEgdXNlci1vd25lZA0KPiBQRiwgaG93IGlzIHRoaXMgbm90IGEg
Y2hhbmdlIGluIGJlaGF2aW9yPw0KPiANCj4gPiBTZWNvbmRseSwgdGhlIHNlY3VyaXR5IGNvbmNl
cm4gKGluY2x1ZGluZyBkZW5pYWwgb2Ygc2VydmljZSkgaW4gZ2VuZXJhbCBpcyB0byBlbnN1cmUg
dHJ1c3QNCj4gPiBlbnRpdHkgdG8gYmUgdHJ1c3Qtd29ydGh5LiBObyBtYXR0ZXIgdGhlIFBGIGRy
aXZlciBpcyBpbiBrZXJuZWwtc3BhY2Ugb3IgaW4gdXNlci0gc3BhY2UsDQo+ID4gbmVjZXNzYXJ5
IG1lY2hhbmlzbSBuZWVkcyB0byBiZSBlbmZvcmNlZCBvbiB0aGUgZGV2aWNlIGRyaXZlciB0byBl
bnN1cmUgaXQncw0KPiA+IHRydXN0ZWQgd29ydGh5LiBGb3IgZXhhbXBsZSwgaXhnYmUga2VybmVs
IGRyaXZlciBpbnRyb2R1Y2VzIGEgVHggaGFuZyBkZXRlY3Rpb24NCj4gPiB0byBhdm9pZCBkcml2
ZXIgc3RheXMgaW4gYSBiYWQgc3RhdGUuIFRoZXJlZm9yZSwgaXQncyB0aGUgcmVzcG9uc2liaWxp
dHkgb2YgdXNlci1zcGFjZQ0KPiA+IGRyaXZlciBmdW5jdGlvbiwgd2hpY2ggYmFzZWQgdXBvbiB2
ZmlvLXBjaSwgdG8gZW5mb3JjZSBuZWNlc3NhcnkgbWVjaGFuaXNtIHRvIGVuc3VyZQ0KPiA+IGl0
cyB0cnVzdC1uZXNzLiBUaGF0J3MgYSBnaXZlbi4NCj4gDQo+IFVzZXJzcGFjZSBpcyBub3QgdHJ1
c3R3b3J0aHksIHRoZXJlZm9yZSB0aGUgaG9zdCBrZXJuZWwgY2Fubm90IHBsYWNlDQo+IHJlc3Bv
bnNpYmlsaXR5IG9uIGEgdXNlcnNwYWNlIGRyaXZlciBmb3IgYW55dGhpbmcsIGluY2x1ZGluZyB0
aGUNCj4gYmVoYXZpb3Igb2YgVkZzLiAgSSdtIHNvcnJ5LCBidXQgaXQncyBhIE5BSyB1bmxlc3Mg
eW91IGludGVuZCB0bw0KPiBmb2xsb3ctdXAgd2l0aCBzb21lIHByb3Bvc2FsIHRvIHF1YXJhbnRp
bmUgdGhlIFZGcyBlbmFibGVkIGJ5IHRoZQ0KPiB1c2Vyc3BhY2UgUEYgZHJpdmVyLiAgVGhhbmtz
LA0KPiANCj4gQWxleA0KDQpJIGRvbid0IHNlZSB0aGlzIHNvIG11Y2ggYXMgYSBzZWN1cml0eSBw
cm9ibGVtIHBlci1zZS4gSXQgYWxsIGRlcGVuZHMNCm9uIHRoZSBoYXJkd2FyZSBzZXR1cC4gSWYg
SSByZWNhbGwgY29ycmVjdGx5LCB0aGVyZSBhcmUgZGV2aWNlcyB3aGVyZQ0KdGhlIFBGIGZ1bmN0
aW9uIGRvZXNuJ3QgcmVhbGx5IGRvIG11Y2ggb3RoZXIgdGhhbiBhY3QgYXMgYSBiaXQgbW9yZQ0K
aGVhdnktd2VpZ2h0IFZGLCBhbmQgdGhlIGFjdHVhbCBsb2dpYyBpcyBoYW5kbGVkIGJ5IGEgZmly
bXdhcmUgZW5naW5lDQpvbiB0aGUgZGV2aWNlLiBUaGUgb25seSByZWFsIGlzc3VlIGlzIHRoYXQg
Zm9yIGRldmljZXMgbGlrZSB0aGUgSW50ZWwNCk5JQ3MgaW5zdGVhZCBvZiB0cnVzdGluZyBhIGZp
cm13YXJlIGVuZ2luZSB3ZSBoYXZlIGhpc3RvcmljYWxseSB1c2VkIGENCmtlcm5lbCBkcml2ZXIg
YW5kIG5vdyB3ZSBhcmUgd2FudGluZyB0byB0cnVzdCBhIHVzZXItc3BhY2UgYWdlbnQNCmluc3Rl
YWQuDQoNCkkgZG8gdGhpbmsgdGhhdCB3ZSBwcm9iYWJseSBuZWVkIHRvIGhhdmUgc29tZSBzb3J0
IG9mIHNpZ25hbGluZyBiZXR3ZWVuDQp1c2VyLXNwYWNlIGFuZCB2ZmlvLXBjaSB0aGF0IHdvdWxk
IGFsbG93IGZvciBub3RpZnlpbmcgdGhlIHVzZXItc3BhY2UNCm9mIHRoZSBjaGFuZ2UgYW5kIGZv
ciB1c2VyLXNwYWNlIHRvIG5vdGlmeSB2ZmlvLXBjaSB0aGF0IGl0IGlzIGNhcGFibGUNCm9mIGhh
bmRsaW5nIHRoZSBub3RpZmljYXRpb24uIFRoaXMgaXMgc29tZXRoaW5nIHRoYXQgY2FuIGJlIHRv
Z2dsZWQgYXQNCmFueSB0aW1lIGFmdGVyIGFsbCBhbmQgbm90IGFsbCBkZXZpY2VzIGhhdmUgYSBt
ZWFucyBvZiBub3RpZnlpbmcgdGhlIFBGDQp0aGF0IHRoaXMgaGFzIGJlZW4gY2hhbmdlZC4NCg0K
QmV5b25kIHRoYXQgb25jZSB0aGUgcm9vdCB1c2VyIGVuYWJsZXMgdGhlIFZGcyBJIHdvdWxkIGtp
bmQgb2YgdGhpbmsNCnRoZXkga25vdyB3aGF0IGRyaXZlciB0aGV5IGhhdmUgcnVubmluZyB0aGVt
LiBFbmFibGluZyBWRnMgaW1wbGllcyB0aGUNCnJvb3QgdXNlciB0cnVzdHMgdGhlIGFwcGxpY2F0
aW9uIHJ1bm5pbmcgb24gdG9wIG9mIHZmaW8tcGNpIHRvIGhhbmRsZQ0KdGhlIFBGIHJlc3BvbnNp
Ymx5LiBBdCBsZWFzdCB0aGF0IGlzIGhvdyBpdCB3b3JrcyBpbiBteSBtaW5kLg0KDQpUaGFua3Mu
DQoNCi0gQWxleGFuZGVyDQogICh1c2luZyBmdWxsIG5hbWUgc2luY2UgMiBBbGV4cyBpbiBvbmUg
dGhyZWFkIGNhbiBiZSBjb25mdXNpbmcpDQoNCg==
WARNING: multiple messages have this Message-ID (diff)
From: "Duyck, Alexander H" <alexander.h.duyck@intel.com>
To: "Wang, Liang-min" <liang-min.wang@intel.com>,
"alex.williamson@redhat.com" <alex.williamson@redhat.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Kirsher, Jeffrey T" <jeffrey.t.kirsher@intel.com>,
"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
"bhelgaas@google.com" <bhelgaas@google.com>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>
Subject: Re: [PATCH] Enable SR-IOV instantiation through /sys file
Date: Fri, 27 Oct 2017 23:20:41 +0000 [thread overview]
Message-ID: <1509146439.11655.60.camel@intel.com> (raw)
In-Reply-To: <20171028001907.7b8fa60d@t450s.home>
On Sat, 2017-10-28 at 00:19 +0200, Alex Williamson wrote:
> On Fri, 27 Oct 2017 21:50:43 +0000
> "Wang, Liang-min" <liang-min.wang@intel.com> wrote:
>
> > > -----Original Message-----
> > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > Sent: Tuesday, October 24, 2017 6:07 PM
> > > To: Wang, Liang-min <liang-min.wang@intel.com>
> > > Cc: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; kvm@vger.kernel.org;
> > > linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > bhelgaas@google.com; Duyck, Alexander H <alexander.h.duyck@intel.com>
> > > Subject: Re: [PATCH] Enable SR-IOV instantiation through /sys file
> > >
> > > On Tue, 24 Oct 2017 21:49:15 +0000
> > > "Wang, Liang-min" <liang-min.wang@intel.com> wrote:
> > >
> > > > Just like any PCIe devices that supports SR-IOV. There are restrictions set for
> > >
> > > VF. Also, there is a concept of trust VF now available for PF to manage certain
> > > features that only selected VF could exercise. Are you saying all the devices
> > > supporting SR-IOV all have security issue?
> > >
> > > Here's a simple example, most SR-IOV capable NICs, including those from
> > > Intel, require the PF interface to be up in order to route traffic from
> > > the VF. If the user controls the PF interface and VFs are used
> > > elsewhere in the host, the PF driver in userspace can induce a denial
> > > of service on the VFs. That doesn't even take into account that VFs
> > > might be in separate IOMMU groups from the PF and therefore not
> > > isolated from the host like the PF and that the PF driver can
> > > potentially manipulate the VF, possibly performing DMA on behalf of the
> > > PF. VFs are only considered secure today because the PF is managed by
> > > a driver in the host kernel. Allowing simple enablement of VFs for a
> > > user owned PF seems inherently insecure to me. Thanks,
> > >
> > > Alex
> >
> > Firstly, the concern is on user-space PF driver based upon vfio-pci, this patch doesn't
> > change PF behavior so with/without this patch, the concern remains the same.
>
> This patch enables SR-IOV to be enabled via the host on a user-owned
> PF, how is this not a change in behavior?
>
> > Secondly, the security concern (including denial of service) in general is to ensure trust
> > entity to be trust-worthy. No matter the PF driver is in kernel-space or in user- space,
> > necessary mechanism needs to be enforced on the device driver to ensure it's
> > trusted worthy. For example, ixgbe kernel driver introduces a Tx hang detection
> > to avoid driver stays in a bad state. Therefore, it's the responsibility of user-space
> > driver function, which based upon vfio-pci, to enforce necessary mechanism to ensure
> > its trust-ness. That's a given.
>
> Userspace is not trustworthy, therefore the host kernel cannot place
> responsibility on a userspace driver for anything, including the
> behavior of VFs. I'm sorry, but it's a NAK unless you intend to
> follow-up with some proposal to quarantine the VFs enabled by the
> userspace PF driver. Thanks,
>
> Alex
I don't see this so much as a security problem per-se. It all depends
on the hardware setup. If I recall correctly, there are devices where
the PF function doesn't really do much other than act as a bit more
heavy-weight VF, and the actual logic is handled by a firmware engine
on the device. The only real issue is that for devices like the Intel
NICs instead of trusting a firmware engine we have historically used a
kernel driver and now we are wanting to trust a user-space agent
instead.
I do think that we probably need to have some sort of signaling between
user-space and vfio-pci that would allow for notifying the user-space
of the change and for user-space to notify vfio-pci that it is capable
of handling the notification. This is something that can be toggled at
any time after all and not all devices have a means of notifying the PF
that this has been changed.
Beyond that once the root user enables the VFs I would kind of think
they know what driver they have running them. Enabling VFs implies the
root user trusts the application running on top of vfio-pci to handle
the PF responsibly. At least that is how it works in my mind.
Thanks.
- Alexander
(using full name since 2 Alexs in one thread can be confusing)
next prev parent reply other threads:[~2017-10-27 23:20 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-24 20:04 [PATCH] Enable SR-IOV instantiation through /sys file Jeff Kirsher
2017-10-24 21:43 ` Alex Williamson
2017-10-24 21:49 ` Wang, Liang-min
2017-10-24 22:06 ` Alex Williamson
2017-10-24 22:29 ` Wang, Liang-min
2017-10-25 8:39 ` Alex Williamson
2017-10-27 21:50 ` Wang, Liang-min
2017-10-27 22:19 ` Alex Williamson
2017-10-27 22:30 ` Wang, Liang-min
2017-10-27 23:20 ` Duyck, Alexander H [this message]
2017-10-27 23:20 ` Duyck, Alexander H
2017-10-29 6:16 ` Christoph Hellwig
2017-10-29 21:12 ` Alexander Duyck
2017-10-30 12:39 ` David Woodhouse
2017-10-31 12:55 ` Wang, Liang-min
2017-10-31 12:55 ` Wang, Liang-min
2017-11-06 23:27 ` Alex Williamson
2017-11-06 23:47 ` Alexander Duyck
2017-11-07 16:59 ` Alex Williamson
2017-11-06 19:55 ` Bjorn Helgaas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1509146439.11655.60.camel@intel.com \
--to=alexander.h.duyck@intel.com \
--cc=alex.williamson@redhat.com \
--cc=bhelgaas@google.com \
--cc=jeffrey.t.kirsher@intel.com \
--cc=kvm@vger.kernel.org \
--cc=liang-min.wang@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.