All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Williamson <alex.williamson@redhat.com>
To: "Kay, Allen M" <allen.m.kay@intel.com>,
	"Tian, Kevin" <kevin.tian@intel.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Cc: "igvt-g@ml01.01.org" <igvt-g@ml01.01.org>,
	"xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	Cao jin <caoj.fnst@cn.fujitsu.com>,
	"vfio-users@redhat.com" <vfio-users@redhat.com>
Subject: Re: [Qemu-devel] [iGVT-g] [vfio-users] [PATCH v3 00/11] igd passthrough chipset tweaks
Date: Tue, 02 Feb 2016 12:37:29 -0700	[thread overview]
Message-ID: <1454441849.30910.5.camel@redhat.com> (raw)
In-Reply-To: <003AAFE53969E14CB1F09B6FD68C3CD47BB6CB3B@ORSMSX106.amr.corp.intel.com>

On Tue, 2016-02-02 at 19:10 +0000, Kay, Allen M wrote:
> 
> > -----Original Message-----
> > From: Tian, Kevin
> > Sent: Monday, February 01, 2016 11:08 PM
> > To: Kay, Allen M; Alex Williamson; Gerd Hoffmann; qemu-devel@nongnu.org
> > Cc: igvt-g@ml01.01.org; xen-devel@lists.xensource.com; Eduardo Habkost;
> > Stefano Stabellini; Cao jin; vfio-users@redhat.com
> > Subject: RE: [iGVT-g] [vfio-users] [PATCH v3 00/11] igd passthrough chipset
> > tweaks
> > 
> > > From: Kay, Allen M
> > > Sent: Saturday, January 30, 2016 5:58 AM
> > > 
> > > First of all, I would like to clarify I'm talking about general IGD
> > > passthrough case - not specific to KVMGT.  In IGD passthrough
> > > configuration, one of the following will happen when the driver accesses
> > OpRegion:
> > > 
> > > 1) If the hypervisor sets up OpRegion GPA/HPA mapping, either by
> > > pre-map it (i.e. Xen) or map it during EPT page fault (i.e. KVM),
> > > guest can successfully read the content of the OpRegion and check the ID
> > string.  In this case, everything works fine.
> > > 
> > > 2) if the hypervisor does not setup OpRegion GPA/HPA mapping at all,
> > > then guest driver's attempt to setup GVA/GPA mapping will fail, which
> > > causes the driver to fail.  In this case, guest driver won't have the
> > > opportunity to look into the content of OpRegion memory and check the ID
> > string.
> > > 
> > 
> > Guest mapping of GVA->GPA can always succeed regardless of whether
> > GPA->HPA is valid. Failure will happen only when the GVA is actually
> > accessed by guest.
> > 

Hi Allen,

> That is the data from team debugged IGD passthrough on a closed source hypervisor that does not map OpRegion with EPT.  The end result is the same -driver cannot access inside of OpRegion without
> failing.

Define "failing".

> > I don't understand 2). If hypervisor doesn't want to setup mapping, there is
> > no chance for guest driver to get opregion content, right?
> 
> That was precisely the point I was trying to make.  As a result, guest driver needs some indication from the hypervisor that the address at 0xFC contains GPA that can be safely accessed by the
> driver without causing unrecoverable failure on hypervisors that does not map OpRegion - by leaving HPA address at 0xFC.

I think the thing that doesn't make sense to everyone here is that it's
common practice for x86 systems, especially legacy OSes, to probe
memory, get back -1 and move on.  A hypervisor should support that.  So
if there's a bogus address in the ASL Storage register and the driver
tries to read from the GPA indicated by that address, the VM should at
worst get back -1 or a memory space that doesn't contain the graphics
signature.  If there's a super strict hypervisor that doesn't handle the
VM faulting outside of it's address space, that's very prone to exploit.
If a driver wants to avoid it anyway, perhaps they should be doing
standard things like checking whether the ASL Storage address falls
within a reserved memory region rather than coming up with ad-hoc
register content based solutions.  Thanks,

Alex

WARNING: multiple messages have this Message-ID (diff)
From: Alex Williamson <alex.williamson@redhat.com>
To: "Kay, Allen M" <allen.m.kay@intel.com>,
	"Tian, Kevin" <kevin.tian@intel.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Cc: "igvt-g@ml01.01.org" <igvt-g@ml01.01.org>,
	"xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	Cao jin <caoj.fnst@cn.fujitsu.com>,
	"vfio-users@redhat.com" <vfio-users@redhat.com>
