[pve-devel] [PATCH kernel] cherry-pick fix for CVE-2016-3951 in usbnet driver

Fabian Grünbichler f.gruenbichler at proxmox.com
Thu Apr 21 09:21:49 CEST 2016 

---
cherry-picked from ubuntu xenial master-next

 ...ption-triggered-by-invalid-USB-descriptor.patch | 138 +++++++++++++++++++++
 Makefile                                           |   1 +
 2 files changed, 139 insertions(+)
 create mode 100644 CVE-2016-3951-usbnet-memory-corruption-triggered-by-invalid-USB-descriptor.patch

diff --git a/CVE-2016-3951-usbnet-memory-corruption-triggered-by-invalid-USB-descriptor.patch b/CVE-2016-3951-usbnet-memory-corruption-triggered-by-invalid-USB-descriptor.patch
new file mode 100644
index 0000000..ae96f4d
--- /dev/null
+++ b/CVE-2016-3951-usbnet-memory-corruption-triggered-by-invalid-USB-descriptor.patch
@@ -0,0 +1,138 @@
+From 889c172b1e097eceefc5d9d3639c3862c98c6753 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Bj=C3=B8rn=20Mork?= <bjorn at mork.no>
+Date: Wed, 20 Apr 2016 11:15:11 +0100
+Subject: [PATCH 1/2] cdc_ncm: do not call usbnet_link_change from cdc_ncm_bind
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+usbnet_link_change will call schedule_work and should be
+avoided if bind is failing. Otherwise we will end up with
+scheduled work referring to a netdev which has gone away.
+
+Instead of making the call conditional, we can just defer
+it to usbnet_probe, using the driver_info flag made for
+this purpose.
+
+Fixes: 8a34b0ae8778 ("usbnet: cdc_ncm: apply usbnet_link_change")
+Reported-by: Andrey Konovalov <andreyknvl at gmail.com>
+Suggested-by: Linus Torvalds <torvalds at linux-foundation.org>
+Signed-off-by: Bjørn Mork <bjorn at mork.no>
+Signed-off-by: David S. Miller <davem at davemloft.net>
+(cherry picked from commit 4d06dd537f95683aba3651098ae288b7cbff8274)
+CVE-2016-3951
+BugLink: https://bugs.launchpad.net/bugs/1567191
+Signed-off-by: Luis Henriques <luis.henriques at canonical.com>
+Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
+Signed-off-by: Kamal Mostafa <kamal at canonical.com>
+---
+ drivers/net/usb/cdc_ncm.c | 20 +++++---------------
+ 1 file changed, 5 insertions(+), 15 deletions(-)
+
+diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
+index e8a1144..93c88a2 100644
+--- a/drivers/net/usb/cdc_ncm.c
++++ b/drivers/net/usb/cdc_ncm.c
+@@ -941,8 +941,6 @@ EXPORT_SYMBOL_GPL(cdc_ncm_select_altsetting);
+ 
+ static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
+ {
+-	int ret;
+-
+ 	/* MBIM backwards compatible function? */
+ 	if (cdc_ncm_select_altsetting(intf) != CDC_NCM_COMM_ALTSETTING_NCM)
+ 		return -ENODEV;
+@@ -951,16 +949,7 @@ static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
+ 	 * Additionally, generic NCM devices are assumed to accept arbitrarily
+ 	 * placed NDP.
+ 	 */
+-	ret = cdc_ncm_bind_common(dev, intf, CDC_NCM_DATA_ALTSETTING_NCM, 0);
+-
+-	/*
+-	 * We should get an event when network connection is "connected" or
+-	 * "disconnected". Set network connection in "disconnected" state
+-	 * (carrier is OFF) during attach, so the IP network stack does not
+-	 * start IPv6 negotiation and more.
+-	 */
+-	usbnet_link_change(dev, 0, 0);
+-	return ret;
++	return cdc_ncm_bind_common(dev, intf, CDC_NCM_DATA_ALTSETTING_NCM, 0);
+ }
+ 
+ static void cdc_ncm_align_tail(struct sk_buff *skb, size_t modulus, size_t remainder, size_t max)
+@@ -1543,7 +1532,8 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
+ 
+ static const struct driver_info cdc_ncm_info = {
+ 	.description = "CDC NCM",
+-	.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET,
++	.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
++			| FLAG_LINK_INTR,
+ 	.bind = cdc_ncm_bind,
+ 	.unbind = cdc_ncm_unbind,
+ 	.manage_power = usbnet_manage_power,
+@@ -1556,7 +1546,7 @@ static const struct driver_info cdc_ncm_info = {
+ static const struct driver_info wwan_info = {
+ 	.description = "Mobile Broadband Network Device",
+ 	.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
+-			| FLAG_WWAN,
++			| FLAG_LINK_INTR | FLAG_WWAN,
+ 	.bind = cdc_ncm_bind,
+ 	.unbind = cdc_ncm_unbind,
+ 	.manage_power = usbnet_manage_power,
+@@ -1569,7 +1559,7 @@ static const struct driver_info wwan_info = {
+ static const struct driver_info wwan_noarp_info = {
+ 	.description = "Mobile Broadband Network Device (NO ARP)",
+ 	.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
+-			| FLAG_WWAN | FLAG_NOARP,
++			| FLAG_LINK_INTR | FLAG_WWAN | FLAG_NOARP,
+ 	.bind = cdc_ncm_bind,
+ 	.unbind = cdc_ncm_unbind,
+ 	.manage_power = usbnet_manage_power,
+-- 
+2.1.4
+
+From ac6b36fbfad65378b81338637254f0d23b35e2a1 Mon Sep 17 00:00:00 2001
+From: Oliver Neukum <oneukum at suse.com>
+Date: Wed, 20 Apr 2016 11:15:12 +0100
+Subject: [PATCH 2/2] usbnet: cleanup after bind() in probe()
+
+In case bind() works, but a later error forces bailing
+in probe() in error cases work and a timer may be scheduled.
+They must be killed. This fixes an error case related to
+the double free reported in
+http://www.spinics.net/lists/netdev/msg367669.html
+and needs to go on top of Linus' fix to cdc-ncm.
+
+Signed-off-by: Oliver Neukum <ONeukum at suse.com>
+Signed-off-by: David S. Miller <davem at davemloft.net>
+(cherry picked from commit 1666984c8625b3db19a9abc298931d35ab7bc64b)
+CVE-2016-3951
+BugLink: https://bugs.launchpad.net/bugs/1567191
+Signed-off-by: Luis Henriques <luis.henriques at canonical.com>
+Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
+Signed-off-by: Kamal Mostafa <kamal at canonical.com>
+---
+ drivers/net/usb/usbnet.c | 7 +++++++
+ 1 file changed, 7 insertions(+)
+
+diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
+index 0744bf2..c2ea4e5 100644
+--- a/drivers/net/usb/usbnet.c
++++ b/drivers/net/usb/usbnet.c
+@@ -1766,6 +1766,13 @@ out3:
+ 	if (info->unbind)
+ 		info->unbind (dev, udev);
+ out1:
++	/* subdrivers must undo all they did in bind() if they
++	 * fail it, but we may fail later and a deferred kevent
++	 * may trigger an error resubmitting itself and, worse,
++	 * schedule a timer. So we kill it all just in case.
++	 */
++	cancel_work_sync(&dev->kevent);
++	del_timer_sync(&dev->delay);
+ 	free_netdev(net);
+ out:
+ 	return status;
+-- 
+2.1.4
+
diff --git a/Makefile b/Makefile
index 1c7a621..52365e9 100644
--- a/Makefile
+++ b/Makefile
@@ -237,6 +237,7 @@ ${KERNEL_SRC}/README ${KERNEL_CFG_ORG}: ${KERNELSRCTAR}
 	cd ${KERNEL_SRC}; patch -p1 <../override_for_missing_acs_capabilities.patch
 	#cd ${KERNEL_SRC}; patch -p1 <../vhost-net-extend-device-allocation-to-vmalloc.patch
 	cd ${KERNEL_SRC}; patch -p1 <../CVE-2016-3955-usbip-fix-potential-out-of-bound-write.patch
+	cd ${KERNEL_SRC}; patch -p1 <../CVE-2016-3951-usbnet-memory-corruption-triggered-by-invalid-USB-descriptor.patch
 	sed -i ${KERNEL_SRC}/Makefile -e 's/^EXTRAVERSION.*$$/EXTRAVERSION=${EXTRAVERSION}/'
 	touch $@
 
-- 
2.1.4

More information about the pve-devel mailing list