Xen Security Advisory 270 v3 (CVE-2018-15471) - Linux netback driver OOB access in hash handling

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

            Xen Security Advisory CVE-2018-15471 / XSA-270
                              version 3

           Linux netback driver OOB access in hash handling

UPDATES IN VERSION 3
====================

CVE assigned.

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

Linux's netback driver allows frontends to control mapping of requests
to request queues.  When processing a request to set or change this
mapping, some input validation was missing or flawed.

IMPACT
======

A malicious or buggy frontend may cause the (usually privileged)
backend to make out of bounds memory accesses, potentially resulting
in one or more of privilege escalation, Denial of Service (DoS), or
information leaks.

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

Linux kernel versions from 4.7 onwards are affected.

MITIGATION
==========

There is no known mitigation.

CREDITS
=======

This issue was discovered by Felix Wilhelm of Google Project Zero.

RESOLUTION
==========

Applying the attached patch resolves this issue.

xsa270.patch           Linux 4.7 ... 4.17

$ sha256sum xsa270*
392868c37c1fe0d16c36086208fd0fc045c1baf8ab9b207995bce72681cb8c54  xsa270.patch
$

DEPLOYMENT DURING EMBARGO
=========================

Deployment of the patches and/or mitigations described above (or
others which are substantially similar) is permitted during the
embargo, even on public-facing systems with untrusted guest users and
administrators.

But: Distribution of updated software is prohibited (except to other
members of the predisclosure list).

Predisclosure list members who wish to deploy significantly different
patches and/or mitigations, please contact the Xen Project Security
Team.

(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable.  This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)

For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
  http://www.xenproject.org/security-policy.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBCAAGBQJbeo4MAAoJEIP+FMlX6CvZOpsH/34RpIZaTTVsZWCVyNotieFf
yLfCqu+9bbRVNEqYDq6NViFrj9I6WwvLpp8s7HZheJvdXlyIO1cYCen4QX8VSPqI
VaRD7Jcu99drK1hy/t80AbicS+t9qvew97SzjG+MIIJZK7dnxG/Q0nbHLCg0zdCg
5G+pOTl17DK+4eM7Z1duo2BK1sxCms6I/YJVFfkGjC99vXKYAj2GAWGxVbiEwDWT
4jvf3R3w5athJNR4Lf6FxDz6MzvHaYNFQKikc0AMaTcO5HubumGXQQn5JQelAAno
O6ujB25kF1j29A2PwYvBSxBDTD4uWQeWiv9kWML1YmzsQv1cy6Un0vwXtNhhb6s=
=SC+y
-----END PGP SIGNATURE-----

_______________________________________________
Xen-announce mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-announce

From: Jan Beulich <[email protected]>
Subject: xen-netback: fix input validation in xenvif_set_hash_mapping()

Both len and off are frontend specified values, so we need to make
sure there's no overflow when adding the two for the bounds check. We
also want to avoid undefined behavior and hence use off to index into
->hash.mapping[] only after bounds checking. This at the same time
allows to take care of not applying off twice for the bounds checking
against vif->num_queues.

It is also insufficient to bounds check copy_op.len, as this is len
truncated to 16 bits.

This is XSA-270.

Reported-by: Felix Wilhelm <[email protected]>
Signed-off-by: Jan Beulich <[email protected]>
Reviewed-by: Paul Durrant <[email protected]>
Tested-by: Paul Durrant <[email protected]>
---
The bounds checking against vif->num_queues also occurs too early afaict
(it should be done after the grant copy). I have patches ready as public
follow-ups for both this and the (at least latent) issue of the mapping
array crossing a page boundary.

--- a/drivers/net/xen-netback/hash.c
+++ b/drivers/net/xen-netback/hash.c
@@ -332,20 +332,22 @@ u32 xenvif_set_hash_mapping_size(struct
 u32 xenvif_set_hash_mapping(struct xenvif *vif, u32 gref, u32 len,
 			    u32 off)
 {
-	u32 *mapping = &vif->hash.mapping[off];
+	u32 *mapping = vif->hash.mapping;
 	struct gnttab_copy copy_op = {
 		.source.u.ref = gref,
 		.source.domid = vif->domid,
-		.dest.u.gmfn = virt_to_gfn(mapping),
 		.dest.domid = DOMID_SELF,
-		.dest.offset = xen_offset_in_page(mapping),
-		.len = len * sizeof(u32),
+		.len = len * sizeof(*mapping),
 		.flags = GNTCOPY_source_gref
 	};
 
-	if ((off + len > vif->hash.size) || copy_op.len > XEN_PAGE_SIZE)
+	if ((off + len < off) || (off + len > vif->hash.size) ||
+	    len > XEN_PAGE_SIZE / sizeof(*mapping))
 		return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
 
+	copy_op.dest.u.gmfn = virt_to_gfn(mapping + off);
+	copy_op.dest.offset = xen_offset_in_page(mapping + off);
+
 	while (len-- != 0)
 		if (mapping[off++] >= vif->num_queues)
 			return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;