Xen Security Advisory 367 v2 (CVE-2021-28038) - Linux: netback fails to honor grant mapping errors

Xen Security Advisory 367 v2 (CVE-2021-28038) - Linux: netback fails to honor grant mapping errors

[Xen.org security team]

[-- Attachment #1: Type: text/plain, Size: 2369 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

            Xen Security Advisory CVE-2021-28038 / XSA-367
                              version 2

          Linux: netback fails to honor grant mapping errors

UPDATES IN VERSION 2
====================

CVE assigned.

ISSUE DESCRIPTION
=================

XSA-362 tried to address issues here, but in the case of the netback
driver the changes were insufficient: It left the relevant function
invocation with, effectively, no error handling at all.  As a result,
memory allocation failures there could still lead to frontend-induced
crashes of the backend.

IMPACT
======

A malicious or buggy networking frontend driver may be able to crash
the corresponding backend driver, potentially affecting the entire
domain running the backend driver.  In a typical (non-disaggregated)
system that is a host-wide denial of service (DoS).

VULNERABLE SYSTEMS
==================

Linux versions from at least 2.6.39 onwards are vulnerable, when run in
PV mode.  Earlier versions differ significantly in behavior and may
therefore instead surface other issues under the same conditions.  Linux
run in HVM / PVH modes is not vulnerable.

MITIGATION
==========

For Linux, running the backends in HVM or PVH domains will avoid the
vulnerability.  For example, by running the dom0 in PVH mode.

In all other cases there is no known mitigation.

RESOLUTION
==========

Applying the attached patch resolves this issue.

xsa367-linux.patch           Linux 5.12-rc

$ sha256sum xsa367*
b0244bfddee91cd7986172893e70664b74e698c5d44f25865870f179f80f9a92  xsa367-linux.patch
$

CREDITS
=======

This issue was reported by Intel's kernel test robot and recognized as a
security issue by Jan Beulich of SUSE.

NOTE REGARDING LACK OF EMBARGO
==============================

This issue was reported publicly, before the XSA could be issued.
-----BEGIN PGP SIGNATURE-----

iQFABAEBCAAqFiEEI+MiLBRfRHX6gGCng/4UyVfoK9kFAmBCZVEMHHBncEB4ZW4u
b3JnAAoJEIP+FMlX6CvZfqAH/i7ypTUP90UIxeyMB9XmNRiqD+LaTSBExt8xTowd
zbsWrxFYnZRPSLqs/dVHlDQfF65eD40Agh/Hxp5f0hGHjv8x1kepvpo2di1ovA2h
C8/WpOK2nFq77/GTG2mAsJA3ltDF0WJsr5oqaBNVf/lwQSmiescTWtI6+LDFmmpd
q1EyKPUClKZW3PoZkCVmiWDtqhVJc3LaJJcy4x/Zd4EgV+uGi2wsYsiQzObrwPss
2D5laUr8RJcSTE7+bXlMA8KnzrOZ6UqK1YIPSGIYBOJnhizGf9CBZCxcNTONWQFC
zh1d9GAv93fugE37xRHE7PRjgl/RVO5rn0k5EQw5GTa676A=
=GKdV
-----END PGP SIGNATURE-----

[-- Attachment #2: xsa367-linux.patch --]
[-- Type: application/octet-stream, Size: 3974 bytes --]

From: Jan Beulich <jbeulich@suse.com>
Subject: Xen/gnttab: handle p2m update errors on a per-slot basis

Bailing immediately from set_foreign_p2m_mapping() upon a p2m updating
error leaves the full batch in an ambiguous state as far as the caller
is concerned. Instead flags respective slots as bad, unmapping what
was mapped there right away.

HYPERVISOR_grant_table_op()'s return value and the individual unmap
slots' status fields get used only for a one-time - there's not much we
can do in case of a failure.

Note that there's no GNTST_enomem or alike, so GNTST_general_error gets
used.

The map ops' handle fields get overwritten just to be on the safe side.

This is XSA-367.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Wei Liu <wei.liu@kernel.org>
---
v2: Log message. Invalidate map ops' handles.

--- a/arch/arm/xen/p2m.c
+++ b/arch/arm/xen/p2m.c
@@ -93,12 +93,39 @@ int set_foreign_p2m_mapping(struct gntta
 	int i;
 
 	for (i = 0; i < count; i++) {
+		struct gnttab_unmap_grant_ref unmap;
+		int rc;
+
 		if (map_ops[i].status)
 			continue;
-		if (unlikely(!set_phys_to_machine(map_ops[i].host_addr >> XEN_PAGE_SHIFT,
-				    map_ops[i].dev_bus_addr >> XEN_PAGE_SHIFT))) {
-			return -ENOMEM;
-		}
+		if (likely(set_phys_to_machine(map_ops[i].host_addr >> XEN_PAGE_SHIFT,
+				    map_ops[i].dev_bus_addr >> XEN_PAGE_SHIFT)))
+			continue;
+
+		/*
+		 * Signal an error for this slot. This in turn requires
+		 * immediate unmapping.
+		 */
+		map_ops[i].status = GNTST_general_error;
+		unmap.host_addr = map_ops[i].host_addr,
+		unmap.handle = map_ops[i].handle;
+		map_ops[i].handle = ~0;
+		if (map_ops[i].flags & GNTMAP_device_map)
+			unmap.dev_bus_addr = map_ops[i].dev_bus_addr;
+		else
+			unmap.dev_bus_addr = 0;
+
+		/*
+		 * Pre-populate the status field, to be recognizable in
+		 * the log message below.
+		 */
+		unmap.status = 1;
+
+		rc = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref,
+					       &unmap, 1);
+		if (rc || unmap.status != GNTST_okay)
+			pr_err_once("gnttab unmap failed: rc=%d st=%d\n",
+				    rc, unmap.status);
 	}
 
 	return 0;
--- a/arch/x86/xen/p2m.c
+++ b/arch/x86/xen/p2m.c
@@ -710,6 +710,8 @@ int set_foreign_p2m_mapping(struct gntta
 
 	for (i = 0; i < count; i++) {
 		unsigned long mfn, pfn;
+		struct gnttab_unmap_grant_ref unmap[2];
+		int rc;
 
 		/* Do not add to override if the map failed. */
 		if (map_ops[i].status != GNTST_okay ||
@@ -727,10 +729,46 @@ int set_foreign_p2m_mapping(struct gntta
 
 		WARN(pfn_to_mfn(pfn) != INVALID_P2M_ENTRY, "page must be ballooned");
 
-		if (unlikely(!set_phys_to_machine(pfn, FOREIGN_FRAME(mfn)))) {
-			ret = -ENOMEM;
-			goto out;
+		if (likely(set_phys_to_machine(pfn, FOREIGN_FRAME(mfn))))
+			continue;
+
+		/*
+		 * Signal an error for this slot. This in turn requires
+		 * immediate unmapping.
+		 */
+		map_ops[i].status = GNTST_general_error;
+		unmap[0].host_addr = map_ops[i].host_addr,
+		unmap[0].handle = map_ops[i].handle;
+		map_ops[i].handle = ~0;
+		if (map_ops[i].flags & GNTMAP_device_map)
+			unmap[0].dev_bus_addr = map_ops[i].dev_bus_addr;
+		else
+			unmap[0].dev_bus_addr = 0;
+
+		if (kmap_ops) {
+			kmap_ops[i].status = GNTST_general_error;
+			unmap[1].host_addr = kmap_ops[i].host_addr,
+			unmap[1].handle = kmap_ops[i].handle;
+			kmap_ops[i].handle = ~0;
+			if (kmap_ops[i].flags & GNTMAP_device_map)
+				unmap[1].dev_bus_addr = kmap_ops[i].dev_bus_addr;
+			else
+				unmap[1].dev_bus_addr = 0;
 		}
+
+		/*
+		 * Pre-populate both status fields, to be recognizable in
+		 * the log message below.
+		 */
+		unmap[0].status = 1;
+		unmap[1].status = 1;
+
+		rc = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref,
+					       unmap, 1 + !!kmap_ops);
+		if (rc || unmap[0].status != GNTST_okay ||
+		    unmap[1].status != GNTST_okay)
+			pr_err_once("gnttab unmap failed: rc=%d st0=%d st1=%d\n",
+				    rc, unmap[0].status, unmap[1].status);
 	}
 
 out: