[PATCH] libxl: passthrough: avoid passing through devices not owned by pciback

[PATCH] libxl: passthrough: avoid passing through devices not owned by pciback

[Hao, Xudong]

<Porting from Xen 4.1 tree.>

libxl: passthrough: avoid passing through devices not owned by pciback

This patch makes sure the passthrough device belongs to pciback before allow them passthrough to the guest.  There are still many other checks missing.

xm terminates the guest startup process when this type of condition is found.  This patch just allows the guest to continue to boot but with no device passthrough.

Signed-off-by: Allen Kay <allen.m.kay@intel.com>
Signed-off-by: Xudong Hao <xudong.hao@intel.com>

diff -r 4e1d091d10d8 tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c	Fri Mar 16 15:24:25 2012 +0000
+++ b/tools/libxl/libxl_pci.c	Thu Mar 22 00:43:14 2012 +0800
@@ -779,6 +779,24 @@ int libxl_device_pci_add(libxl_ctx *ctx,
     return rc;
 }
 
+static int libxl_pcidev_assignable(libxl_ctx *ctx, libxl_device_pci 
+*pcidev) {
+    libxl_device_pci *pcidevs;
+    int num, i;
+
+    pcidevs = libxl_device_pci_list_assignable(ctx, &num);
+    for (i = 0; i < num; i++) {
+        if (pcidevs[i].domain == pcidev->domain &&
+            pcidevs[i].bus == pcidev->bus &&
+            pcidevs[i].dev == pcidev->dev &&
+            pcidevs[i].func == pcidev->func)
+        {
+            return 1;
+        }
+    }
+    return 0;
+}
+
 int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting)  {
     libxl_ctx *ctx = libxl__gc_owner(gc); @@ -789,6 +807,13 @@ int libxl__device_pci_add(libxl__gc *gc,
 
     rc = libxl__device_pci_setdefault(gc, pcidev);
     if (rc) goto out;
+
+    if (!libxl_pcidev_assignable(ctx, pcidev)) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "PCI device %x:%x:%x.%x is not assignable",
+                   pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+        rc = ERROR_FAIL;
+        goto out;
+    }
 
     rc = get_all_assigned_devices(gc, &assigned, &num_assigned);
     if ( rc ) {

Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback

[George Dunlap]

On Thu, Mar 22, 2012 at 1:20 AM, Hao, Xudong <xudong.hao@intel.com> wrote:
> <Porting from Xen 4.1 tree.>
>
> libxl: passthrough: avoid passing through devices not owned by pciback
>
> This patch makes sure the passthrough device belongs to pciback before allow them passthrough to the guest.  There are still many other checks missing.
>
> xm terminates the guest startup process when this type of condition is found.  This patch just allows the guest to continue to boot but with no device passthrough.

Thanks for the patch.  My one comment is that I think that xm's
behavior is the correct one.  If I tell the computer "Do X with
feature Y", I want the computer to either do X with feature Y, or to
say, "I'm sorry, feature Y is not available."  Then I can figure out
if I want X without Y, or whether I want to fix things so Y is
available.  Saying, "Y is not available so I'll just to X" is rarely
the right thing.

On a different (but related) note, it appears that libxl doesn't have
the pci-quirks functionality that xend has.  Do you know if this is
still needed, if it's in the pvops tree and needs to be upstreamed?
Is anyone at Intel working on libxl pci-quirks support for the 4.2
release?

 -George

>
> Signed-off-by: Allen Kay <allen.m.kay@intel.com>
> Signed-off-by: Xudong Hao <xudong.hao@intel.com>
>
> diff -r 4e1d091d10d8 tools/libxl/libxl_pci.c
> --- a/tools/libxl/libxl_pci.c   Fri Mar 16 15:24:25 2012 +0000
> +++ b/tools/libxl/libxl_pci.c   Thu Mar 22 00:43:14 2012 +0800
> @@ -779,6 +779,24 @@ int libxl_device_pci_add(libxl_ctx *ctx,
>     return rc;
>  }
>
> +static int libxl_pcidev_assignable(libxl_ctx *ctx, libxl_device_pci
> +*pcidev) {
> +    libxl_device_pci *pcidevs;
> +    int num, i;
> +
> +    pcidevs = libxl_device_pci_list_assignable(ctx, &num);
> +    for (i = 0; i < num; i++) {
> +        if (pcidevs[i].domain == pcidev->domain &&
> +            pcidevs[i].bus == pcidev->bus &&
> +            pcidevs[i].dev == pcidev->dev &&
> +            pcidevs[i].func == pcidev->func)
> +        {
> +            return 1;
> +        }
> +    }
> +    return 0;
> +}
> +
>  int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting)  {
>     libxl_ctx *ctx = libxl__gc_owner(gc); @@ -789,6 +807,13 @@ int libxl__device_pci_add(libxl__gc *gc,
>
>     rc = libxl__device_pci_setdefault(gc, pcidev);
>     if (rc) goto out;
> +
> +    if (!libxl_pcidev_assignable(ctx, pcidev)) {
> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "PCI device %x:%x:%x.%x is not assignable",
> +                   pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
>
>     rc = get_all_assigned_devices(gc, &assigned, &num_assigned);
>     if ( rc ) {
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback

[Hao, Xudong]

> -----Original Message-----
> From: dunlapg@gmail.com [mailto:dunlapg@gmail.com] On Behalf Of George
> Dunlap
> Sent: Thursday, March 22, 2012 6:16 PM
> To: Hao, Xudong
> Cc: ian.jackson@eu.citrix.com; xen-devel@lists.xensource.com; Kay, Allen M
> Subject: Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
> devices not owned by pciback
> 
> On Thu, Mar 22, 2012 at 1:20 AM, Hao, Xudong <xudong.hao@intel.com>
> wrote:
> > <Porting from Xen 4.1 tree.>
> >
> > libxl: passthrough: avoid passing through devices not owned by pciback
> >
> > This patch makes sure the passthrough device belongs to pciback before
> allow them passthrough to the guest.  There are still many other checks
> missing.
> >
> > xm terminates the guest startup process when this type of condition is
> found.  This patch just allows the guest to continue to boot but with no device
> passthrough.
> 
> Thanks for the patch.  My one comment is that I think that xm's behavior is
> the correct one.  If I tell the computer "Do X with feature Y", I want the
> computer to either do X with feature Y, or to say, "I'm sorry, feature Y is not
> available."  Then I can figure out if I want X without Y, or whether I want to fix
> things so Y is available.  Saying, "Y is not available so I'll just to X" is rarely the
> right thing.
> 
George, your theory is correct, but I think it's not much suitable for this patch. This patch just add an additional checking, it return ERROR_FAIL to its call function, but it can't control the behavior by the result of upper function libxl_device_pci_add().
 
> On a different (but related) note, it appears that libxl doesn't have the
> pci-quirks functionality that xend has.  Do you know if this is still needed, if it's
> in the pvops tree and needs to be upstreamed?
> Is anyone at Intel working on libxl pci-quirks support for the 4.2 release?
> 
We did not see any Intel pci device need quirk in libxl till now, so maybe no plan for libxl pci-quirks support for the 4.2 release.

>  -George
> 
> >
> > Signed-off-by: Allen Kay <allen.m.kay@intel.com>
> > Signed-off-by: Xudong Hao <xudong.hao@intel.com>
> >
> > diff -r 4e1d091d10d8 tools/libxl/libxl_pci.c
> > --- a/tools/libxl/libxl_pci.c   Fri Mar 16 15:24:25 2012 +0000
> > +++ b/tools/libxl/libxl_pci.c   Thu Mar 22 00:43:14 2012 +0800
> > @@ -779,6 +779,24 @@ int libxl_device_pci_add(libxl_ctx *ctx,
> >     return rc;
> >  }
> >
> > +static int libxl_pcidev_assignable(libxl_ctx *ctx, libxl_device_pci
> > +*pcidev) {
> > +    libxl_device_pci *pcidevs;
> > +    int num, i;
> > +
> > +    pcidevs = libxl_device_pci_list_assignable(ctx, &num);
> > +    for (i = 0; i < num; i++) {
> > +        if (pcidevs[i].domain == pcidev->domain &&
> > +            pcidevs[i].bus == pcidev->bus &&
> > +            pcidevs[i].dev == pcidev->dev &&
> > +            pcidevs[i].func == pcidev->func)
> > +        {
> > +            return 1;
> > +        }
> > +    }
> > +    return 0;
> > +}
> > +
> >  int libxl__device_pci_add(libxl__gc *gc, uint32_t domid,
> > libxl_device_pci *pcidev, int starting)  {
> >     libxl_ctx *ctx = libxl__gc_owner(gc); @@ -789,6 +807,13 @@ int
> > libxl__device_pci_add(libxl__gc *gc,
> >
> >     rc = libxl__device_pci_setdefault(gc, pcidev);
> >     if (rc) goto out;
> > +
> > +    if (!libxl_pcidev_assignable(ctx, pcidev)) {
> > +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "PCI device %x:%x:%x.%x
> is
> > + not assignable",
> > +                   pcidev->domain, pcidev->bus, pcidev->dev,
> > + pcidev->func);
> > +        rc = ERROR_FAIL;
> > +        goto out;
> > +    }
> >
> >     rc = get_all_assigned_devices(gc, &assigned, &num_assigned);
> >     if ( rc ) {
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel

Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback

[Ian Jackson]

Hao, Xudong writes ("RE: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback"):
> We did not see any Intel pci device need quirk in libxl till now, so maybe no plan for libxl pci-quirks support for the 4.2 release.

OK.  The rest of the patch looks sensible (although the code
formatting isn't ideal).

Ian.

Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback

[Hao, Xudong]

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Tuesday, March 27, 2012 10:00 PM
> To: Hao, Xudong
> Cc: George Dunlap; xen-devel@lists.xensource.com; Kay, Allen M
> Subject: RE: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
> devices not owned by pciback
> 
> Hao, Xudong writes ("RE: [Xen-devel] [PATCH] libxl: passthrough: avoid passing
> through devices not owned by pciback"):
> > We did not see any Intel pci device need quirk in libxl till now, so maybe no
> plan for libxl pci-quirks support for the 4.2 release.
> 
> OK.  The rest of the patch looks sensible (although the code formatting isn't
> ideal).
>
Ian, 

Do you want me to rewrite a patch with correct code formatting, or check this patch in?
 
> Ian.

Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback

[George Dunlap]

On Fri, Mar 23, 2012 at 2:47 AM, Hao, Xudong <xudong.hao@intel.com> wrote:
>> On a different (but related) note, it appears that libxl doesn't have the
>> pci-quirks functionality that xend has.  Do you know if this is still needed, if it's
>> in the pvops tree and needs to be upstreamed?
>> Is anyone at Intel working on libxl pci-quirks support for the 4.2 release?
>>
> We did not see any Intel pci device need quirk in libxl till now, so maybe no plan for libxl pci-quirks support for the 4.2 release.

FWIW, I did see a quirk on the following Intel NIC (don't have the
details of the warning message handy):

07:00.0 Ethernet controller [0200]: Intel Corporation 82575GB Gigabit
Network Connection [8086:10d6] (rev 02)

But given the proximity to the 4.2 release, it would make perfect
sense to wait for 4.3.  I'm going to try to implement a per-device
"permissive" feature in libxl which should tide us over.

 -George

>
>>  -George
>>
>> >
>> > Signed-off-by: Allen Kay <allen.m.kay@intel.com>
>> > Signed-off-by: Xudong Hao <xudong.hao@intel.com>
>> >
>> > diff -r 4e1d091d10d8 tools/libxl/libxl_pci.c
>> > --- a/tools/libxl/libxl_pci.c   Fri Mar 16 15:24:25 2012 +0000
>> > +++ b/tools/libxl/libxl_pci.c   Thu Mar 22 00:43:14 2012 +0800
>> > @@ -779,6 +779,24 @@ int libxl_device_pci_add(libxl_ctx *ctx,
>> >     return rc;
>> >  }
>> >
>> > +static int libxl_pcidev_assignable(libxl_ctx *ctx, libxl_device_pci
>> > +*pcidev) {
>> > +    libxl_device_pci *pcidevs;
>> > +    int num, i;
>> > +
>> > +    pcidevs = libxl_device_pci_list_assignable(ctx, &num);
>> > +    for (i = 0; i < num; i++) {
>> > +        if (pcidevs[i].domain == pcidev->domain &&
>> > +            pcidevs[i].bus == pcidev->bus &&
>> > +            pcidevs[i].dev == pcidev->dev &&
>> > +            pcidevs[i].func == pcidev->func)
>> > +        {
>> > +            return 1;
>> > +        }
>> > +    }
>> > +    return 0;
>> > +}
>> > +
>> >  int libxl__device_pci_add(libxl__gc *gc, uint32_t domid,
>> > libxl_device_pci *pcidev, int starting)  {
>> >     libxl_ctx *ctx = libxl__gc_owner(gc); @@ -789,6 +807,13 @@ int
>> > libxl__device_pci_add(libxl__gc *gc,
>> >
>> >     rc = libxl__device_pci_setdefault(gc, pcidev);
>> >     if (rc) goto out;
>> > +
>> > +    if (!libxl_pcidev_assignable(ctx, pcidev)) {
>> > +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "PCI device %x:%x:%x.%x
>> is
>> > + not assignable",
>> > +                   pcidev->domain, pcidev->bus, pcidev->dev,
>> > + pcidev->func);
>> > +        rc = ERROR_FAIL;
>> > +        goto out;
>> > +    }
>> >
>> >     rc = get_all_assigned_devices(gc, &assigned, &num_assigned);
>> >     if ( rc ) {
>> >
>> > _______________________________________________
>> > Xen-devel mailing list
>> > Xen-devel@lists.xen.org
>> > http://lists.xen.org/xen-devel
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback

[Hao, Xudong]

> -----Original Message-----
> From: dunlapg@gmail.com [mailto:dunlapg@gmail.com] On Behalf Of George
> Dunlap
> Sent: Tuesday, March 27, 2012 10:42 PM
> To: Hao, Xudong
> Cc: Kay, Allen M; xen-devel@lists.xensource.com; ian.jackson@eu.citrix.com
> Subject: Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
> devices not owned by pciback
> 
> On Fri, Mar 23, 2012 at 2:47 AM, Hao, Xudong <xudong.hao@intel.com>
> wrote:
> >> On a different (but related) note, it appears that libxl doesn't have
> >> the pci-quirks functionality that xend has.  Do you know if this is
> >> still needed, if it's in the pvops tree and needs to be upstreamed?
> >> Is anyone at Intel working on libxl pci-quirks support for the 4.2 release?
> >>
> > We did not see any Intel pci device need quirk in libxl till now, so maybe no
> plan for libxl pci-quirks support for the 4.2 release.
> 
> FWIW, I did see a quirk on the following Intel NIC (don't have the details of the
> warning message handy):
> 
> 07:00.0 Ethernet controller [0200]: Intel Corporation 82575GB Gigabit Network
> Connection [8086:10d6] (rev 02)
> 
> But given the proximity to the 4.2 release, it would make perfect sense to wait
> for 4.3.  I'm going to try to implement a per-device "permissive" feature in
> libxl which should tide us over.
>
That's great, looking forward to your new feature. 

Thanks,
-Xudong
 
>  -George
> 
> >
> >>  -George
> >>
> >> >
> >> > Signed-off-by: Allen Kay <allen.m.kay@intel.com>
> >> > Signed-off-by: Xudong Hao <xudong.hao@intel.com>
> >> >
> >> > diff -r 4e1d091d10d8 tools/libxl/libxl_pci.c
> >> > --- a/tools/libxl/libxl_pci.c   Fri Mar 16 15:24:25 2012 +0000
> >> > +++ b/tools/libxl/libxl_pci.c   Thu Mar 22 00:43:14 2012 +0800
> >> > @@ -779,6 +779,24 @@ int libxl_device_pci_add(libxl_ctx *ctx,
> >> >     return rc;
> >> >  }
> >> >
> >> > +static int libxl_pcidev_assignable(libxl_ctx *ctx,
> >> > +libxl_device_pci
> >> > +*pcidev) {
> >> > +    libxl_device_pci *pcidevs;
> >> > +    int num, i;
> >> > +
> >> > +    pcidevs = libxl_device_pci_list_assignable(ctx, &num);
> >> > +    for (i = 0; i < num; i++) {
> >> > +        if (pcidevs[i].domain == pcidev->domain &&
> >> > +            pcidevs[i].bus == pcidev->bus &&
> >> > +            pcidevs[i].dev == pcidev->dev &&
> >> > +            pcidevs[i].func == pcidev->func)
> >> > +        {
> >> > +            return 1;
> >> > +        }
> >> > +    }
> >> > +    return 0;
> >> > +}
> >> > +
> >> >  int libxl__device_pci_add(libxl__gc *gc, uint32_t domid,
> >> > libxl_device_pci *pcidev, int starting)  {
> >> >     libxl_ctx *ctx = libxl__gc_owner(gc); @@ -789,6 +807,13 @@ int
> >> > libxl__device_pci_add(libxl__gc *gc,
> >> >
> >> >     rc = libxl__device_pci_setdefault(gc, pcidev);
> >> >     if (rc) goto out;
> >> > +
> >> > +    if (!libxl_pcidev_assignable(ctx, pcidev)) {
> >> > +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "PCI
> device %x:%x:%x.%x
> >> is
> >> > + not assignable",
> >> > +                   pcidev->domain, pcidev->bus, pcidev->dev,
> >> > + pcidev->func);
> >> > +        rc = ERROR_FAIL;
> >> > +        goto out;
> >> > +    }
> >> >
> >> >     rc = get_all_assigned_devices(gc, &assigned, &num_assigned);
> >> >     if ( rc ) {
> >> >
> >> > _______________________________________________
> >> > Xen-devel mailing list
> >> > Xen-devel@lists.xen.org
> >> > http://lists.xen.org/xen-devel
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel

Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback

[Ian Jackson]

Hao, Xudong writes ("[Xen-devel] [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback"):
> <Porting from Xen 4.1 tree.>
> 
> libxl: passthrough: avoid passing through devices not owned by pciback

I'm afraid this no longer applies to xen-unstable.hg tip.

Ian.

Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback

[Hao, Xudong]

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Tuesday, April 03, 2012 1:01 AM
> To: Hao, Xudong
> Cc: xen-devel@lists.xensource.com; Kay, Allen M
> Subject: Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
> devices not owned by pciback
> 
> Hao, Xudong writes ("[Xen-devel] [PATCH] libxl: passthrough: avoid passing
> through devices not owned by pciback"):
> > <Porting from Xen 4.1 tree.>
> >
> > libxl: passthrough: avoid passing through devices not owned by pciback
> 
> I'm afraid this no longer applies to xen-unstable.hg tip.
> 
Reason?

If no pciback checking, one device could be assigned to guest even it's being used by dom0, is there security issue?

Thanks,
-Xudong

Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback

[George Dunlap]

On Thu, Apr 5, 2012 at 2:13 AM, Hao, Xudong <xudong.hao@intel.com> wrote:
>
>> -----Original Message-----
>> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
>> Sent: Tuesday, April 03, 2012 1:01 AM
>> To: Hao, Xudong
>> Cc: xen-devel@lists.xensource.com; Kay, Allen M
>> Subject: Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
>> devices not owned by pciback
>>
>> Hao, Xudong writes ("[Xen-devel] [PATCH] libxl: passthrough: avoid passing
>> through devices not owned by pciback"):
>> > <Porting from Xen 4.1 tree.>
>> >
>> > libxl: passthrough: avoid passing through devices not owned by pciback
>>
>> I'm afraid this no longer applies to xen-unstable.hg tip.
>>
> Reason?
>
> If no pciback checking, one device could be assigned to guest even it's being used by dom0, is there security issue?

I think Ian is saying that the actual patch itself doesn't apply. That is,

$ patch -p1 < passthru.diff

fails; so he's asking you to send a new version of the patch.

 -George

Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback

[Ian Jackson]

Hao, Xudong writes ("RE: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback"):
> 
> > -----Original Message-----
> > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> > Sent: Tuesday, April 03, 2012 1:01 AM
> > To: Hao, Xudong
> > Cc: xen-devel@lists.xensource.com; Kay, Allen M
> > Subject: Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
> > devices not owned by pciback
> > 
> > Hao, Xudong writes ("[Xen-devel] [PATCH] libxl: passthrough: avoid passing
> > through devices not owned by pciback"):
> > > <Porting from Xen 4.1 tree.>
> > >
> > > libxl: passthrough: avoid passing through devices not owned by pciback
> > 
> > I'm afraid this no longer applies to xen-unstable.hg tip.
> > 
> Reason?
> 
> If no pciback checking, one device could be assigned to guest even it's being used by dom0, is there security issue?

I mean that it has conflicts when I try to apply it.  You need to
refresh it.

Thanks,
Ian.

Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback

[Hao, Xudong]

<Porting from xen 4.1, patch on Xen unstable 25138>

libxl: passthrough: avoid passing through devices not owned by pciback

This patch makes sure the passthrough device belongs to pciback before allow them passthrough to the guest.  There are still many other checks missing.

xm terminates the guest startup process when this type of condition is found.  This patch just allows the guest to continue to boot but with no device passthrough.

Signed-off-by: Allen Kay <allen.m.kay@intel.com>
Signed-off-by: Xudong Hao <xudong.hao@intel.com>

diff -r 4e1d091d10d8 tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c	Fri Mar 16 15:24:25 2012 +0000
+++ b/tools/libxl/libxl_pci.c	Thu Mar 22 00:43:14 2012 +0800
@@ -779,6 +779,24 @@ int libxl_device_pci_add(libxl_ctx *ctx,
     return rc;
 }
 
+static int libxl_pcidev_assignable(libxl_ctx *ctx, libxl_device_pci 
+*pcidev) {
+    libxl_device_pci *pcidevs;
+    int num, i;
+
+    pcidevs = libxl_device_pci_list_assignable(ctx, &num);
+    for (i = 0; i < num; i++) {
+        if (pcidevs[i].domain == pcidev->domain &&
+            pcidevs[i].bus == pcidev->bus &&
+            pcidevs[i].dev == pcidev->dev &&
+            pcidevs[i].func == pcidev->func)
+        {
+            return 1;
+        }
+    }
+    return 0;
+}
+
 int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting)  {
     libxl_ctx *ctx = libxl__gc_owner(gc); @@ -789,6 +807,13 @@ int libxl__device_pci_add(libxl__gc *gc,
 
     rc = libxl__device_pci_setdefault(gc, pcidev);
     if (rc) goto out;
+
+    if (!libxl_pcidev_assignable(ctx, pcidev)) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "PCI device %x:%x:%x.%x is not assignable",
+                   pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+        rc = ERROR_FAIL;
+        goto out;
+    }
 
     rc = get_all_assigned_devices(gc, &assigned, &num_assigned);
     if ( rc ) {

Thanks,
-Xudong

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Thursday, April 05, 2012 10:42 PM
> To: Hao, Xudong
> Cc: xen-devel@lists.xensource.com; Kay, Allen M
> Subject: RE: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
> devices not owned by pciback
> 
> Hao, Xudong writes ("RE: [Xen-devel] [PATCH] libxl: passthrough: avoid passing
> through devices not owned by pciback"):
> >
> > > -----Original Message-----
> > > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> > > Sent: Tuesday, April 03, 2012 1:01 AM
> > > To: Hao, Xudong
> > > Cc: xen-devel@lists.xensource.com; Kay, Allen M
> > > Subject: Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing
> > > through devices not owned by pciback
> > >
> > > Hao, Xudong writes ("[Xen-devel] [PATCH] libxl: passthrough: avoid
> > > passing through devices not owned by pciback"):
> > > > <Porting from Xen 4.1 tree.>
> > > >
> > > > libxl: passthrough: avoid passing through devices not owned by
> > > > pciback
> > >
> > > I'm afraid this no longer applies to xen-unstable.hg tip.
> > >
> > Reason?
> >
> > If no pciback checking, one device could be assigned to guest even it's being
> used by dom0, is there security issue?
> 
> I mean that it has conflicts when I try to apply it.  You need to refresh it.
> 
> Thanks,
> Ian.

Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback

[Ian Jackson]

Hao, Xudong writes ("Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback"):
> <Porting from xen 4.1, patch on Xen unstable 25138>

I'm afraid that 25138 was out of date even when you posted your
revised patch, and it still doesn't apply.

Are you working with the staging tree ?

Ian.

Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback

[Hao, Xudong]

I work on unstable tree.

I think the confliction is my MS outlook configuration issue, I'll re-resend a revised patch.

Thanks,
-Xudong

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Wednesday, April 25, 2012 2:03 AM
> To: Hao, Xudong
> Cc: xen-devel@lists.xensource.com; Kay, Allen M
> Subject: Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
> devices not owned by pciback
> 
> Hao, Xudong writes ("Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing
> through devices not owned by pciback"):
> > <Porting from xen 4.1, patch on Xen unstable 25138>
> 
> I'm afraid that 25138 was out of date even when you posted your revised patch,
> and it still doesn't apply.
> 
> Are you working with the staging tree ?
> 
> Ian.

Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback

[Ian Jackson]

Hao, Xudong writes ("RE: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback"):
> I work on unstable tree.

xen-unstable.hg comes in two variants.  See the text under
  http://wiki.xen.org/wiki/Submitting_Xen_Patches#After_your_patch_is_committed

> I think the confliction is my MS outlook configuration issue, I'll re-resend a revised patch.

I think it's because you used an out of date tip.  You based your
patch on 25138, according to your email.  But at the time you sent
your email, the most recent changeset was something later than 25156,
and 25156 changed libcl_pci.c.

But I will try your repost.

Ian.

Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback

[Hao, Xudong]

<RESEND: Porting from xen 4.1, patch on Xen unstable 25220>

libxl: passthrough: avoid passing through devices not owned by pciback

This patch makes sure the passthrough device belongs to pciback before allow them passthrough to the guest.  There are still many other checks missing.

xm terminates the guest startup process when this type of condition is found.  This patch just allows the guest to continue to boot but with no device passthrough.

Signed-off-by: Allen Kay <allen.m.kay@intel.com>
Signed-off-by: Xudong Hao <xudong.hao@intel.com>

diff -r a06e6cdeafe3 tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c	Mon Apr 16 13:05:28 2012 +0200
+++ b/tools/libxl/libxl_pci.c	Wed Apr 17 17:02:25 2013 +0800
@@ -664,6 +664,24 @@ int libxl_device_pci_add(libxl_ctx *ctx,
     return rc;
 }
 
+static int libxl_pcidev_assignable(libxl_ctx *ctx, libxl_device_pci *pcidev)
+{
+    libxl_device_pci *pcidevs;
+    int num, i;
+
+    pcidevs = libxl_device_pci_list_assignable(ctx, &num);
+    for (i = 0; i < num; i++) {
+        if (pcidevs[i].domain == pcidev->domain &&
+            pcidevs[i].bus == pcidev->bus &&
+            pcidevs[i].dev == pcidev->dev &&
+            pcidevs[i].func == pcidev->func)
+        {
+            return 1;
+        }
+    }
+    return 0;
+}
+
 int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -674,6 +692,13 @@ int libxl__device_pci_add(libxl__gc *gc,
 
     rc = libxl__device_pci_setdefault(gc, pcidev);
     if (rc) goto out;
+
+    if (!libxl_pcidev_assignable(ctx, pcidev)) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "PCI device %x:%x:%x.%x is not assignable",
+                   pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+        rc = ERROR_FAIL;
+        goto out;
+    }
 
     rc = get_all_assigned_devices(gc, &assigned, &num_assigned);
     if ( rc ) {


Thanks,
-Xudong


> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Wednesday, April 25, 2012 2:03 AM
> To: Hao, Xudong
> Cc: xen-devel@lists.xensource.com; Kay, Allen M
> Subject: Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
> devices not owned by pciback
> 
> Hao, Xudong writes ("Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing
> through devices not owned by pciback"):
> > <Porting from xen 4.1, patch on Xen unstable 25138>
> 
> I'm afraid that 25138 was out of date even when you posted your
> revised patch, and it still doesn't apply.
> 
> Are you working with the staging tree ?
> 
> Ian.

Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback

[Ian Jackson]

Hao, Xudong writes ("Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback"):
> libxl: passthrough: avoid passing through devices not owned by pciback

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>

Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback

[Hao, Xudong]

Hi, Ian

Any other comments for this patch?

Thanks,
-Xudong


> -----Original Message-----
> From: Hao, Xudong
> Sent: Thursday, April 05, 2012 11:37 PM
> To: 'Ian Jackson'
> Cc: xen-devel@lists.xensource.com; Kay, Allen M
> Subject: RE: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
> devices not owned by pciback
> 
> <Porting from xen 4.1, patch on Xen unstable 25138>
> 
> libxl: passthrough: avoid passing through devices not owned by pciback
> 
> This patch makes sure the passthrough device belongs to pciback before allow
> them passthrough to the guest.  There are still many other checks missing.
> 
> xm terminates the guest startup process when this type of condition is found.
> This patch just allows the guest to continue to boot but with no device
> passthrough.
> 
> Signed-off-by: Allen Kay <allen.m.kay@intel.com>
> Signed-off-by: Xudong Hao <xudong.hao@intel.com>
> 
> diff -r 4e1d091d10d8 tools/libxl/libxl_pci.c
> --- a/tools/libxl/libxl_pci.c	Fri Mar 16 15:24:25 2012 +0000
> +++ b/tools/libxl/libxl_pci.c	Thu Mar 22 00:43:14 2012 +0800
> @@ -779,6 +779,24 @@ int libxl_device_pci_add(libxl_ctx *ctx,
>      return rc;
>  }
> 
> +static int libxl_pcidev_assignable(libxl_ctx *ctx, libxl_device_pci
> +*pcidev) {
> +    libxl_device_pci *pcidevs;
> +    int num, i;
> +
> +    pcidevs = libxl_device_pci_list_assignable(ctx, &num);
> +    for (i = 0; i < num; i++) {
> +        if (pcidevs[i].domain == pcidev->domain &&
> +            pcidevs[i].bus == pcidev->bus &&
> +            pcidevs[i].dev == pcidev->dev &&
> +            pcidevs[i].func == pcidev->func)
> +        {
> +            return 1;
> +        }
> +    }
> +    return 0;
> +}
> +
>  int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci
> *pcidev, int starting)  {
>      libxl_ctx *ctx = libxl__gc_owner(gc); @@ -789,6 +807,13 @@ int
> libxl__device_pci_add(libxl__gc *gc,
> 
>      rc = libxl__device_pci_setdefault(gc, pcidev);
>      if (rc) goto out;
> +
> +    if (!libxl_pcidev_assignable(ctx, pcidev)) {
> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "PCI device %x:%x:%x.%x is
> not assignable",
> +                   pcidev->domain, pcidev->bus, pcidev->dev,
> pcidev->func);
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> 
>      rc = get_all_assigned_devices(gc, &assigned, &num_assigned);
>      if ( rc ) {
> 
> Thanks,
> -Xudong
> 
> > -----Original Message-----
> > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> > Sent: Thursday, April 05, 2012 10:42 PM
> > To: Hao, Xudong
> > Cc: xen-devel@lists.xensource.com; Kay, Allen M
> > Subject: RE: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
> > devices not owned by pciback
> >
> > Hao, Xudong writes ("RE: [Xen-devel] [PATCH] libxl: passthrough: avoid
> passing
> > through devices not owned by pciback"):
> > >
> > > > -----Original Message-----
> > > > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> > > > Sent: Tuesday, April 03, 2012 1:01 AM
> > > > To: Hao, Xudong
> > > > Cc: xen-devel@lists.xensource.com; Kay, Allen M
> > > > Subject: Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing
> > > > through devices not owned by pciback
> > > >
> > > > Hao, Xudong writes ("[Xen-devel] [PATCH] libxl: passthrough: avoid
> > > > passing through devices not owned by pciback"):
> > > > > <Porting from Xen 4.1 tree.>
> > > > >
> > > > > libxl: passthrough: avoid passing through devices not owned by
> > > > > pciback
> > > >
> > > > I'm afraid this no longer applies to xen-unstable.hg tip.
> > > >
> > > Reason?
> > >
> > > If no pciback checking, one device could be assigned to guest even it's being
> > used by dom0, is there security issue?
> >
> > I mean that it has conflicts when I try to apply it.  You need to refresh it.
> >
> > Thanks,
> > Ian.