linux-kernel - [PATCH 4.18 11/44] xen-netback: fix input validation in xenvif_set_hash

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 11 Oct 2018 17:39:49 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     linux-kernel@...r.kernel.org
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        stable@...r.kernel.org, Felix Wilhelm <fwilhelm@...gle.com>,
        Jan Beulich <jbeulich@...e.com>,
        Paul Durrant <paul.durrant@...rix.com>,
        "David S. Miller" <davem@...emloft.net>
Subject: [PATCH 4.18 11/44] xen-netback: fix input validation in xenvif_set_hash_mapping()

4.18-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Jan Beulich <JBeulich@...e.com>

commit 780e83c259fc33e8959fed8dfdad17e378d72b62 upstream.

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 / CVE-2018-15471.

Reported-by: Felix Wilhelm <fwilhelm@...gle.com>
Signed-off-by: Jan Beulich <jbeulich@...e.com>
Reviewed-by: Paul Durrant <paul.durrant@...rix.com>
Tested-by: Paul Durrant <paul.durrant@...rix.com>
Cc: stable@...r.kernel.org [4.7 onwards]
Signed-off-by: David S. Miller <davem@...emloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>

---
 drivers/net/xen-netback/hash.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

--- 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;