Subject: Re: [iGVT-g] [vfio-users] [PATCH v3 00/11] igd passthrough chipset tweaks
Date: Tue, 02 Feb 2016 12:37:29 -0700	[thread overview]
Message-ID: <1454441849.30910.5.camel@redhat.com> (raw)
In-Reply-To: <003AAFE53969E14CB1F09B6FD68C3CD47BB6CB3B@ORSMSX106.amr.corp.intel.com>

On Tue, 2016-02-02 at 19:10 +0000, Kay, Allen M wrote:
> 
> > -----Original Message-----
> > From: Tian, Kevin
> > Sent: Monday, February 01, 2016 11:08 PM
> > To: Kay, Allen M; Alex Williamson; Gerd Hoffmann; qemu-devel@nongnu.org
> > Cc: igvt-g@ml01.01.org; xen-devel@lists.xensource.com; Eduardo Habkost;
> > Stefano Stabellini; Cao jin; vfio-users@redhat.com
> > Subject: RE: [iGVT-g] [vfio-users] [PATCH v3 00/11] igd passthrough chipset
> > tweaks
> > 
> > > From: Kay, Allen M
> > > Sent: Saturday, January 30, 2016 5:58 AM
> > > 
> > > First of all, I would like to clarify I'm talking about general IGD
> > > passthrough case - not specific to KVMGT.  In IGD passthrough
> > > configuration, one of the following will happen when the driver accesses
> > OpRegion:
> > > 
> > > 1) If the hypervisor sets up OpRegion GPA/HPA mapping, either by
> > > pre-map it (i.e. Xen) or map it during EPT page fault (i.e. KVM),
> > > guest can successfully read the content of the OpRegion and check the ID
> > string.  In this case, everything works fine.
> > > 
> > > 2) if the hypervisor does not setup OpRegion GPA/HPA mapping at all,
> > > then guest driver's attempt to setup GVA/GPA mapping will fail, which
> > > causes the driver to fail.  In this case, guest driver won't have the
> > > opportunity to look into the content of OpRegion memory and check the ID
> > string.
> > > 
> > 
> > Guest mapping of GVA->GPA can always succeed regardless of whether
> > GPA->HPA is valid. Failure will happen only when the GVA is actually
> > accessed by guest.
> > 

Hi Allen,

> That is the data from team debugged IGD passthrough on a closed source hypervisor that does not map OpRegion with EPT.  The end result is the same -driver cannot access inside of OpRegion without
> failing.

Define "failing".

> > I don't understand 2). If hypervisor doesn't want to setup mapping, there is
> > no chance for guest driver to get opregion content, right?
> 
> That was precisely the point I was trying to make.  As a result, guest driver needs some indication from the hypervisor that the address at 0xFC contains GPA that can be safely accessed by the
> driver without causing unrecoverable failure on hypervisors that does not map OpRegion - by leaving HPA address at 0xFC.

I think the thing that doesn't make sense to everyone here is that it's
common practice for x86 systems, especially legacy OSes, to probe
memory, get back -1 and move on.  A hypervisor should support that.  So
if there's a bogus address in the ASL Storage register and the driver
tries to read from the GPA indicated by that address, the VM should at
worst get back -1 or a memory space that doesn't contain the graphics
signature.  If there's a super strict hypervisor that doesn't handle the
VM faulting outside of it's address space, that's very prone to exploit.
If a driver wants to avoid it anyway, perhaps they should be doing
standard things like checking whether the ASL Storage address falls
within a reserved memory region rather than coming up with ad-hoc
register content based solutions.  Thanks,

Alex

  reply	other threads:[~2016-02-02 19:37 UTC|newest]

