linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 4.4 0/4] Backport missing sccurity and deadlock fix
@ 2018-01-25 18:37 Shuah Khan
  2018-01-25 18:37 ` [PATCH 4.4 1/4] usb: usbip: Fix possible deadlocks reported by lockdep Shuah Khan
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Shuah Khan @ 2018-01-25 18:37 UTC (permalink / raw)
  To: valentina.manea.m, shuah, gregkh
  Cc: Shuah Khan, linux-usb, linux-kernel, stable

As I started backporting security fixes, I found a deadlock bug that was
fixed in a later release. This patch series contains backports for all
these problems.

Andrew Goodbody (1):
  usb: usbip: Fix possible deadlocks reported by lockdep

Shuah Khan (3):
  usbip: fix stub_rx: get_pipe() to validate endpoint number
  usbip: fix stub_rx: harden CMD_SUBMIT path to handle malicious input
  usbip: prevent leaking socket pointer address in messages

 drivers/usb/usbip/stub_dev.c     |  3 +-
 drivers/usb/usbip/stub_rx.c      | 46 ++++++++++++++++----
 drivers/usb/usbip/usbip_common.c | 15 ++-----
 drivers/usb/usbip/usbip_event.c  |  5 ++-
 drivers/usb/usbip/vhci_hcd.c     | 90 +++++++++++++++++++++++-----------------
 drivers/usb/usbip/vhci_rx.c      | 30 ++++++++------
 drivers/usb/usbip/vhci_sysfs.c   | 19 +++++----
 drivers/usb/usbip/vhci_tx.c      | 14 ++++---
 8 files changed, 134 insertions(+), 88 deletions(-)

-- 
2.14.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 4.4 1/4] usb: usbip: Fix possible deadlocks reported by lockdep
  2018-01-25 18:37 [PATCH 4.4 0/4] Backport missing sccurity and deadlock fix Shuah Khan
@ 2018-01-25 18:37 ` Shuah Khan
  2018-01-25 18:37 ` [PATCH 4.4 2/4] usbip: fix stub_rx: get_pipe() to validate endpoint number Shuah Khan
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Shuah Khan @ 2018-01-25 18:37 UTC (permalink / raw)
  To: valentina.manea.m, shuah, gregkh
  Cc: Andrew Goodbody, linux-usb, linux-kernel, stable, Shuah Khan

From: Andrew Goodbody <andrew.goodbody@cambrionix.com>

Upstream commit 21619792d1ec ("usb: usbip: Fix possible deadlocks
reported by lockdep")

Change spin_lock calls to spin_lock_irqsave to prevent
attmpted recursive lock taking in interrupt context.

This patch fixes Bug 109351
  https://bugzilla.kernel.org/show_bug.cgi?id=109351

Signed-off-by: Andrew Goodbody <andrew.goodbody@cambrionix.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 drivers/usb/usbip/usbip_event.c |  5 ++-
 drivers/usb/usbip/vhci_hcd.c    | 88 ++++++++++++++++++++++++-----------------
 drivers/usb/usbip/vhci_rx.c     | 30 ++++++++------
 drivers/usb/usbip/vhci_sysfs.c  | 19 +++++----
 drivers/usb/usbip/vhci_tx.c     | 14 ++++---
 5 files changed, 91 insertions(+), 65 deletions(-)

diff --git a/drivers/usb/usbip/usbip_event.c b/drivers/usb/usbip/usbip_event.c
index 64933b993d7a..2580a32bcdff 100644
--- a/drivers/usb/usbip/usbip_event.c
+++ b/drivers/usb/usbip/usbip_event.c
@@ -117,11 +117,12 @@ EXPORT_SYMBOL_GPL(usbip_event_add);
 int usbip_event_happened(struct usbip_device *ud)
 {
 	int happened = 0;
+	unsigned long flags;
 
-	spin_lock(&ud->lock);
+	spin_lock_irqsave(&ud->lock, flags);
 	if (ud->event != 0)
 		happened = 1;
-	spin_unlock(&ud->lock);
+	spin_unlock_irqrestore(&ud->lock, flags);
 
 	return happened;
 }
diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index f9af04d7f02f..0aaa8e524afd 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -121,9 +121,11 @@ static void dump_port_status_diff(u32 prev_status, u32 new_status)
 
 void rh_port_connect(int rhport, enum usb_device_speed speed)
 {
+	unsigned long	flags;
+
 	usbip_dbg_vhci_rh("rh_port_connect %d\n", rhport);
 
-	spin_lock(&the_controller->lock);
+	spin_lock_irqsave(&the_controller->lock, flags);
 
 	the_controller->port_status[rhport] |= USB_PORT_STAT_CONNECTION
 		| (1 << USB_PORT_FEAT_C_CONNECTION);
@@ -139,22 +141,24 @@ void rh_port_connect(int rhport, enum usb_device_speed speed)
 		break;
 	}
 
-	spin_unlock(&the_controller->lock);
+	spin_unlock_irqrestore(&the_controller->lock, flags);
 
 	usb_hcd_poll_rh_status(vhci_to_hcd(the_controller));
 }
 
 static void rh_port_disconnect(int rhport)
 {
+	unsigned long	flags;
+
 	usbip_dbg_vhci_rh("rh_port_disconnect %d\n", rhport);
 
-	spin_lock(&the_controller->lock);
+	spin_lock_irqsave(&the_controller->lock, flags);
 
 	the_controller->port_status[rhport] &= ~USB_PORT_STAT_CONNECTION;
 	the_controller->port_status[rhport] |=
 					(1 << USB_PORT_FEAT_C_CONNECTION);
 
-	spin_unlock(&the_controller->lock);
+	spin_unlock_irqrestore(&the_controller->lock, flags);
 	usb_hcd_poll_rh_status(vhci_to_hcd(the_controller));
 }
 
@@ -182,13 +186,14 @@ static int vhci_hub_status(struct usb_hcd *hcd, char *buf)
 	int		retval;
 	int		rhport;
 	int		changed = 0;
+	unsigned long	flags;
 
 	retval = DIV_ROUND_UP(VHCI_NPORTS + 1, 8);
 	memset(buf, 0, retval);
 
 	vhci = hcd_to_vhci(hcd);
 
-	spin_lock(&vhci->lock);
+	spin_lock_irqsave(&vhci->lock, flags);
 	if (!HCD_HW_ACCESSIBLE(hcd)) {
 		usbip_dbg_vhci_rh("hw accessible flag not on?\n");
 		goto done;
@@ -209,7 +214,7 @@ static int vhci_hub_status(struct usb_hcd *hcd, char *buf)
 		usb_hcd_resume_root_hub(hcd);
 
 done:
-	spin_unlock(&vhci->lock);
+	spin_unlock_irqrestore(&vhci->lock, flags);
 	return changed ? retval : 0;
 }
 
@@ -236,6 +241,7 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 	struct vhci_hcd	*dum;
 	int             retval = 0;
 	int		rhport;
+	unsigned long	flags;
 
 	u32 prev_port_status[VHCI_NPORTS];
 
@@ -254,7 +260,7 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 
 	dum = hcd_to_vhci(hcd);
 
-	spin_lock(&dum->lock);
+	spin_lock_irqsave(&dum->lock, flags);
 
 	/* store old status and compare now and old later */
 	if (usbip_dbg_flag_vhci_rh) {
@@ -408,7 +414,7 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 	}
 	usbip_dbg_vhci_rh(" bye\n");
 
-	spin_unlock(&dum->lock);
+	spin_unlock_irqrestore(&dum->lock, flags);
 
 	return retval;
 }
@@ -431,6 +437,7 @@ static void vhci_tx_urb(struct urb *urb)
 {
 	struct vhci_device *vdev = get_vdev(urb->dev);
 	struct vhci_priv *priv;
+	unsigned long flags;
 
 	if (!vdev) {
 		pr_err("could not get virtual device");
@@ -443,7 +450,7 @@ static void vhci_tx_urb(struct urb *urb)
 		return;
 	}
 
-	spin_lock(&vdev->priv_lock);
+	spin_lock_irqsave(&vdev->priv_lock, flags);
 
 	priv->seqnum = atomic_inc_return(&the_controller->seqnum);
 	if (priv->seqnum == 0xffff)
@@ -457,7 +464,7 @@ static void vhci_tx_urb(struct urb *urb)
 	list_add_tail(&priv->list, &vdev->priv_tx);
 
 	wake_up(&vdev->waitq_tx);
-	spin_unlock(&vdev->priv_lock);
+	spin_unlock_irqrestore(&vdev->priv_lock, flags);
 }
 
 static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
@@ -466,15 +473,16 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
 	struct device *dev = &urb->dev->dev;
 	int ret = 0;
 	struct vhci_device *vdev;
+	unsigned long flags;
 
 	/* patch to usb_sg_init() is in 2.5.60 */
 	BUG_ON(!urb->transfer_buffer && urb->transfer_buffer_length);
 
-	spin_lock(&the_controller->lock);
+	spin_lock_irqsave(&the_controller->lock, flags);
 
 	if (urb->status != -EINPROGRESS) {
 		dev_err(dev, "URB already unlinked!, status %d\n", urb->status);
-		spin_unlock(&the_controller->lock);
+		spin_unlock_irqrestore(&the_controller->lock, flags);
 		return urb->status;
 	}
 
@@ -486,7 +494,7 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
 	    vdev->ud.status == VDEV_ST_ERROR) {
 		dev_err(dev, "enqueue for inactive port %d\n", vdev->rhport);
 		spin_unlock(&vdev->ud.lock);
-		spin_unlock(&the_controller->lock);
+		spin_unlock_irqrestore(&the_controller->lock, flags);
 		return -ENODEV;
 	}
 	spin_unlock(&vdev->ud.lock);
@@ -559,14 +567,14 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
 
 out:
 	vhci_tx_urb(urb);
-	spin_unlock(&the_controller->lock);
+	spin_unlock_irqrestore(&the_controller->lock, flags);
 
 	return 0;
 
 no_need_xmit:
 	usb_hcd_unlink_urb_from_ep(hcd, urb);
 no_need_unlink:
-	spin_unlock(&the_controller->lock);
+	spin_unlock_irqrestore(&the_controller->lock, flags);
 	if (!ret)
 		usb_hcd_giveback_urb(vhci_to_hcd(the_controller),
 				     urb, urb->status);
@@ -623,14 +631,15 @@ static int vhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 {
 	struct vhci_priv *priv;
 	struct vhci_device *vdev;
+	unsigned long flags;
 
-	spin_lock(&the_controller->lock);
+	spin_lock_irqsave(&the_controller->lock, flags);
 
 	priv = urb->hcpriv;
 	if (!priv) {
 		/* URB was never linked! or will be soon given back by
 		 * vhci_rx. */
-		spin_unlock(&the_controller->lock);
+		spin_unlock_irqrestore(&the_controller->lock, flags);
 		return -EIDRM;
 	}
 
@@ -639,7 +648,7 @@ static int vhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 
 		ret = usb_hcd_check_unlink_urb(hcd, urb, status);
 		if (ret) {
-			spin_unlock(&the_controller->lock);
+			spin_unlock_irqrestore(&the_controller->lock, flags);
 			return ret;
 		}
 	}
@@ -664,10 +673,10 @@ static int vhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 		 */
 		usb_hcd_unlink_urb_from_ep(hcd, urb);
 
-		spin_unlock(&the_controller->lock);
+		spin_unlock_irqrestore(&the_controller->lock, flags);
 		usb_hcd_giveback_urb(vhci_to_hcd(the_controller), urb,
 				     urb->status);
-		spin_lock(&the_controller->lock);
+		spin_lock_irqsave(&the_controller->lock, flags);
 
 	} else {
 		/* tcp connection is alive */
@@ -679,7 +688,7 @@ static int vhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 		unlink = kzalloc(sizeof(struct vhci_unlink), GFP_ATOMIC);
 		if (!unlink) {
 			spin_unlock(&vdev->priv_lock);
-			spin_unlock(&the_controller->lock);
+			spin_unlock_irqrestore(&the_controller->lock, flags);
 			usbip_event_add(&vdev->ud, VDEV_EVENT_ERROR_MALLOC);
 			return -ENOMEM;
 		}
@@ -698,7 +707,7 @@ static int vhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 		spin_unlock(&vdev->priv_lock);
 	}
 
-	spin_unlock(&the_controller->lock);
+	spin_unlock_irqrestore(&the_controller->lock, flags);
 
 	usbip_dbg_vhci_hc("leave\n");
 	return 0;
@@ -707,8 +716,9 @@ static int vhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 static void vhci_device_unlink_cleanup(struct vhci_device *vdev)
 {
 	struct vhci_unlink *unlink, *tmp;
+	unsigned long flags;
 
-	spin_lock(&the_controller->lock);
+	spin_lock_irqsave(&the_controller->lock, flags);
 	spin_lock(&vdev->priv_lock);
 
 	list_for_each_entry_safe(unlink, tmp, &vdev->unlink_tx, list) {
@@ -742,19 +752,19 @@ static void vhci_device_unlink_cleanup(struct vhci_device *vdev)
 		list_del(&unlink->list);
 
 		spin_unlock(&vdev->priv_lock);
-		spin_unlock(&the_controller->lock);
+		spin_unlock_irqrestore(&the_controller->lock, flags);
 
 		usb_hcd_giveback_urb(vhci_to_hcd(the_controller), urb,
 				     urb->status);
 
-		spin_lock(&the_controller->lock);
+		spin_lock_irqsave(&the_controller->lock, flags);
 		spin_lock(&vdev->priv_lock);
 
 		kfree(unlink);
 	}
 
 	spin_unlock(&vdev->priv_lock);
-	spin_unlock(&the_controller->lock);
+	spin_unlock_irqrestore(&the_controller->lock, flags);
 }
 
 /*
@@ -821,8 +831,9 @@ static void vhci_shutdown_connection(struct usbip_device *ud)
 static void vhci_device_reset(struct usbip_device *ud)
 {
 	struct vhci_device *vdev = container_of(ud, struct vhci_device, ud);
+	unsigned long flags;
 
-	spin_lock(&ud->lock);
+	spin_lock_irqsave(&ud->lock, flags);
 
 	vdev->speed  = 0;
 	vdev->devid  = 0;
@@ -836,14 +847,16 @@ static void vhci_device_reset(struct usbip_device *ud)
 	}
 	ud->status = VDEV_ST_NULL;
 
-	spin_unlock(&ud->lock);
+	spin_unlock_irqrestore(&ud->lock, flags);
 }
 
 static void vhci_device_unusable(struct usbip_device *ud)
 {
-	spin_lock(&ud->lock);
+	unsigned long flags;
+
+	spin_lock_irqsave(&ud->lock, flags);
 	ud->status = VDEV_ST_ERROR;
-	spin_unlock(&ud->lock);
+	spin_unlock_irqrestore(&ud->lock, flags);
 }
 
 static void vhci_device_init(struct vhci_device *vdev)
@@ -933,12 +946,13 @@ static int vhci_get_frame_number(struct usb_hcd *hcd)
 static int vhci_bus_suspend(struct usb_hcd *hcd)
 {
 	struct vhci_hcd *vhci = hcd_to_vhci(hcd);
+	unsigned long flags;
 
 	dev_dbg(&hcd->self.root_hub->dev, "%s\n", __func__);
 
-	spin_lock(&vhci->lock);
+	spin_lock_irqsave(&vhci->lock, flags);
 	hcd->state = HC_STATE_SUSPENDED;
-	spin_unlock(&vhci->lock);
+	spin_unlock_irqrestore(&vhci->lock, flags);
 
 	return 0;
 }
@@ -947,15 +961,16 @@ static int vhci_bus_resume(struct usb_hcd *hcd)
 {
 	struct vhci_hcd *vhci = hcd_to_vhci(hcd);
 	int rc = 0;
+	unsigned long flags;
 
 	dev_dbg(&hcd->self.root_hub->dev, "%s\n", __func__);
 
-	spin_lock(&vhci->lock);
+	spin_lock_irqsave(&vhci->lock, flags);
 	if (!HCD_HW_ACCESSIBLE(hcd))
 		rc = -ESHUTDOWN;
 	else
 		hcd->state = HC_STATE_RUNNING;
-	spin_unlock(&vhci->lock);
+	spin_unlock_irqrestore(&vhci->lock, flags);
 
 	return rc;
 }
@@ -1053,17 +1068,18 @@ static int vhci_hcd_suspend(struct platform_device *pdev, pm_message_t state)
 	int rhport = 0;
 	int connected = 0;
 	int ret = 0;
+	unsigned long flags;
 
 	hcd = platform_get_drvdata(pdev);
 
-	spin_lock(&the_controller->lock);
+	spin_lock_irqsave(&the_controller->lock, flags);
 
 	for (rhport = 0; rhport < VHCI_NPORTS; rhport++)
 		if (the_controller->port_status[rhport] &
 		    USB_PORT_STAT_CONNECTION)
 			connected += 1;
 
-	spin_unlock(&the_controller->lock);
+	spin_unlock_irqrestore(&the_controller->lock, flags);
 
 	if (connected > 0) {
 		dev_info(&pdev->dev,
diff --git a/drivers/usb/usbip/vhci_rx.c b/drivers/usb/usbip/vhci_rx.c
index bc4eb0855314..323aa7789989 100644
--- a/drivers/usb/usbip/vhci_rx.c
+++ b/drivers/usb/usbip/vhci_rx.c
@@ -71,10 +71,11 @@ static void vhci_recv_ret_submit(struct vhci_device *vdev,
 {
 	struct usbip_device *ud = &vdev->ud;
 	struct urb *urb;
+	unsigned long flags;
 
-	spin_lock(&vdev->priv_lock);
+	spin_lock_irqsave(&vdev->priv_lock, flags);
 	urb = pickup_urb_and_free_priv(vdev, pdu->base.seqnum);
-	spin_unlock(&vdev->priv_lock);
+	spin_unlock_irqrestore(&vdev->priv_lock, flags);
 
 	if (!urb) {
 		pr_err("cannot find a urb of seqnum %u max seqnum %d\n",
@@ -103,9 +104,9 @@ static void vhci_recv_ret_submit(struct vhci_device *vdev,
 
 	usbip_dbg_vhci_rx("now giveback urb %u\n", pdu->base.seqnum);
 
-	spin_lock(&the_controller->lock);
+	spin_lock_irqsave(&the_controller->lock, flags);
 	usb_hcd_unlink_urb_from_ep(vhci_to_hcd(the_controller), urb);
-	spin_unlock(&the_controller->lock);
+	spin_unlock_irqrestore(&the_controller->lock, flags);
 
 	usb_hcd_giveback_urb(vhci_to_hcd(the_controller), urb, urb->status);
 
@@ -116,8 +117,9 @@ static struct vhci_unlink *dequeue_pending_unlink(struct vhci_device *vdev,
 						  struct usbip_header *pdu)
 {
 	struct vhci_unlink *unlink, *tmp;
+	unsigned long flags;
 
-	spin_lock(&vdev->priv_lock);
+	spin_lock_irqsave(&vdev->priv_lock, flags);
 
 	list_for_each_entry_safe(unlink, tmp, &vdev->unlink_rx, list) {
 		pr_info("unlink->seqnum %lu\n", unlink->seqnum);
@@ -126,12 +128,12 @@ static struct vhci_unlink *dequeue_pending_unlink(struct vhci_device *vdev,
 					  unlink->seqnum);
 			list_del(&unlink->list);
 
-			spin_unlock(&vdev->priv_lock);
+			spin_unlock_irqrestore(&vdev->priv_lock, flags);
 			return unlink;
 		}
 	}
 
-	spin_unlock(&vdev->priv_lock);
+	spin_unlock_irqrestore(&vdev->priv_lock, flags);
 
 	return NULL;
 }
@@ -141,6 +143,7 @@ static void vhci_recv_ret_unlink(struct vhci_device *vdev,
 {
 	struct vhci_unlink *unlink;
 	struct urb *urb;
+	unsigned long flags;
 
 	usbip_dump_header(pdu);
 
@@ -151,9 +154,9 @@ static void vhci_recv_ret_unlink(struct vhci_device *vdev,
 		return;
 	}
 
-	spin_lock(&vdev->priv_lock);
+	spin_lock_irqsave(&vdev->priv_lock, flags);
 	urb = pickup_urb_and_free_priv(vdev, unlink->unlink_seqnum);
-	spin_unlock(&vdev->priv_lock);
+	spin_unlock_irqrestore(&vdev->priv_lock, flags);
 
 	if (!urb) {
 		/*
@@ -170,9 +173,9 @@ static void vhci_recv_ret_unlink(struct vhci_device *vdev,
 		urb->status = pdu->u.ret_unlink.status;
 		pr_info("urb->status %d\n", urb->status);
 
-		spin_lock(&the_controller->lock);
+		spin_lock_irqsave(&the_controller->lock, flags);
 		usb_hcd_unlink_urb_from_ep(vhci_to_hcd(the_controller), urb);
-		spin_unlock(&the_controller->lock);
+		spin_unlock_irqrestore(&the_controller->lock, flags);
 
 		usb_hcd_giveback_urb(vhci_to_hcd(the_controller), urb,
 				     urb->status);
@@ -184,10 +187,11 @@ static void vhci_recv_ret_unlink(struct vhci_device *vdev,
 static int vhci_priv_tx_empty(struct vhci_device *vdev)
 {
 	int empty = 0;
+	unsigned long flags;
 
-	spin_lock(&vdev->priv_lock);
+	spin_lock_irqsave(&vdev->priv_lock, flags);
 	empty = list_empty(&vdev->priv_rx);
-	spin_unlock(&vdev->priv_lock);
+	spin_unlock_irqrestore(&vdev->priv_lock, flags);
 
 	return empty;
 }
diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
index 84c21c4ccf46..1c7f41a65565 100644
--- a/drivers/usb/usbip/vhci_sysfs.c
+++ b/drivers/usb/usbip/vhci_sysfs.c
@@ -32,10 +32,11 @@ static ssize_t status_show(struct device *dev, struct device_attribute *attr,
 {
 	char *s = out;
 	int i = 0;
+	unsigned long flags;
 
 	BUG_ON(!the_controller || !out);
 
-	spin_lock(&the_controller->lock);
+	spin_lock_irqsave(&the_controller->lock, flags);
 
 	/*
 	 * output example:
@@ -74,7 +75,7 @@ static ssize_t status_show(struct device *dev, struct device_attribute *attr,
 		spin_unlock(&vdev->ud.lock);
 	}
 
-	spin_unlock(&the_controller->lock);
+	spin_unlock_irqrestore(&the_controller->lock, flags);
 
 	return out - s;
 }
@@ -84,11 +85,12 @@ static DEVICE_ATTR_RO(status);
 static int vhci_port_disconnect(__u32 rhport)
 {
 	struct vhci_device *vdev;
+	unsigned long flags;
 
 	usbip_dbg_vhci_sysfs("enter\n");
 
 	/* lock */
-	spin_lock(&the_controller->lock);
+	spin_lock_irqsave(&the_controller->lock, flags);
 
 	vdev = port_to_vdev(rhport);
 
@@ -98,14 +100,14 @@ static int vhci_port_disconnect(__u32 rhport)
 
 		/* unlock */
 		spin_unlock(&vdev->ud.lock);
-		spin_unlock(&the_controller->lock);
+		spin_unlock_irqrestore(&the_controller->lock, flags);
 
 		return -EINVAL;
 	}
 
 	/* unlock */
 	spin_unlock(&vdev->ud.lock);
-	spin_unlock(&the_controller->lock);
+	spin_unlock_irqrestore(&the_controller->lock, flags);
 
 	usbip_event_add(&vdev->ud, VDEV_EVENT_DOWN);
 
@@ -181,6 +183,7 @@ static ssize_t store_attach(struct device *dev, struct device_attribute *attr,
 	int sockfd = 0;
 	__u32 rhport = 0, devid = 0, speed = 0;
 	int err;
+	unsigned long flags;
 
 	/*
 	 * @rhport: port number of vhci_hcd
@@ -206,14 +209,14 @@ static ssize_t store_attach(struct device *dev, struct device_attribute *attr,
 	/* now need lock until setting vdev status as used */
 
 	/* begin a lock */
-	spin_lock(&the_controller->lock);
+	spin_lock_irqsave(&the_controller->lock, flags);
 	vdev = port_to_vdev(rhport);
 	spin_lock(&vdev->ud.lock);
 
 	if (vdev->ud.status != VDEV_ST_NULL) {
 		/* end of the lock */
 		spin_unlock(&vdev->ud.lock);
-		spin_unlock(&the_controller->lock);
+		spin_unlock_irqrestore(&the_controller->lock, flags);
 
 		sockfd_put(socket);
 
@@ -232,7 +235,7 @@ static ssize_t store_attach(struct device *dev, struct device_attribute *attr,
 	vdev->ud.status     = VDEV_ST_NOTASSIGNED;
 
 	spin_unlock(&vdev->ud.lock);
-	spin_unlock(&the_controller->lock);
+	spin_unlock_irqrestore(&the_controller->lock, flags);
 	/* end the lock */
 
 	vdev->ud.tcp_rx = kthread_get_run(vhci_rx_loop, &vdev->ud, "vhci_rx");
diff --git a/drivers/usb/usbip/vhci_tx.c b/drivers/usb/usbip/vhci_tx.c
index 3c5796c8633a..a9a663a578b6 100644
--- a/drivers/usb/usbip/vhci_tx.c
+++ b/drivers/usb/usbip/vhci_tx.c
@@ -47,16 +47,17 @@ static void setup_cmd_submit_pdu(struct usbip_header *pdup,  struct urb *urb)
 static struct vhci_priv *dequeue_from_priv_tx(struct vhci_device *vdev)
 {
 	struct vhci_priv *priv, *tmp;
+	unsigned long flags;
 
-	spin_lock(&vdev->priv_lock);
+	spin_lock_irqsave(&vdev->priv_lock, flags);
 
 	list_for_each_entry_safe(priv, tmp, &vdev->priv_tx, list) {
 		list_move_tail(&priv->list, &vdev->priv_rx);
-		spin_unlock(&vdev->priv_lock);
+		spin_unlock_irqrestore(&vdev->priv_lock, flags);
 		return priv;
 	}
 
-	spin_unlock(&vdev->priv_lock);
+	spin_unlock_irqrestore(&vdev->priv_lock, flags);
 
 	return NULL;
 }
@@ -137,16 +138,17 @@ static int vhci_send_cmd_submit(struct vhci_device *vdev)
 static struct vhci_unlink *dequeue_from_unlink_tx(struct vhci_device *vdev)
 {
 	struct vhci_unlink *unlink, *tmp;
+	unsigned long flags;
 
-	spin_lock(&vdev->priv_lock);
+	spin_lock_irqsave(&vdev->priv_lock, flags);
 
 	list_for_each_entry_safe(unlink, tmp, &vdev->unlink_tx, list) {
 		list_move_tail(&unlink->list, &vdev->unlink_rx);
-		spin_unlock(&vdev->priv_lock);
+		spin_unlock_irqrestore(&vdev->priv_lock, flags);
 		return unlink;
 	}
 
-	spin_unlock(&vdev->priv_lock);
+	spin_unlock_irqrestore(&vdev->priv_lock, flags);
 
 	return NULL;
 }
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4.4 2/4] usbip: fix stub_rx: get_pipe() to validate endpoint number
  2018-01-25 18:37 [PATCH 4.4 0/4] Backport missing sccurity and deadlock fix Shuah Khan
  2018-01-25 18:37 ` [PATCH 4.4 1/4] usb: usbip: Fix possible deadlocks reported by lockdep Shuah Khan
