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(-)
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; }
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;
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;
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); }
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
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Shuah Khan