Thread overview: 132+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-05 11:41 [Qemu-devel] [PATCH v3 00/11] igd passthrough chipset tweaks Gerd Hoffmann
2016-01-05 11:41 ` Gerd Hoffmann
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 01/11] pc: wire up TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE for !xen Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 02/11] pc: remove has_igd_gfx_passthru global Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-06 14:32   ` [Qemu-devel] [Xen-devel] " Stefano Stabellini
2016-01-06 14:32     ` Stefano Stabellini
2016-01-19 15:09   ` [Qemu-devel] " Eduardo Habkost
2016-01-19 15:09     ` Eduardo Habkost
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 03/11] pc: move igd support code to igd.c Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 04/11] igd: switch TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE to realize Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-06 14:32   ` [Qemu-devel] " Stefano Stabellini
2016-01-06 14:32     ` Stefano Stabellini
2016-01-23 14:51   ` [Qemu-devel] " Eduardo Habkost
2016-01-23 14:51     ` Eduardo Habkost
2016-01-25  8:59     ` [Qemu-devel] " Gerd Hoffmann
2016-01-25  8:59       ` Gerd Hoffmann
2016-01-25 11:53       ` [Qemu-devel] " Stefano Stabellini
2016-01-25 11:53         ` Stefano Stabellini
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 05/11] igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-06 14:41   ` [Qemu-devel] " Stefano Stabellini
2016-01-06 14:41     ` Stefano Stabellini
2016-01-06 15:45     ` [Qemu-devel] " Gerd Hoffmann
2016-01-06 15:45       ` Gerd Hoffmann
2016-01-19 15:13       ` [Qemu-devel] " Eduardo Habkost
2016-01-19 15:13         ` Eduardo Habkost
2016-01-20  9:10         ` [Qemu-devel] " Gerd Hoffmann
2016-01-20  9:10           ` Gerd Hoffmann
2016-01-23 14:52           ` [Qemu-devel] " Eduardo Habkost
2016-01-23 14:52             ` Eduardo Habkost
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 06/11] igd: use defines for standard pci config space offsets Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-06 14:43   ` [Qemu-devel] " Stefano Stabellini
2016-01-06 14:43     ` Stefano Stabellini
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 07/11] igd: revamp host config read Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-06 15:02   ` [Qemu-devel] " Stefano Stabellini
2016-01-06 15:02     ` Stefano Stabellini
2016-01-06 15:51     ` [Qemu-devel] " Gerd Hoffmann
2016-01-06 15:51       ` Gerd Hoffmann
2016-01-06 16:23       ` [Qemu-devel] [Xen-devel] " Stefano Stabellini
2016-01-06 16:23         ` Stefano Stabellini
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 08/11] igd: add q35 support Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 09/11] igd: move igd-passthrough-isa-bridge to igd.c too Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 10/11] igd: handle igd-passthrough-isa-bridge setup in realize() Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-06 15:29   ` [Qemu-devel] " Stefano Stabellini
2016-01-06 15:29     ` Stefano Stabellini
2016-01-06 15:52     ` [Qemu-devel] " Gerd Hoffmann
2016-01-06 15:52       ` Gerd Hoffmann
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 11/11] igd: move igd-passthrough-isa-bridge creation to machine init Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-06 15:36   ` [Qemu-devel] " Stefano Stabellini
2016-01-06 15:36     ` Stefano Stabellini
2016-01-07  7:38     ` [Qemu-devel] " Gerd Hoffmann
2016-01-07  7:38       ` Gerd Hoffmann
2016-01-07 13:10       ` [Qemu-devel] " Stefano Stabellini
2016-01-07 13:10         ` Stefano Stabellini
2016-01-07 15:50         ` [Qemu-devel] " Gerd Hoffmann
2016-01-07 15:50           ` Gerd Hoffmann
2016-01-08 11:20           ` Stefano Stabellini
2016-01-08 11:20             ` Stefano Stabellini
2016-01-08 12:12             ` [Qemu-devel] " Stefano Stabellini
2016-01-08 12:12               ` Stefano Stabellini
2016-01-08 12:32               ` Gerd Hoffmann
2016-01-08 12:32                 ` Gerd Hoffmann
2016-01-08 12:38                 ` [Qemu-devel] " Stefano Stabellini
2016-01-08 12:38                   ` Stefano Stabellini
2016-01-05 13:07 ` [Qemu-devel] [PATCH v3 00/11] igd passthrough chipset tweaks Michael S. Tsirkin
2016-01-05 13:07   ` Michael S. Tsirkin
2016-01-28 19:35 ` [Qemu-devel] [vfio-users] " Alex Williamson
2016-01-28 19:35   ` Alex Williamson
2016-01-29  2:22   ` [Qemu-devel] [iGVT-g] " Kay, Allen M
2016-01-29  2:22     ` Kay, Allen M
2016-01-29  2:54     ` [Qemu-devel] " Alex Williamson
2016-01-29  2:54       ` Alex Williamson
2016-01-29  6:21       ` [Qemu-devel] " Jike Song
2016-01-29  6:21         ` Jike Song
2016-01-29 21:58       ` [Qemu-devel] " Kay, Allen M
2016-01-29 21:58         ` Kay, Allen M
2016-02-02  7:07         ` [Qemu-devel] " Tian, Kevin
2016-02-02  7:07           ` Tian, Kevin
2016-02-02 19:10           ` [Qemu-devel] " Kay, Allen M
2016-02-02 19:10             ` [iGVT-g] " Kay, Allen M
2016-02-02 19:37             ` Alex Williamson [this message]
2016-02-02 19:37               ` [iGVT-g] [vfio-users] " Alex Williamson
2016-02-02 23:32               ` [Qemu-devel] " Kay, Allen M
2016-02-02 23:32                 ` Kay, Allen M
2016-01-29  7:09   ` [Qemu-devel] " Gerd Hoffmann
2016-01-29  7:09     ` Gerd Hoffmann
2016-01-29 17:59     ` [Qemu-devel] " Alex Williamson
2016-01-29 17:59       ` Alex Williamson
2016-01-30  1:18       ` [Qemu-devel] [iGVT-g] " Kay, Allen M
2016-01-30  1:18         ` Kay, Allen M
2016-01-31 17:42         ` [Qemu-devel] " Alex Williamson
2016-01-31 17:42           ` Alex Williamson
2016-02-02  0:04           ` [Qemu-devel] " Kay, Allen M
2016-02-02  0:04             ` Kay, Allen M
2016-02-02  6:42             ` [Qemu-devel] [Xen-devel] " Tian, Kevin
2016-02-02  6:42               ` Tian, Kevin
2016-02-02 11:50               ` [Qemu-devel] " David Woodhouse
2016-02-02 11:50                 ` David Woodhouse
2016-02-02 14:54                 ` [Qemu-devel] " Alex Williamson
2016-02-02 14:54                   ` Alex Williamson
2016-02-02 15:06                   ` [Qemu-devel] " David Woodhouse
2016-02-02 15:06                     ` David Woodhouse
2016-02-02 14:38             ` [Qemu-devel] " Alex Williamson
2016-02-02 14:38               ` Alex Williamson
2016-02-01 12:49       ` [Qemu-devel] " Gerd Hoffmann
2016-02-01 12:49         ` Gerd Hoffmann
2016-02-01 22:16         ` [Qemu-devel] " Alex Williamson
2016-02-01 22:16           ` Alex Williamson
2016-02-02  7:43           ` [Qemu-devel] [vfio-users] " Gerd Hoffmann
2016-02-02  7:43             ` Gerd Hoffmann
2016-02-02  7:01         ` [Qemu-devel] [iGVT-g] " Tian, Kevin
2016-02-02  7:01           ` [iGVT-g] " Tian, Kevin
2016-02-02  8:56           ` [Qemu-devel] [iGVT-g] [vfio-users] " Gerd Hoffmann
2016-02-02  8:56             ` Gerd Hoffmann
2016-02-02 16:31             ` [Qemu-devel] " Kevin O'Connor
2016-02-02 16:31               ` Kevin O'Connor
2016-02-02 16:49               ` [Qemu-devel] " Laszlo Ersek
2016-02-02 16:49                 ` Laszlo Ersek
2016-02-02 20:18               ` [Qemu-devel] " Alex Williamson
2016-02-02 20:18                 ` Alex Williamson
2016-02-03  6:08             ` [Qemu-devel] " Tian, Kevin
2016-02-03  6:08               ` Tian, Kevin

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=1454441849.30910.5.camel@redhat.com \
    --to=alex.williamson@redhat.com \
    --cc=allen.m.kay@intel.com \
    --cc=caoj.fnst@cn.fujitsu.com \
    --cc=ehabkost@redhat.com \
    --cc=igvt-g@ml01.01.org \
    --cc=kevin.tian@intel.com \
    --cc=kraxel@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=vfio-users@redhat.com \
    --cc=xen-devel@lists.xensource.com \
    /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.