@ 2018-01-25 18:37 ` Shuah Khan
  2018-01-25 18:37 ` [PATCH 4.4 3/4] usbip: fix stub_rx: harden CMD_SUBMIT path to handle malicious input Shuah Khan
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Shuah Khan @ 2018-01-25 18:37 UTC (permalink / raw)
  To: valentina.manea.m, shuah, gregkh
  Cc: Shuah Khan, linux-usb, linux-kernel, stable

Upstream commit 635f545a7e8b ("usbip: fix stub_rx: get_pipe() to validate
endpoint number")

get_pipe() routine doesn't validate the input endpoint number
and uses to reference ep_in and ep_out arrays. Invalid endpoint
number can trigger BUG(). Range check the epnum and returning
error instead of calling BUG().

Change caller stub_recv_cmd_submit() to handle the get_pipe()
error return.

Reported-by: Secunia Research <vuln@secunia.com>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/usb/usbip/stub_rx.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/usbip/stub_rx.c b/drivers/usb/usbip/stub_rx.c
index 7de54a66044f..e617c90661b4 100644
--- a/drivers/usb/usbip/stub_rx.c
+++ b/drivers/usb/usbip/stub_rx.c
@@ -344,15 +344,15 @@ static int get_pipe(struct stub_device *sdev, int epnum, int dir)
 	struct usb_host_endpoint *ep;
 	struct usb_endpoint_descriptor *epd = NULL;
 
+	if (epnum < 0 || epnum > 15)
+		goto err_ret;
+
 	if (dir == USBIP_DIR_IN)
 		ep = udev->ep_in[epnum & 0x7f];
 	else
 		ep = udev->ep_out[epnum & 0x7f];
-	if (!ep) {
-		dev_err(&sdev->interface->dev, "no such endpoint?, %d\n",
-			epnum);
-		BUG();
-	}
+	if (!ep)
+		goto err_ret;
 
 	epd = &ep->desc;
 	if (usb_endpoint_xfer_control(epd)) {
@@ -383,9 +383,10 @@ static int get_pipe(struct stub_device *sdev, int epnum, int dir)
 			return usb_rcvisocpipe(udev, epnum);
 	}
 
+err_ret:
 	/* NOT REACHED */
-	dev_err(&sdev->interface->dev, "get pipe, epnum %d\n", epnum);
-	return 0;
+	dev_err(&sdev->udev->dev, "get pipe() invalid epnum %d\n", epnum);
+	return -1;
 }
 
 static void masking_bogus_flags(struct urb *urb)
