svn commit: r443982 - in branches/2017Q2/emulators/xen-kernel: . files
Roger Pau Monné
royger at FreeBSD.org
Tue Jun 20 14:46:16 UTC 2017
Author: royger (src committer)
Date: Tue Jun 20 14:46:14 2017
New Revision: 443982
URL: https://svnweb.freebsd.org/changeset/ports/443982
Log:
MFH: r443949
xen: apply XSA-{217,218,219,220,221,222,224}
Sponsored by: Citrix Systems R&D
Approved by: ports-secteam (feld)
Added:
branches/2017Q2/emulators/xen-kernel/files/0001-IOMMU-handle-IOMMU-mapping-and-unmapping-failures.patch
- copied unchanged from r443949, head/emulators/xen-kernel/files/0001-IOMMU-handle-IOMMU-mapping-and-unmapping-failures.patch
branches/2017Q2/emulators/xen-kernel/files/0001-gnttab-Fix-handling-of-dev_bus_addr-during-unmap.patch
- copied unchanged from r443949, head/emulators/xen-kernel/files/0001-gnttab-Fix-handling-of-dev_bus_addr-during-unmap.patch
branches/2017Q2/emulators/xen-kernel/files/0002-gnttab-fix-unmap-pin-accounting-race.patch
- copied unchanged from r443949, head/emulators/xen-kernel/files/0002-gnttab-fix-unmap-pin-accounting-race.patch
branches/2017Q2/emulators/xen-kernel/files/0002-gnttab-never-create-host-mapping-unless-asked-to.patch
- copied unchanged from r443949, head/emulators/xen-kernel/files/0002-gnttab-never-create-host-mapping-unless-asked-to.patch
branches/2017Q2/emulators/xen-kernel/files/0003-gnttab-Avoid-potential-double-put-of-maptrack-entry.patch
- copied unchanged from r443949, head/emulators/xen-kernel/files/0003-gnttab-Avoid-potential-double-put-of-maptrack-entry.patch
branches/2017Q2/emulators/xen-kernel/files/0003-gnttab-correct-logic-to-get-page-references-during-m.patch
- copied unchanged from r443949, head/emulators/xen-kernel/files/0003-gnttab-correct-logic-to-get-page-references-during-m.patch
branches/2017Q2/emulators/xen-kernel/files/0004-gnttab-__gnttab_unmap_common_complete-is-all-or-noth.patch
- copied unchanged from r443949, head/emulators/xen-kernel/files/0004-gnttab-__gnttab_unmap_common_complete-is-all-or-noth.patch
branches/2017Q2/emulators/xen-kernel/files/0004-gnttab-correct-maptrack-table-accesses.patch
- copied unchanged from r443949, head/emulators/xen-kernel/files/0004-gnttab-correct-maptrack-table-accesses.patch
branches/2017Q2/emulators/xen-kernel/files/xsa217.patch
- copied unchanged from r443949, head/emulators/xen-kernel/files/xsa217.patch
branches/2017Q2/emulators/xen-kernel/files/xsa219-4.8.patch
- copied unchanged from r443949, head/emulators/xen-kernel/files/xsa219-4.8.patch
branches/2017Q2/emulators/xen-kernel/files/xsa220-4.7.patch
- copied unchanged from r443949, head/emulators/xen-kernel/files/xsa220-4.7.patch
branches/2017Q2/emulators/xen-kernel/files/xsa221.patch
- copied unchanged from r443949, head/emulators/xen-kernel/files/xsa221.patch
branches/2017Q2/emulators/xen-kernel/files/xsa222-1-4.7.patch
- copied unchanged from r443949, head/emulators/xen-kernel/files/xsa222-1-4.7.patch
branches/2017Q2/emulators/xen-kernel/files/xsa222-2-4.7.patch
- copied unchanged from r443949, head/emulators/xen-kernel/files/xsa222-2-4.7.patch
Modified:
branches/2017Q2/emulators/xen-kernel/Makefile
Directory Properties:
branches/2017Q2/ (props changed)
Modified: branches/2017Q2/emulators/xen-kernel/Makefile
==============================================================================
--- branches/2017Q2/emulators/xen-kernel/Makefile Tue Jun 20 14:45:02 2017 (r443981)
+++ branches/2017Q2/emulators/xen-kernel/Makefile Tue Jun 20 14:46:14 2017 (r443982)
@@ -2,7 +2,7 @@
PORTNAME= xen
PORTVERSION= 4.7.2
-PORTREVISION= 2
+PORTREVISION= 3
CATEGORIES= emulators
MASTER_SITES= http://downloads.xenproject.org/release/xen/${PORTVERSION}/
PKGNAMESUFFIX= -kernel
@@ -45,7 +45,21 @@ EXTRA_PATCHES= ${FILESDIR}/0001-xen-logdirty-prevent-p
${FILESDIR}/xsa212.patch:-p1 \
${FILESDIR}/xsa213-4.7.patch:-p1 \
${FILESDIR}/xsa214.patch:-p1 \
- ${FILESDIR}/xsa215.patch:-p1
+ ${FILESDIR}/xsa215.patch:-p1 \
+ ${FILESDIR}/xsa217.patch:-p1 \
+ ${FILESDIR}/0001-IOMMU-handle-IOMMU-mapping-and-unmapping-failures.patch:-p1 \
+ ${FILESDIR}/0002-gnttab-fix-unmap-pin-accounting-race.patch:-p1 \
+ ${FILESDIR}/0003-gnttab-Avoid-potential-double-put-of-maptrack-entry.patch:-p1 \
+ ${FILESDIR}/0004-gnttab-correct-maptrack-table-accesses.patch:-p1 \
+ ${FILESDIR}/xsa219-4.8.patch:-p1 \
+ ${FILESDIR}/xsa220-4.7.patch:-p1 \
+ ${FILESDIR}/xsa221.patch:-p1 \
+ ${FILESDIR}/xsa222-1-4.7.patch:-p1 \
+ ${FILESDIR}/xsa222-2-4.7.patch:-p1 \
+ ${FILESDIR}/0001-gnttab-Fix-handling-of-dev_bus_addr-during-unmap.patch:-p1 \
+ ${FILESDIR}/0002-gnttab-never-create-host-mapping-unless-asked-to.patch:-p1 \
+ ${FILESDIR}/0003-gnttab-correct-logic-to-get-page-references-during-m.patch:-p1 \
+ ${FILESDIR}/0004-gnttab-__gnttab_unmap_common_complete-is-all-or-noth.patch:-p1
.include <bsd.port.options.mk>
Copied: branches/2017Q2/emulators/xen-kernel/files/0001-IOMMU-handle-IOMMU-mapping-and-unmapping-failures.patch (from r443949, head/emulators/xen-kernel/files/0001-IOMMU-handle-IOMMU-mapping-and-unmapping-failures.patch)
==============================================================================
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
+++ branches/2017Q2/emulators/xen-kernel/files/0001-IOMMU-handle-IOMMU-mapping-and-unmapping-failures.patch Tue Jun 20 14:46:14 2017 (r443982, copy of r443949, head/emulators/xen-kernel/files/0001-IOMMU-handle-IOMMU-mapping-and-unmapping-failures.patch)
@@ -0,0 +1,78 @@
+From 03f872b98f24e25cafb478b5d7c34e1eb18e1e4c Mon Sep 17 00:00:00 2001
+From: Quan Xu <quan.xu at intel.com>
+Date: Fri, 2 Jun 2017 12:30:34 +0100
+Subject: [PATCH 1/4] IOMMU: handle IOMMU mapping and unmapping failures
+
+Treat IOMMU mapping and unmapping failures as a fatal to the DomU
+If IOMMU mapping and unmapping failed, crash the DomU and propagate
+the error up to the call trees.
+
+No spamming of the log can occur. For DomU, we avoid logging any
+message for already dying domains. For Dom0, that'll still be more
+verbose than we'd really like, but it at least wouldn't outright
+flood the console.
+
+Signed-off-by: Quan Xu <quan.xu at intel.com>
+Reviewed-by: Kevin Tian <kevin.tian at intel.com>
+Reviewed-by: Jan Beulich <jbeulich at suse.com>
+---
+ xen/drivers/passthrough/iommu.c | 30 ++++++++++++++++++++++++++++--
+ 1 file changed, 28 insertions(+), 2 deletions(-)
+
+diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
+index 1a315ee..927966f 100644
+--- a/xen/drivers/passthrough/iommu.c
++++ b/xen/drivers/passthrough/iommu.c
+@@ -239,21 +239,47 @@ int iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn,
+ unsigned int flags)
+ {
+ const struct domain_iommu *hd = dom_iommu(d);
++ int rc;
+
+ if ( !iommu_enabled || !hd->platform_ops )
+ return 0;
+
+- return hd->platform_ops->map_page(d, gfn, mfn, flags);
++ rc = hd->platform_ops->map_page(d, gfn, mfn, flags);
++ if ( unlikely(rc) )
++ {
++ if ( !d->is_shutting_down && printk_ratelimit() )
++ printk(XENLOG_ERR
++ "d%d: IOMMU mapping gfn %#lx to mfn %#lx failed: %d\n",
++ d->domain_id, gfn, mfn, rc);
++
++ if ( !is_hardware_domain(d) )
++ domain_crash(d);
++ }
++
++ return rc;
+ }
+
+ int iommu_unmap_page(struct domain *d, unsigned long gfn)
+ {
+ const struct domain_iommu *hd = dom_iommu(d);
++ int rc;
+
+ if ( !iommu_enabled || !hd->platform_ops )
+ return 0;
+
+- return hd->platform_ops->unmap_page(d, gfn);
++ rc = hd->platform_ops->unmap_page(d, gfn);
++ if ( unlikely(rc) )
++ {
++ if ( !d->is_shutting_down && printk_ratelimit() )
++ printk(XENLOG_ERR
++ "d%d: IOMMU unmapping gfn %#lx failed: %d\n",
++ d->domain_id, gfn, rc);
++
++ if ( !is_hardware_domain(d) )
++ domain_crash(d);
++ }
++
++ return rc;
+ }
+
+ static void iommu_free_pagetables(unsigned long unused)
+--
+2.1.4
+
Copied: branches/2017Q2/emulators/xen-kernel/files/0001-gnttab-Fix-handling-of-dev_bus_addr-during-unmap.patch (from r443949, head/emulators/xen-kernel/files/0001-gnttab-Fix-handling-of-dev_bus_addr-during-unmap.patch)
==============================================================================
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
+++ branches/2017Q2/emulators/xen-kernel/files/0001-gnttab-Fix-handling-of-dev_bus_addr-during-unmap.patch Tue Jun 20 14:46:14 2017 (r443982, copy of r443949, head/emulators/xen-kernel/files/0001-gnttab-Fix-handling-of-dev_bus_addr-during-unmap.patch)
@@ -0,0 +1,111 @@
+From fd97f5f5ba9375163c8d8771fe551bb4a6423b36 Mon Sep 17 00:00:00 2001
+From: George Dunlap <george.dunlap at citrix.com>
+Date: Thu, 15 Jun 2017 16:24:02 +0100
+Subject: [PATCH 1/4] gnttab: Fix handling of dev_bus_addr during unmap
+
+If a grant has been mapped with the GNTTAB_device_map flag, calling
+grant_unmap_ref() with dev_bus_addr set to zero should cause the
+GNTTAB_device_map part of the mapping to be left alone.
+
+Unfortunately, at the moment, op->dev_bus_addr is implicitly checked
+before clearing the map and adjusting the pin count, but only the bits
+above 12; and it is not checked at all before dropping page
+references. This means a guest can repeatedly make such a call to
+cause the reference count to drop to zero, causing the page to be
+freed and re-used, even though it's still mapped in its pagetables.
+
+To fix this, always check op->dev_bus_addr explicitly for being
+non-zero, as well as op->flag & GNTMAP_device_map, before doing
+operations on the device_map.
+
+While we're here, make the logic a bit cleaner:
+
+* Always initialize op->frame to zero and set it from act->frame, to reduce the
+chance of untrusted input being used
+
+* Explicitly check the full dev_bus_addr against act->frame <<
+ PAGE_SHIFT, rather than ignoring the lower 12 bits
+
+This is part of XSA-224.
+
+Reported-by: Jan Beulich <jbeulich at suse.com>
+Signed-off-by: George Dunlap <george.dunlap at citrix.com>
+Signed-off-by: Jan Beulich <jbeulich at suse.com>
+---
+ xen/common/grant_table.c | 23 +++++++++++------------
+ 1 file changed, 11 insertions(+), 12 deletions(-)
+
+diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
+index c4d73af..69cbdb6 100644
+--- a/xen/common/grant_table.c
++++ b/xen/common/grant_table.c
+@@ -1089,8 +1089,6 @@ __gnttab_unmap_common(
+ ld = current->domain;
+ lgt = ld->grant_table;
+
+- op->frame = (unsigned long)(op->dev_bus_addr >> PAGE_SHIFT);
+-
+ if ( unlikely(op->handle >= lgt->maptrack_limit) )
+ {
+ gdprintk(XENLOG_INFO, "Bad handle (%d).\n", op->handle);
+@@ -1174,16 +1172,14 @@ __gnttab_unmap_common(
+ goto act_release_out;
+ }
+
+- if ( op->frame == 0 )
+- {
+- op->frame = act->frame;
+- }
+- else
++ op->frame = act->frame;
++
++ if ( op->dev_bus_addr )
+ {
+- if ( unlikely(op->frame != act->frame) )
++ if ( unlikely(op->dev_bus_addr != pfn_to_paddr(act->frame)) )
+ PIN_FAIL(act_release_out, GNTST_general_error,
+- "Bad frame number doesn't match gntref. (%lx != %lx)\n",
+- op->frame, act->frame);
++ "Bus address doesn't match gntref (%"PRIx64" != %"PRIpaddr")\n",
++ op->dev_bus_addr, pfn_to_paddr(act->frame));
+
+ map->flags &= ~GNTMAP_device_map;
+ }
+@@ -1276,7 +1272,8 @@ __gnttab_unmap_common_complete(struct gnttab_unmap_common *op)
+ else
+ status = &status_entry(rgt, op->ref);
+
+- if ( unlikely(op->frame != act->frame) )
++ if ( op->dev_bus_addr &&
++ unlikely(op->dev_bus_addr != pfn_to_paddr(act->frame)) )
+ {
+ /*
+ * Suggests that __gntab_unmap_common failed early and so
+@@ -1287,7 +1284,7 @@ __gnttab_unmap_common_complete(struct gnttab_unmap_common *op)
+
+ pg = mfn_to_page(op->frame);
+
+- if ( op->flags & GNTMAP_device_map )
++ if ( op->dev_bus_addr && (op->flags & GNTMAP_device_map) )
+ {
+ if ( !is_iomem_page(act->frame) )
+ {
+@@ -1358,6 +1355,7 @@ __gnttab_unmap_grant_ref(
+ /* Intialise these in case common contains old state */
+ common->new_addr = 0;
+ common->rd = NULL;
++ common->frame = 0;
+
+ __gnttab_unmap_common(common);
+ op->status = common->status;
+@@ -1422,6 +1420,7 @@ __gnttab_unmap_and_replace(
+ /* Intialise these in case common contains old state */
+ common->dev_bus_addr = 0;
+ common->rd = NULL;
++ common->frame = 0;
+
+ __gnttab_unmap_common(common);
+ op->status = common->status;
+--
+2.1.4
+
Copied: branches/2017Q2/emulators/xen-kernel/files/0002-gnttab-fix-unmap-pin-accounting-race.patch (from r443949, head/emulators/xen-kernel/files/0002-gnttab-fix-unmap-pin-accounting-race.patch)
==============================================================================
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
+++ branches/2017Q2/emulators/xen-kernel/files/0002-gnttab-fix-unmap-pin-accounting-race.patch Tue Jun 20 14:46:14 2017 (r443982, copy of r443949, head/emulators/xen-kernel/files/0002-gnttab-fix-unmap-pin-accounting-race.patch)
@@ -0,0 +1,102 @@
+From 2c146b4f763f47180a0effb8d8045b0ebb93652c Mon Sep 17 00:00:00 2001
+From: Jan Beulich <jbeulich at suse.com>
+Date: Fri, 2 Jun 2017 12:22:42 +0100
+Subject: [PATCH 2/4] gnttab: fix unmap pin accounting race
+
+Once all {writable} mappings of a grant entry have been unmapped, the
+hypervisor informs the guest that the grant entry has been released by
+clearing the _GTF_{reading,writing} usage flags in the guest's grant
+table as appropriate.
+
+Unfortunately, at the moment, the code that updates the accounting
+happens in a different critical section than the one which updates the
+usage flags; this means that under the right circumstances, there may be
+a window in time after the hypervisor reported the grant as being free
+during which the grant referee still had access to the page.
+
+Move the grant accounting code into the same critical section as the
+reporting code to make sure this kind of race can't happen.
+
+This is part of XSA-218.
+
+Reported-by: Jann Horn <jannh.com>
+Signed-off-by: Jan Beulich <jbeulich at suse.com>
+---
+ xen/common/grant_table.c | 32 +++++++++++++++++---------------
+ 1 file changed, 17 insertions(+), 15 deletions(-)
+
+diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
+index 8b22299..cfc483f 100644
+--- a/xen/common/grant_table.c
++++ b/xen/common/grant_table.c
+@@ -1150,15 +1150,8 @@ __gnttab_unmap_common(
+ PIN_FAIL(act_release_out, GNTST_general_error,
+ "Bad frame number doesn't match gntref. (%lx != %lx)\n",
+ op->frame, act->frame);
+- if ( op->flags & GNTMAP_device_map )
+- {
+- ASSERT(act->pin & (GNTPIN_devw_mask | GNTPIN_devr_mask));
+- op->map->flags &= ~GNTMAP_device_map;
+- if ( op->flags & GNTMAP_readonly )
+- act->pin -= GNTPIN_devr_inc;
+- else
+- act->pin -= GNTPIN_devw_inc;
+- }
++
++ op->map->flags &= ~GNTMAP_device_map;
+ }
+
+ if ( (op->host_addr != 0) && (op->flags & GNTMAP_host_map) )
+@@ -1168,12 +1161,7 @@ __gnttab_unmap_common(
+ op->flags)) < 0 )
+ goto act_release_out;
+
+- ASSERT(act->pin & (GNTPIN_hstw_mask | GNTPIN_hstr_mask));
+ op->map->flags &= ~GNTMAP_host_map;
+- if ( op->flags & GNTMAP_readonly )
+- act->pin -= GNTPIN_hstr_inc;
+- else
+- act->pin -= GNTPIN_hstw_inc;
+ }
+
+ act_release_out:
+@@ -1266,6 +1254,12 @@ __gnttab_unmap_common_complete(struct gnttab_unmap_common *op)
+ else
+ put_page_and_type(pg);
+ }
++
++ ASSERT(act->pin & (GNTPIN_devw_mask | GNTPIN_devr_mask));
++ if ( op->flags & GNTMAP_readonly )
++ act->pin -= GNTPIN_devr_inc;
++ else
++ act->pin -= GNTPIN_devw_inc;
+ }
+
+ if ( (op->host_addr != 0) && (op->flags & GNTMAP_host_map) )
+@@ -1274,7 +1268,9 @@ __gnttab_unmap_common_complete(struct gnttab_unmap_common *op)
+ {
+ /*
+ * Suggests that __gntab_unmap_common failed in
+- * replace_grant_host_mapping() so nothing further to do
++ * replace_grant_host_mapping() or IOMMU handling, so nothing
++ * further to do (short of re-establishing the mapping in the
++ * latter case).
+ */
+ goto act_release_out;
+ }
+@@ -1285,6 +1281,12 @@ __gnttab_unmap_common_complete(struct gnttab_unmap_common *op)
+ put_page_type(pg);
+ put_page(pg);
+ }
++
++ ASSERT(act->pin & (GNTPIN_hstw_mask | GNTPIN_hstr_mask));
++ if ( op->flags & GNTMAP_readonly )
++ act->pin -= GNTPIN_hstr_inc;
++ else
++ act->pin -= GNTPIN_hstw_inc;
+ }
+
+ if ( (op->map->flags & (GNTMAP_device_map|GNTMAP_host_map)) == 0 )
+--
+2.1.4
+
Copied: branches/2017Q2/emulators/xen-kernel/files/0002-gnttab-never-create-host-mapping-unless-asked-to.patch (from r443949, head/emulators/xen-kernel/files/0002-gnttab-never-create-host-mapping-unless-asked-to.patch)
==============================================================================
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
+++ branches/2017Q2/emulators/xen-kernel/files/0002-gnttab-never-create-host-mapping-unless-asked-to.patch Tue Jun 20 14:46:14 2017 (r443982, copy of r443949, head/emulators/xen-kernel/files/0002-gnttab-never-create-host-mapping-unless-asked-to.patch)
@@ -0,0 +1,42 @@
+From 8894a0c20d920aada305aade0591c1e77167b1db Mon Sep 17 00:00:00 2001
+From: Jan Beulich <jbeulich at suse.com>
+Date: Fri, 2 Jun 2017 15:21:27 +0100
+Subject: [PATCH 2/4] gnttab: never create host mapping unless asked to
+
+We shouldn't create a host mapping unless asked to even in the case of
+mapping a granted MMIO page. In particular the mapping wouldn't be torn
+down when processing the matching unmap request.
+
+This is part of XSA-224.
+
+Reported-by: Jan Beulich <jbeulich at suse.com>
+Signed-off-by: Jan Beulich <jbeulich at suse.com>
+---
+ xen/common/grant_table.c | 11 +++++++----
+ 1 file changed, 7 insertions(+), 4 deletions(-)
+
+diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
+index 69cbdb6..452538e 100644
+--- a/xen/common/grant_table.c
++++ b/xen/common/grant_table.c
+@@ -911,10 +911,13 @@ __gnttab_map_grant_ref(
+ goto undo_out;
+ }
+
+- rc = create_grant_host_mapping(
+- op->host_addr, frame, op->flags, cache_flags);
+- if ( rc != GNTST_okay )
+- goto undo_out;
++ if ( op->flags & GNTMAP_host_map )
++ {
++ rc = create_grant_host_mapping(op->host_addr, frame, op->flags,
++ cache_flags);
++ if ( rc != GNTST_okay )
++ goto undo_out;
++ }
+ }
+ else if ( owner == rd || owner == dom_cow )
+ {
+--
+2.1.4
+
Copied: branches/2017Q2/emulators/xen-kernel/files/0003-gnttab-Avoid-potential-double-put-of-maptrack-entry.patch (from r443949, head/emulators/xen-kernel/files/0003-gnttab-Avoid-potential-double-put-of-maptrack-entry.patch)
==============================================================================
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
+++ branches/2017Q2/emulators/xen-kernel/files/0003-gnttab-Avoid-potential-double-put-of-maptrack-entry.patch Tue Jun 20 14:46:14 2017 (r443982, copy of r443949, head/emulators/xen-kernel/files/0003-gnttab-Avoid-potential-double-put-of-maptrack-entry.patch)
@@ -0,0 +1,232 @@
+From 39b704785a8d330c02e8e2d2368c80dbaf679bc0 Mon Sep 17 00:00:00 2001
+From: George Dunlap <george.dunlap at citrix.com>
+Date: Thu, 15 Jun 2017 12:05:14 +0100
+Subject: [PATCH 3/4] gnttab: Avoid potential double-put of maptrack entry
+
+Each grant mapping for a particular domain is tracked by an in-Xen
+"maptrack" entry. This entry is is referenced by a "handle", which is
+given to the guest when it calls gnttab_map_grant_ref().
+
+There are two types of mapping a particular handle can refer to:
+GNTMAP_host_map and GNTMAP_device_map. A given
+gnttab_unmap_grant_ref() call can remove either only one or both of
+these entries. When a particular handle has no entries left, it must
+be freed.
+
+gnttab_unmap_grant_ref() loops through its grant unmap request list
+twice. It first removes entries from any host pagetables and (if
+appropraite) iommus; then it does a single domain TLB flush; then it
+does the clean-up, including telling the granter that entries are no
+longer being used (if appropriate).
+
+At the moment, it's during the first pass that the maptrack flags are
+cleared, but the second pass that the maptrack entry is freed.
+
+Unfortunately this allows the following race, which results in a
+double-free:
+
+ A: (pass 1) clear host_map
+ B: (pass 1) clear device_map
+ A: (pass 2) See that maptrack entry has no mappings, free it
+ B: (pass 2) See that maptrack entry has no mappings, free it #
+
+Unfortunately, unlike the active entry pinning update, we can't simply
+move the maptrack flag changes to the second half, because the
+maptrack flags are used to determine if iommu entries need to be
+added: a domain's iommu must never have fewer permissions than the
+maptrack flags indicate, or a subsequent map_grant_ref() might fail to
+add the necessary iommu entries.
+
+Instead, free the maptrack entry in the first pass if there are no
+further mappings.
+
+This is part of XSA-218.
+
+Reported-by: Jan Beulich <jbeulich.com>
+Signed-off-by: George Dunlap <george.dunlap at citrix.com>
+Signed-off-by: Jan Beulich <jbeulich at suse.com>
+---
+ xen/common/grant_table.c | 79 +++++++++++++++++++++++++++++++++---------------
+ 1 file changed, 54 insertions(+), 25 deletions(-)
+
+diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
+index cfc483f..81a1a8b 100644
+--- a/xen/common/grant_table.c
++++ b/xen/common/grant_table.c
+@@ -98,8 +98,8 @@ struct gnttab_unmap_common {
+ /* Shared state beteen *_unmap and *_unmap_complete */
+ u16 flags;
+ unsigned long frame;
+- struct grant_mapping *map;
+ struct domain *rd;
++ grant_ref_t ref;
+ };
+
+ /* Number of unmap operations that are done between each tlb flush */
+@@ -1079,6 +1079,8 @@ __gnttab_unmap_common(
+ struct grant_table *lgt, *rgt;
+ struct active_grant_entry *act;
+ s16 rc = 0;
++ struct grant_mapping *map;
++ bool_t put_handle = 0;
+
+ ld = current->domain;
+ lgt = ld->grant_table;
+@@ -1092,11 +1094,11 @@ __gnttab_unmap_common(
+ return;
+ }
+
+- op->map = &maptrack_entry(lgt, op->handle);
++ map = &maptrack_entry(lgt, op->handle);
+
+ grant_read_lock(lgt);
+
+- if ( unlikely(!read_atomic(&op->map->flags)) )
++ if ( unlikely(!read_atomic(&map->flags)) )
+ {
+ grant_read_unlock(lgt);
+ gdprintk(XENLOG_INFO, "Zero flags for handle (%d).\n", op->handle);
+@@ -1104,7 +1106,7 @@ __gnttab_unmap_common(
+ return;
+ }
+
+- dom = op->map->domid;
++ dom = map->domid;
+ grant_read_unlock(lgt);
+
+ if ( unlikely((rd = rcu_lock_domain_by_id(dom)) == NULL) )
+@@ -1129,16 +1131,43 @@ __gnttab_unmap_common(
+
+ grant_read_lock(rgt);
+
+- op->flags = read_atomic(&op->map->flags);
+- if ( unlikely(!op->flags) || unlikely(op->map->domid != dom) )
++ op->rd = rd;
++ op->ref = map->ref;
++
++ /*
++ * We can't assume there was no racing unmap for this maptrack entry,
++ * and hence we can't assume map->ref is valid for rd. While the checks
++ * below (with the active entry lock held) will reject any such racing
++ * requests, we still need to make sure we don't attempt to acquire an
++ * invalid lock.
++ */
++ smp_rmb();
++ if ( unlikely(op->ref >= nr_grant_entries(rgt)) )
+ {
+- gdprintk(XENLOG_WARNING, "Unstable handle %u\n", op->handle);
++ gdprintk(XENLOG_WARNING, "Unstable handle %#x\n", op->handle);
+ rc = GNTST_bad_handle;
+- goto unmap_out;
++ goto unlock_out;
+ }
+
+- op->rd = rd;
+- act = active_entry_acquire(rgt, op->map->ref);
++ act = active_entry_acquire(rgt, op->ref);
++
++ /*
++ * Note that we (ab)use the active entry lock here to protect against
++ * multiple unmaps of the same mapping here. We don't want to hold lgt's
++ * lock, and we only hold rgt's lock for reading (but the latter wouldn't
++ * be the right one anyway). Hence the easiest is to rely on a lock we
++ * hold anyway; see docs/misc/grant-tables.txt's "Locking" section.
++ */
++
++ op->flags = read_atomic(&map->flags);
++ smp_rmb();
++ if ( unlikely(!op->flags) || unlikely(map->domid != dom) ||
++ unlikely(map->ref != op->ref) )
++ {
++ gdprintk(XENLOG_WARNING, "Unstable handle %#x\n", op->handle);
++ rc = GNTST_bad_handle;
++ goto act_release_out;
++ }
+
+ if ( op->frame == 0 )
+ {
+@@ -1151,7 +1180,7 @@ __gnttab_unmap_common(
+ "Bad frame number doesn't match gntref. (%lx != %lx)\n",
+ op->frame, act->frame);
+
+- op->map->flags &= ~GNTMAP_device_map;
++ map->flags &= ~GNTMAP_device_map;
+ }
+
+ if ( (op->host_addr != 0) && (op->flags & GNTMAP_host_map) )
+@@ -1161,14 +1190,23 @@ __gnttab_unmap_common(
+ op->flags)) < 0 )
+ goto act_release_out;
+
+- op->map->flags &= ~GNTMAP_host_map;
++ map->flags &= ~GNTMAP_host_map;
++ }
++
++ if ( !(map->flags & (GNTMAP_device_map|GNTMAP_host_map)) )
++ {
++ map->flags = 0;
++ put_handle = 1;
+ }
+
+ act_release_out:
+ active_entry_release(act);
+- unmap_out:
++ unlock_out:
+ grant_read_unlock(rgt);
+
++ if ( put_handle )
++ put_maptrack_handle(lgt, op->handle);
++
+ if ( rc == GNTST_okay && gnttab_need_iommu_mapping(ld) )
+ {
+ unsigned int kind;
+@@ -1205,7 +1243,6 @@ __gnttab_unmap_common_complete(struct gnttab_unmap_common *op)
+ grant_entry_header_t *sha;
+ struct page_info *pg;
+ uint16_t *status;
+- bool_t put_handle = 0;
+
+ if ( rd == NULL )
+ {
+@@ -1226,13 +1263,13 @@ __gnttab_unmap_common_complete(struct gnttab_unmap_common *op)
+ if ( rgt->gt_version == 0 )
+ goto unlock_out;
+
+- act = active_entry_acquire(rgt, op->map->ref);
+- sha = shared_entry_header(rgt, op->map->ref);
++ act = active_entry_acquire(rgt, op->ref);
++ sha = shared_entry_header(rgt, op->ref);
+
+ if ( rgt->gt_version == 1 )
+ status = &sha->flags;
+ else
+- status = &status_entry(rgt, op->map->ref);
++ status = &status_entry(rgt, op->ref);
+
+ if ( unlikely(op->frame != act->frame) )
+ {
+@@ -1289,9 +1326,6 @@ __gnttab_unmap_common_complete(struct gnttab_unmap_common *op)
+ act->pin -= GNTPIN_hstw_inc;
+ }
+
+- if ( (op->map->flags & (GNTMAP_device_map|GNTMAP_host_map)) == 0 )
+- put_handle = 1;
+-
+ if ( ((act->pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask)) == 0) &&
+ !(op->flags & GNTMAP_readonly) )
+ gnttab_clear_flag(_GTF_writing, status);
+@@ -1304,11 +1338,6 @@ __gnttab_unmap_common_complete(struct gnttab_unmap_common *op)
+ unlock_out:
+ grant_read_unlock(rgt);
+
+- if ( put_handle )
+- {
+- op->map->flags = 0;
+- put_maptrack_handle(ld->grant_table, op->handle);
+- }
+ rcu_unlock_domain(rd);
+ }
+
+--
+2.1.4
+
Copied: branches/2017Q2/emulators/xen-kernel/files/0003-gnttab-correct-logic-to-get-page-references-during-m.patch (from r443949, head/emulators/xen-kernel/files/0003-gnttab-correct-logic-to-get-page-references-during-m.patch)
==============================================================================
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
+++ branches/2017Q2/emulators/xen-kernel/files/0003-gnttab-correct-logic-to-get-page-references-during-m.patch Tue Jun 20 14:46:14 2017 (r443982, copy of r443949, head/emulators/xen-kernel/files/0003-gnttab-correct-logic-to-get-page-references-during-m.patch)
@@ -0,0 +1,186 @@
+From 5d491e3cf32ff03552db9d66e842964fec06dcd4 Mon Sep 17 00:00:00 2001
+From: George Dunlap <george.dunlap at citrix.com>
+Date: Fri, 2 Jun 2017 15:21:27 +0100
+Subject: [PATCH 3/4] gnttab: correct logic to get page references during map
+ requests
+
+The rules for reference counting are somewhat complicated:
+
+* Each of GNTTAB_host_map and GNTTAB_device_map need their own
+reference count
+
+* If the mapping is writeable:
+ - GNTTAB_host_map needs a type count under only some conditions
+ - GNTTAB_device_map always needs a type count
+
+If the mapping succeeds, we need to keep all of these; if the mapping
+fails, we need to release whatever references we have acquired so far.
+
+Additionally, the code that does a lot of this calculation "inherits"
+a reference as part of the process of finding out who the owner is.
+
+Finally, if the grant is mapped as writeable (without the
+GNTMAP_readonly flag), but the hypervisor cannot grab a
+PGT_writeable_page type, the entire operation should fail.
+
+Unfortunately, the current code has several logic holes:
+
+* If a grant is mapped only GNTTAB_device_map, and with a writeable
+ mapping, but in conditions where a *host* type count is not
+ necessary, the code will fail to grab the necessary type count.
+
+* If a grant is mapped both GNTTAB_device_map and GNTTAB_host_map,
+ with a writeable mapping, in conditions where the host type count is
+ not necessary, *and* where the page cannot be changed to type
+ PGT_writeable, the condition will not be detected.
+
+In both cases, this means that on success, the type count will be
+erroneously reduced when the grant is unmapped. In the second case,
+the type count will be erroneously reduced on the failure path as
+well. (In the first case the failure path logic has the same hole
+as the reference grabbing logic.)
+
+Additionally, the return value of get_page() is not checked; but this
+may fail even if the first get_page() succeeded due to a reference
+counting overflow.
+
+First of all, simplify the restoration logic by explicitly counting
+the reference and type references acquired.
+
+Consider each mapping type separately, explicitly marking the
+'incoming' reference as used so we know when we need to grab a second
+one.
+
+Finally, always check the return value of get_page[_type]() and go to
+the failure path if appropriate.
+
+This is part of XSA-224.
+
+Reported-by: Jan Beulich <jbeulich at suse.com>
+Signed-off-by: George Dunlap <george.dunlap at citrix.com>
+Signed-off-by: Jan Beulich <jbeulich at suse.com>
+---
+ xen/common/grant_table.c | 58 +++++++++++++++++++++++++++---------------------
+ 1 file changed, 33 insertions(+), 25 deletions(-)
+
+diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
+index 452538e..5e92e2c 100644
+--- a/xen/common/grant_table.c
++++ b/xen/common/grant_table.c
+@@ -758,12 +758,12 @@ __gnttab_map_grant_ref(
+ struct grant_table *lgt, *rgt;
+ struct vcpu *led;
+ int handle;
+- unsigned long frame = 0, nr_gets = 0;
++ unsigned long frame = 0;
+ struct page_info *pg = NULL;
+ int rc = GNTST_okay;
+ u32 old_pin;
+ u32 act_pin;
+- unsigned int cache_flags;
++ unsigned int cache_flags, refcnt = 0, typecnt = 0;
+ struct active_grant_entry *act = NULL;
+ struct grant_mapping *mt;
+ grant_entry_header_t *shah;
+@@ -889,11 +889,17 @@ __gnttab_map_grant_ref(
+ else
+ owner = page_get_owner(pg);
+
++ if ( owner )
++ refcnt++;
++
+ if ( !pg || (owner == dom_io) )
+ {
+ /* Only needed the reference to confirm dom_io ownership. */
+ if ( pg )
++ {
+ put_page(pg);
++ refcnt--;
++ }
+
+ if ( paging_mode_external(ld) )
+ {
+@@ -921,27 +927,38 @@ __gnttab_map_grant_ref(
+ }
+ else if ( owner == rd || owner == dom_cow )
+ {
+- if ( gnttab_host_mapping_get_page_type(op, ld, rd) )
++ if ( (op->flags & GNTMAP_device_map) && !(op->flags & GNTMAP_readonly) )
+ {
+ if ( (owner == dom_cow) ||
+ !get_page_type(pg, PGT_writable_page) )
+ goto could_not_pin;
++ typecnt++;
+ }
+
+- nr_gets++;
+ if ( op->flags & GNTMAP_host_map )
+ {
+- rc = create_grant_host_mapping(op->host_addr, frame, op->flags, 0);
+- if ( rc != GNTST_okay )
+- goto undo_out;
+-
++ /*
++ * Only need to grab another reference if device_map claimed
++ * the other one.
++ */
+ if ( op->flags & GNTMAP_device_map )
+ {
+- nr_gets++;
+- (void)get_page(pg, rd);
+- if ( !(op->flags & GNTMAP_readonly) )
+- get_page_type(pg, PGT_writable_page);
++ if ( !get_page(pg, rd) )
++ goto could_not_pin;
++ refcnt++;
++ }
++
++ if ( gnttab_host_mapping_get_page_type(op, ld, rd) )
++ {
++ if ( (owner == dom_cow) ||
++ !get_page_type(pg, PGT_writable_page) )
++ goto could_not_pin;
++ typecnt++;
+ }
++
++ rc = create_grant_host_mapping(op->host_addr, frame, op->flags, 0);
++ if ( rc != GNTST_okay )
++ goto undo_out;
+ }
+ }
+ else
+@@ -950,8 +967,6 @@ __gnttab_map_grant_ref(
+ if ( !rd->is_dying )
+ gdprintk(XENLOG_WARNING, "Could not pin grant frame %lx\n",
+ frame);
+- if ( owner != NULL )
+- put_page(pg);
+ rc = GNTST_general_error;
+ goto undo_out;
+ }
+@@ -1014,18 +1029,11 @@ __gnttab_map_grant_ref(
+ return;
+
+ undo_out:
+- if ( nr_gets > 1 )
+- {
+- if ( !(op->flags & GNTMAP_readonly) )
+- put_page_type(pg);
+- put_page(pg);
+- }
+- if ( nr_gets > 0 )
+- {
+- if ( gnttab_host_mapping_get_page_type(op, ld, rd) )
+- put_page_type(pg);
++ while ( typecnt-- )
++ put_page_type(pg);
++
++ while ( refcnt-- )
+ put_page(pg);
+- }
+
+ grant_read_lock(rgt);
+
+--
+2.1.4
+
Copied: branches/2017Q2/emulators/xen-kernel/files/0004-gnttab-__gnttab_unmap_common_complete-is-all-or-noth.patch (from r443949, head/emulators/xen-kernel/files/0004-gnttab-__gnttab_unmap_common_complete-is-all-or-noth.patch)
==============================================================================
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
+++ branches/2017Q2/emulators/xen-kernel/files/0004-gnttab-__gnttab_unmap_common_complete-is-all-or-noth.patch Tue Jun 20 14:46:14 2017 (r443982, copy of r443949, head/emulators/xen-kernel/files/0004-gnttab-__gnttab_unmap_common_complete-is-all-or-noth.patch)
@@ -0,0 +1,319 @@
+From 3ad26b95cd9bacedad5ba503515cf6e618162be1 Mon Sep 17 00:00:00 2001
+From: Jan Beulich <jbeulich at suse.com>
+Date: Thu, 15 Jun 2017 16:25:27 +0100
+Subject: [PATCH 4/4] gnttab: __gnttab_unmap_common_complete() is
+ all-or-nothing
+
+All failures have to be detected in __gnttab_unmap_common(), the
+completion function must not skip part of its processing. In particular
+the GNTMAP_device_map related putting of page references and adjustment
+of pin count must not occur if __gnttab_unmap_common() signaled an
+error. Furthermore the function must not make adjustments to global
+state (here: clearing GNTTAB_device_map) before all possibly failing
+operations have been performed.
+
+There's one exception for IOMMU related failures: As IOMMU manipulation
+occurs after GNTMAP_*_map have been cleared already, the related page
+reference and pin count adjustments need to be done nevertheless. A
+fundamental requirement for the correctness of this is that
+iommu_{,un}map_page() crash any affected DomU in case of failure.
+
+The version check appears to be pointless (or could perhaps be a
+BUG_ON() or ASSERT()), but for the moment also move it.
+
+This is part of XSA-224.
+
+Reported-by: Jan Beulich <jbeulich at suse.com>
+Signed-off-by: Jan Beulich <jbeulich at suse.com>
+---
+ xen/common/grant_table.c | 108 ++++++++++++++++++--------------------
+ xen/include/asm-arm/grant_table.h | 2 +-
+ xen/include/asm-x86/grant_table.h | 5 +-
+ 3 files changed, 55 insertions(+), 60 deletions(-)
+
+diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
+index 5e92e2c..025aad0 100644
+--- a/xen/common/grant_table.c
++++ b/xen/common/grant_table.c
+@@ -96,7 +96,7 @@ struct gnttab_unmap_common {
+ int16_t status;
+
+ /* Shared state beteen *_unmap and *_unmap_complete */
+- u16 flags;
++ u16 done;
+ unsigned long frame;
+ struct domain *rd;
+ grant_ref_t ref;
+@@ -948,7 +948,8 @@ __gnttab_map_grant_ref(
+ refcnt++;
+ }
+
+- if ( gnttab_host_mapping_get_page_type(op, ld, rd) )
++ if ( gnttab_host_mapping_get_page_type(op->flags & GNTMAP_readonly,
++ ld, rd) )
+ {
+ if ( (owner == dom_cow) ||
+ !get_page_type(pg, PGT_writable_page) )
+@@ -1095,6 +1096,7 @@ __gnttab_unmap_common(
+ struct active_grant_entry *act;
+ s16 rc = 0;
+ struct grant_mapping *map;
++ unsigned int flags;
+ bool_t put_handle = 0;
+
+ ld = current->domain;
+@@ -1145,6 +1147,20 @@ __gnttab_unmap_common(
+
+ grant_read_lock(rgt);
+
++ if ( rgt->gt_version == 0 )
++ {
++ /*
++ * This ought to be impossible, as such a mapping should not have
++ * been established (see the nr_grant_entries(rgt) bounds check in
++ * __gnttab_map_grant_ref()). Doing this check only in
++ * __gnttab_unmap_common_complete() - as it used to be done - would,
++ * however, be too late.
++ */
++ rc = GNTST_bad_gntref;
++ flags = 0;
++ goto unlock_out;
++ }
++
+ op->rd = rd;
+ op->ref = map->ref;
+
+@@ -1160,6 +1176,7 @@ __gnttab_unmap_common(
+ {
+ gdprintk(XENLOG_WARNING, "Unstable handle %#x\n", op->handle);
+ rc = GNTST_bad_handle;
++ flags = 0;
+ goto unlock_out;
+ }
+
+@@ -1173,9 +1190,9 @@ __gnttab_unmap_common(
+ * hold anyway; see docs/misc/grant-tables.txt's "Locking" section.
+ */
+
+- op->flags = read_atomic(&map->flags);
++ flags = read_atomic(&map->flags);
+ smp_rmb();
+- if ( unlikely(!op->flags) || unlikely(map->domid != dom) ||
++ if ( unlikely(!flags) || unlikely(map->domid != dom) ||
+ unlikely(map->ref != op->ref) )
+ {
+ gdprintk(XENLOG_WARNING, "Unstable handle %#x\n", op->handle);
+@@ -1185,24 +1202,27 @@ __gnttab_unmap_common(
+
+ op->frame = act->frame;
+
+- if ( op->dev_bus_addr )
+- {
+- if ( unlikely(op->dev_bus_addr != pfn_to_paddr(act->frame)) )
+- PIN_FAIL(act_release_out, GNTST_general_error,
+- "Bus address doesn't match gntref (%"PRIx64" != %"PRIpaddr")\n",
+- op->dev_bus_addr, pfn_to_paddr(act->frame));
+-
+- map->flags &= ~GNTMAP_device_map;
+- }
++ if ( op->dev_bus_addr &&
++ unlikely(op->dev_bus_addr != pfn_to_paddr(act->frame)) )
++ PIN_FAIL(act_release_out, GNTST_general_error,
++ "Bus address doesn't match gntref (%"PRIx64" != %"PRIpaddr")\n",
++ op->dev_bus_addr, pfn_to_paddr(act->frame));
+
+- if ( (op->host_addr != 0) && (op->flags & GNTMAP_host_map) )
++ if ( op->host_addr && (flags & GNTMAP_host_map) )
+ {
+ if ( (rc = replace_grant_host_mapping(op->host_addr,
+ op->frame, op->new_addr,
+- op->flags)) < 0 )
++ flags)) < 0 )
+ goto act_release_out;
+
+ map->flags &= ~GNTMAP_host_map;
++ op->done |= GNTMAP_host_map | (flags & GNTMAP_readonly);
++ }
++
++ if ( op->dev_bus_addr && (flags & GNTMAP_device_map) )
++ {
++ map->flags &= ~GNTMAP_device_map;
++ op->done |= GNTMAP_device_map | (flags & GNTMAP_readonly);
+ }
+
+ if ( !(map->flags & (GNTMAP_device_map|GNTMAP_host_map)) )
+@@ -1239,7 +1259,7 @@ __gnttab_unmap_common(
+ }
+
+ /* If just unmapped a writable mapping, mark as dirtied */
+- if ( rc == GNTST_okay && !(op->flags & GNTMAP_readonly) )
++ if ( rc == GNTST_okay && !(flags & GNTMAP_readonly) )
+ gnttab_mark_dirty(rd, op->frame);
+
+ op->status = rc;
+@@ -1256,13 +1276,9 @@ __gnttab_unmap_common_complete(struct gnttab_unmap_common *op)
+ struct page_info *pg;
+ uint16_t *status;
+
+- if ( rd == NULL )
++ if ( !op->done )
+ {
+- /*
+- * Suggests that __gntab_unmap_common failed in
+- * rcu_lock_domain_by_id() or earlier, and so we have nothing
+- * to complete
+- */
++ /* __gntab_unmap_common() didn't do anything - nothing to complete. */
+ return;
+ }
+
+@@ -1272,8 +1288,6 @@ __gnttab_unmap_common_complete(struct gnttab_unmap_common *op)
+ rgt = rd->grant_table;
+
+ grant_read_lock(rgt);
+- if ( rgt->gt_version == 0 )
+- goto unlock_out;
+
+ act = active_entry_acquire(rgt, op->ref);
+ sha = shared_entry_header(rgt, op->ref);
+@@ -1283,72 +1297,50 @@ __gnttab_unmap_common_complete(struct gnttab_unmap_common *op)
+ else
+ status = &status_entry(rgt, op->ref);
+
+- if ( op->dev_bus_addr &&
+- unlikely(op->dev_bus_addr != pfn_to_paddr(act->frame)) )
+- {
+- /*
+- * Suggests that __gntab_unmap_common failed early and so
+- * nothing further to do
+- */
+- goto act_release_out;
+- }
+-
+ pg = mfn_to_page(op->frame);
+
*** DIFF OUTPUT TRUNCATED AT 1000 LINES ***
More information about the svn-ports-branches
mailing list