From e7eda9329372f5e436e5a9291eb115eab0feae02 Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Fri, 28 Mar 2014 11:10:30 +0100 Subject: uas: fix GFP_NOIO under spinlock Quote Dan: The patch e36e64930cff: "uas: Use GFP_NOIO rather then GFP_ATOMIC where possible" from Nov 7, 2013, leads to the following static checker warning: drivers/usb/storage/uas.c:806 uas_eh_task_mgmt() error: scheduling with locks held: 'spin_lock:lock' Some other allocations under spinlock are not caught. The fix essentially reverts e36e64930cffd94e1c37fdb82f35989384aa946b Signed-off-by: Oliver Neukum Reported-by: Dan Carpenter Reviewed-by: Hans de Goede Acked-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index a7ac97cc5949..8f4222640bd6 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -137,7 +137,7 @@ static void uas_do_work(struct work_struct *work) if (!(cmdinfo->state & IS_IN_WORK_LIST)) continue; - err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_NOIO); + err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_ATOMIC); if (!err) cmdinfo->state &= ~IS_IN_WORK_LIST; else @@ -803,7 +803,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, devinfo->running_task = 1; memset(&devinfo->response, 0, sizeof(devinfo->response)); - sense_urb = uas_submit_sense_urb(cmnd, GFP_NOIO, + sense_urb = uas_submit_sense_urb(cmnd, GFP_ATOMIC, devinfo->use_streams ? tag : 0); if (!sense_urb) { shost_printk(KERN_INFO, shost, @@ -813,7 +813,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, spin_unlock_irqrestore(&devinfo->lock, flags); return FAILED; } - if (uas_submit_task_urb(cmnd, GFP_NOIO, function, tag)) { + if (uas_submit_task_urb(cmnd, GFP_ATOMIC, function, tag)) { shost_printk(KERN_INFO, shost, "%s: %s: submit task mgmt urb failed\n", __func__, fname); -- cgit v1.2.3 From c637f1fa7b0452b71eebd35d00906d371c04714e Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Fri, 28 Mar 2014 11:29:25 +0100 Subject: uas: fix error handling during scsi_scan() intfdata is set only after scsi_scan(). uas_pre_reset() however needs intfdata to be valid and will follow the NULL pointer killing khubd. intfdata must be preemptively set before the host is registered and undone in the error case. Signed-off-by: Oliver Neukum Reviewed-by: Hans de Goede Acked-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 8f4222640bd6..fcab9b79d9fb 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -1096,16 +1096,17 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) if (result) goto free_streams; + usb_set_intfdata(intf, shost); result = scsi_add_host(shost, &intf->dev); if (result) goto free_streams; scsi_scan_host(shost); - usb_set_intfdata(intf, shost); return result; free_streams: uas_free_streams(devinfo); + usb_set_intfdata(intf, NULL); set_alt0: usb_set_interface(udev, intf->altsetting[0].desc.bInterfaceNumber, 0); if (shost) -- cgit v1.2.3 From 94d72f008909610710bb1841d665eeeb010a0be1 Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Fri, 28 Mar 2014 11:25:50 +0100 Subject: uas: fix deadlocky memory allocations There are also two allocations with GFP_KERNEL in the pre-/post_reset code paths. That is no good because that is a part of the SCSI error handler. Signed-off-by: Oliver Neukum Reviewed-by: Hans de Goede Acked-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index fcab9b79d9fb..511b22953167 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -1030,7 +1030,7 @@ static int uas_configure_endpoints(struct uas_dev_info *devinfo) devinfo->use_streams = 0; } else { devinfo->qdepth = usb_alloc_streams(devinfo->intf, eps + 1, - 3, 256, GFP_KERNEL); + 3, 256, GFP_NOIO); if (devinfo->qdepth < 0) return devinfo->qdepth; devinfo->use_streams = 1; @@ -1047,7 +1047,7 @@ static void uas_free_streams(struct uas_dev_info *devinfo) eps[0] = usb_pipe_endpoint(udev, devinfo->status_pipe); eps[1] = usb_pipe_endpoint(udev, devinfo->data_in_pipe); eps[2] = usb_pipe_endpoint(udev, devinfo->data_out_pipe); - usb_free_streams(devinfo->intf, eps, 3, GFP_KERNEL); + usb_free_streams(devinfo->intf, eps, 3, GFP_NOIO); } static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) -- cgit v1.2.3