@@ -451,6 +452,9 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
 	struct usb_device *udev = sdev->udev;
 	int pipe = get_pipe(sdev, pdu->base.ep, pdu->base.direction);
 
+	if (pipe == -1)
+		return;
+
 	priv = stub_priv_alloc(sdev, pdu);
 	if (!priv)
 		return;
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4.4 3/4] usbip: fix stub_rx: harden CMD_SUBMIT path to handle malicious input
  2018-01-25 18:37 [PATCH 4.4 0/4] Backport missing sccurity and deadlock fix Shuah Khan
  2018-01-25 18:37 ` [PATCH 4.4 1/4] usb: usbip: Fix possible deadlocks reported by lockdep Shuah Khan
  2018-01-25 18:37 ` [PATCH 4.4 2/4] usbip: fix stub_rx: get_pipe() to validate endpoint number Shuah Khan
@ 2018-01-25 18:37 ` Shuah Khan
  2018-01-25 18:37 ` [PATCH 4.4 4/4] usbip: prevent leaking socket pointer address in messages Shuah Khan
  2018-01-26  9:33 ` [PATCH 4.4 0/4] Backport missing sccurity and deadlock fix Greg KH
  4 siblings, 0 replies; 6+ messages in thread
From: Shuah Khan @ 2018-01-25 18:37 UTC (permalink / raw)
  To: valentina.manea.m, shuah, gregkh
  Cc: Shuah Khan, linux-usb, linux-kernel, stable

Upstream commit c6688ef9f297 ("usbip: fix stub_rx: harden CMD_SUBMIT path
to handle malicious input")

Harden CMD_SUBMIT path to handle malicious input that could trigger
large memory allocations. Add checks to validate transfer_buffer_length
and number_of_packets to protect against bad input requesting for
unbounded memory allocations. Validate early in get_pipe() and return
failure.

Reported-by: Secunia Research <vuln@secunia.com>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/usb/usbip/stub_rx.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/usbip/stub_rx.c b/drivers/usb/usbip/stub_rx.c
index e617c90661b4..56cacb68040c 100644
--- a/drivers/usb/usbip/stub_rx.c
+++ b/drivers/usb/usbip/stub_rx.c
@@ -338,11 +338,13 @@ static struct stub_priv *stub_priv_alloc(struct stub_device *sdev,
 	return priv;
 }
 
-static int get_pipe(struct stub_device *sdev, int epnum, int dir)
+static int get_pipe(struct stub_device *sdev, struct usbip_header *pdu)
 {
 	struct usb_device *udev = sdev->udev;
 	struct usb_host_endpoint *ep;
 	struct usb_endpoint_descriptor *epd = NULL;
+	int epnum = pdu->base.ep;
+	int dir = pdu->base.direction;
 
 	if (epnum < 0 || epnum > 15)
 		goto err_ret;
@@ -355,6 +357,7 @@ static int get_pipe(struct stub_device *sdev, int epnum, int dir)
 		goto err_ret;
 
 	epd = &ep->desc;
+
 	if (usb_endpoint_xfer_control(epd)) {
 		if (dir == USBIP_DIR_OUT)
 			return usb_sndctrlpipe(udev, epnum);
@@ -377,6 +380,27 @@ static int get_pipe(struct stub_device *sdev, int epnum, int dir)
 	}
 
 	if (usb_endpoint_xfer_isoc(epd)) {
+		/* validate packet size and number of packets */
+		unsigned int maxp, packets, bytes;
+
+#define USB_EP_MAXP_MULT_SHIFT  11
+#define USB_EP_MAXP_MULT_MASK   (3 << USB_EP_MAXP_MULT_SHIFT)
+#define USB_EP_MAXP_MULT(m) \
+	(((m) & USB_EP_MAXP_MULT_MASK) >> USB_EP_MAXP_MULT_SHIFT)
+
+		maxp = usb_endpoint_maxp(epd);
+		maxp *= (USB_EP_MAXP_MULT(
+				__le16_to_cpu(epd->wMaxPacketSize)) + 1);
+		bytes = pdu->u.cmd_submit.transfer_buffer_length;
+		packets = DIV_ROUND_UP(bytes, maxp);
+
+		if (pdu->u.cmd_submit.number_of_packets < 0 ||
+		    pdu->u.cmd_submit.number_of_packets > packets) {
+			dev_err(&sdev->udev->dev,
+				"CMD_SUBMIT: isoc invalid num packets %d\n",
+				pdu->u.cmd_submit.number_of_packets);
+			return -1;
+		}
 		if (dir == USBIP_DIR_OUT)
 			return usb_sndisocpipe(udev, epnum);
 		else
@@ -385,7 +409,7 @@ static int get_pipe(struct stub_device *sdev, int epnum, int dir)
 
 err_ret:
 	/* NOT REACHED */
-	dev_err(&sdev->udev->dev, "get pipe() invalid epnum %d\n", epnum);
+	dev_err(&sdev->udev->dev, "CMD_SUBMIT: invalid epnum %d\n", epnum);
 	return -1;
 }
 
@@ -450,7 +474,7 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
 	struct stub_priv *priv;
 	struct usbip_device *ud = &sdev->ud;
 	struct usb_device *udev = sdev->udev;
-	int pipe = get_pipe(sdev, pdu->base.ep, pdu->base.direction);
+	int pipe = get_pipe(sdev, pdu);
 
 	if (pipe == -1)
 		return;
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4.4 4/4] usbip: prevent leaking socket pointer address in messages
  2018-01-25 18:37 [PATCH 4.4 0/4] Backport missing sccurity and deadlock fix Shuah Khan
                   ` (2 preceding siblings ...)
  2018-01-25 18:37 ` [PATCH 4.4 3/4] usbip: fix stub_rx: harden CMD_SUBMIT path to handle malicious input Shuah Khan
@ 2018-01-25 18:37 ` Shuah Khan
  2018-01-26  9:33 ` [PATCH 4.4 0/4] Backport missing sccurity and deadlock fix Greg KH
  4 siblings, 0 replies; 6+ messages in thread
From: Shuah Khan @ 2018-01-25 18:37 UTC (permalink / raw)
  To: valentina.manea.m, shuah, gregkh
  Cc: Shuah Khan, linux-usb, linux-kernel, stable

Upstream commit 90120d15f4c3 ("usbip: prevent leaking socket pointer
address in messages")

usbip driver is leaking socket pointer address in messages. Remove
the messages that aren't useful and print sockfd in the ones that
are useful for debugging.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/usb/usbip/stub_dev.c     |  3 +--
 drivers/usb/usbip/usbip_common.c | 15 ++++-----------
 drivers/usb/usbip/vhci_hcd.c     |  2 +-
 3 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/usbip/stub_dev.c b/drivers/usb/usbip/stub_dev.c
index a3ec49bdc1e6..ec38370ffcab 100644
--- a/drivers/usb/usbip/stub_dev.c
+++ b/drivers/usb/usbip/stub_dev.c
@@ -163,8 +163,7 @@ static void stub_shutdown_connection(struct usbip_device *ud)
 	 * step 1?
 	 */
 	if (ud->tcp_socket) {
-		dev_dbg(&sdev->udev->dev, "shutdown tcp_socket %p\n",
-			ud->tcp_socket);
+		dev_dbg(&sdev->udev->dev, "shutdown sockfd %d\n", ud->sockfd);
 		kernel_sock_shutdown(ud->tcp_socket, SHUT_RDWR);
 	}
 
diff --git a/drivers/usb/usbip/usbip_common.c b/drivers/usb/usbip/usbip_common.c
index 9752b93f754e..1838f1b2c2fa 100644
--- a/drivers/usb/usbip/usbip_common.c
+++ b/drivers/usb/usbip/usbip_common.c
@@ -317,18 +317,14 @@ int usbip_recv(struct socket *sock, void *buf, int size)
 	struct msghdr msg;
 	struct kvec iov;
 	int total = 0;
-
 	/* for blocks of if (usbip_dbg_flag_xmit) */
 	char *bp = buf;
 	int osize = size;
 
-	usbip_dbg_xmit("enter\n");
-
-	if (!sock || !buf || !size) {
-		pr_err("invalid arg, sock %p buff %p size %d\n", sock, buf,
-		       size);
+	if (!sock || !buf || !size)
 		return -EINVAL;
-	}
+
+	usbip_dbg_xmit("enter\n");
 
 	do {
 		sock->sk->sk_allocation = GFP_NOIO;
@@ -341,11 +337,8 @@ int usbip_recv(struct socket *sock, void *buf, int size)
 		msg.msg_flags      = MSG_NOSIGNAL;
 
 		result = kernel_recvmsg(sock, &msg, &iov, 1, size, MSG_WAITALL);
-		if (result <= 0) {
-			pr_debug("receive sock %p buf %p size %u ret %d total %d\n",
-				 sock, buf, size, result, total);
+		if (result <= 0)
 			goto err;
-		}
 
 		size -= result;
 		buf += result;
diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index 0aaa8e524afd..00d68945548e 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -778,7 +778,7 @@ static void vhci_shutdown_connection(struct usbip_device *ud)
 
 	/* need this? see stub_dev.c */
 	if (ud->tcp_socket) {
-		pr_debug("shutdown tcp_socket %p\n", ud->tcp_socket);
+		pr_debug("shutdown sockfd %d\n", ud->sockfd);
 		kernel_sock_shutdown(ud->tcp_socket, SHUT_RDWR);
 	}
 
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 4.4 0/4] Backport missing sccurity and deadlock fix
  2018-01-25 18:37 [PATCH 4.4 0/4] Backport missing sccurity and deadlock fix Shuah Khan
                   ` (3 preceding siblings ...)
  2018-01-25 18:37 ` [PATCH 4.4 4/4] usbip: prevent leaking socket pointer address in messages Shuah Khan
@ 2018-01-26  9:33 ` Greg KH
  4 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2018-01-26  9:33 UTC (permalink / raw)
  To: Shuah Khan; +Cc: valentina.manea.m, shuah, linux-usb, linux-kernel, stable

On Thu, Jan 25, 2018 at 11:37:40AM -0700, Shuah Khan wrote:
> As I started backporting security fixes, I found a deadlock bug that was
> fixed in a later release. This patch series contains backports for all
> these problems.

All now queued up, thanks for the backports.

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-01-26  9:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-25 18:37 [PATCH 4.4 0/4] Backport missing sccurity and deadlock fix Shuah Khan
2018-01-25 18:37 ` [PATCH 4.4 1/4] usb: usbip: Fix possible deadlocks reported by lockdep Shuah Khan
2018-01-25 18:37 ` [PATCH 4.4 2/4] usbip: fix stub_rx: get_pipe() to validate endpoint number Shuah Khan
2018-01-25 18:37 ` [PATCH 4.4 3/4] usbip: fix stub_rx: harden CMD_SUBMIT path to handle malicious input Shuah Khan
2018-01-25 18:37 ` [PATCH 4.4 4/4] usbip: prevent leaking socket pointer address in messages Shuah Khan
2018-01-26  9:33 ` [PATCH 4.4 0/4] Backport missing sccurity and deadlock fix Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).