Skip to content

Commit 2161979

Browse files
Andrew Goodbodygregkh
authored andcommitted
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>
1 parent 46e3caf commit 2161979

File tree

5 files changed

+91
-65
lines changed

5 files changed

+91
-65
lines changed

drivers/usb/usbip/usbip_event.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,11 +117,12 @@ EXPORT_SYMBOL_GPL(usbip_event_add);
117117
int usbip_event_happened(struct usbip_device *ud)
118118
{
119119
int happened = 0;
120+
unsigned long flags;
120121

121-
spin_lock(&ud->lock);
122+
spin_lock_irqsave(&ud->lock, flags);
122123
if (ud->event != 0)
123124
happened = 1;
124-
spin_unlock(&ud->lock);
125+
spin_unlock_irqrestore(&ud->lock, flags);
125126

126127
return happened;
127128
}

drivers/usb/usbip/vhci_hcd.c

Lines changed: 52 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,11 @@ static void dump_port_status_diff(u32 prev_status, u32 new_status)
121121

122122
void rh_port_connect(int rhport, enum usb_device_speed speed)
123123
{
124+
unsigned long flags;
125+
124126
usbip_dbg_vhci_rh("rh_port_connect %d\n", rhport);
125127

126-
spin_lock(&the_controller->lock);
128+
spin_lock_irqsave(&the_controller->lock, flags);
127129

128130
the_controller->port_status[rhport] |= USB_PORT_STAT_CONNECTION
129131
| (1 << USB_PORT_FEAT_C_CONNECTION);
@@ -139,22 +141,24 @@ void rh_port_connect(int rhport, enum usb_device_speed speed)
139141
break;
140142
}
141143

142-
spin_unlock(&the_controller->lock);
144+
spin_unlock_irqrestore(&the_controller->lock, flags);
143145

144146
usb_hcd_poll_rh_status(vhci_to_hcd(the_controller));
145147
}
146148

147149
static void rh_port_disconnect(int rhport)
148150
{
151+
unsigned long flags;
152+
149153
usbip_dbg_vhci_rh("rh_port_disconnect %d\n", rhport);
150154

151-
spin_lock(&the_controller->lock);
155+
spin_lock_irqsave(&the_controller->lock, flags);
152156

153157
the_controller->port_status[rhport] &= ~USB_PORT_STAT_CONNECTION;
154158
the_controller->port_status[rhport] |=
155159
(1 << USB_PORT_FEAT_C_CONNECTION);
156160

157-
spin_unlock(&the_controller->lock);
161+
spin_unlock_irqrestore(&the_controller->lock, flags);
158162
usb_hcd_poll_rh_status(vhci_to_hcd(the_controller));
159163
}
160164

@@ -182,13 +186,14 @@ static int vhci_hub_status(struct usb_hcd *hcd, char *buf)
182186
int retval;
183187
int rhport;
184188
int changed = 0;
189+
unsigned long flags;
185190

186191
retval = DIV_ROUND_UP(VHCI_NPORTS + 1, 8);
187192
memset(buf, 0, retval);
188193

189194
vhci = hcd_to_vhci(hcd);
190195

191-
spin_lock(&vhci->lock);
196+
spin_lock_irqsave(&vhci->lock, flags);
192197
if (!HCD_HW_ACCESSIBLE(hcd)) {
193198
usbip_dbg_vhci_rh("hw accessible flag not on?\n");
194199
goto done;
@@ -209,7 +214,7 @@ static int vhci_hub_status(struct usb_hcd *hcd, char *buf)
209214
usb_hcd_resume_root_hub(hcd);
210215

211216
done:
212-
spin_unlock(&vhci->lock);
217+
spin_unlock_irqrestore(&vhci->lock, flags);
213218
return changed ? retval : 0;
214219
}
215220

@@ -231,6 +236,7 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
231236
struct vhci_hcd *dum;
232237
int retval = 0;
233238
int rhport;
239+
unsigned long flags;
234240

235241
u32 prev_port_status[VHCI_NPORTS];
236242

@@ -249,7 +255,7 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
249255

250256
dum = hcd_to_vhci(hcd);
251257

252-
spin_lock(&dum->lock);
258+
spin_lock_irqsave(&dum->lock, flags);
253259

254260
/* store old status and compare now and old later */
255261
if (usbip_dbg_flag_vhci_rh) {
@@ -403,7 +409,7 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
403409
}
404410
usbip_dbg_vhci_rh(" bye\n");
405411

406-
spin_unlock(&dum->lock);
412+
spin_unlock_irqrestore(&dum->lock, flags);
407413

408414
return retval;
409415
}
@@ -426,6 +432,7 @@ static void vhci_tx_urb(struct urb *urb)
426432
{
427433
struct vhci_device *vdev = get_vdev(urb->dev);
428434
struct vhci_priv *priv;
435+
unsigned long flags;
429436

430437
if (!vdev) {
431438
pr_err("could not get virtual device");
@@ -438,7 +445,7 @@ static void vhci_tx_urb(struct urb *urb)
438445
return;
439446
}
440447

441-
spin_lock(&vdev->priv_lock);
448+
spin_lock_irqsave(&vdev->priv_lock, flags);
442449

443450
priv->seqnum = atomic_inc_return(&the_controller->seqnum);
444451
if (priv->seqnum == 0xffff)
@@ -452,7 +459,7 @@ static void vhci_tx_urb(struct urb *urb)
452459
list_add_tail(&priv->list, &vdev->priv_tx);
453460

454461
wake_up(&vdev->waitq_tx);
455-
spin_unlock(&vdev->priv_lock);
462+
spin_unlock_irqrestore(&vdev->priv_lock, flags);
456463
}
457464

458465
static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
@@ -461,18 +468,19 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
461468
struct device *dev = &urb->dev->dev;
462469
int ret = 0;
463470
struct vhci_device *vdev;
471+
unsigned long flags;
464472

465473
usbip_dbg_vhci_hc("enter, usb_hcd %p urb %p mem_flags %d\n",
466474
hcd, urb, mem_flags);
467475

468476
/* patch to usb_sg_init() is in 2.5.60 */
469477
BUG_ON(!urb->transfer_buffer && urb->transfer_buffer_length);
470478

471-
spin_lock(&the_controller->lock);
479+
spin_lock_irqsave(&the_controller->lock, flags);
472480

473481
if (urb->status != -EINPROGRESS) {
474482
dev_err(dev, "URB already unlinked!, status %d\n", urb->status);
475-
spin_unlock(&the_controller->lock);
483+
spin_unlock_irqrestore(&the_controller->lock, flags);
476484
return urb->status;
477485
}
478486

@@ -484,7 +492,7 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
484492
vdev->ud.status == VDEV_ST_ERROR) {
485493
dev_err(dev, "enqueue for inactive port %d\n", vdev->rhport);
486494
spin_unlock(&vdev->ud.lock);
487-
spin_unlock(&the_controller->lock);
495+
spin_unlock_irqrestore(&the_controller->lock, flags);
488496
return -ENODEV;
489497
}
490498
spin_unlock(&vdev->ud.lock);
@@ -557,14 +565,14 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
557565

558566
out:
559567
vhci_tx_urb(urb);
560-
spin_unlock(&the_controller->lock);
568+
spin_unlock_irqrestore(&the_controller->lock, flags);
561569

562570
return 0;
563571

564572
no_need_xmit:
565573
usb_hcd_unlink_urb_from_ep(hcd, urb);
566574
no_need_unlink:
567-
spin_unlock(&the_controller->lock);
575+
spin_unlock_irqrestore(&the_controller->lock, flags);
568576
if (!ret)
569577
usb_hcd_giveback_urb(vhci_to_hcd(the_controller),
570578
urb, urb->status);
@@ -621,16 +629,17 @@ static int vhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
621629
{
622630
struct vhci_priv *priv;
623631
struct vhci_device *vdev;
632+
unsigned long flags;
624633

625634
pr_info("dequeue a urb %p\n", urb);
626635

627-
spin_lock(&the_controller->lock);
636+
spin_lock_irqsave(&the_controller->lock, flags);
628637

629638
priv = urb->hcpriv;
630639
if (!priv) {
631640
/* URB was never linked! or will be soon given back by
632641
* vhci_rx. */
633-
spin_unlock(&the_controller->lock);
642+
spin_unlock_irqrestore(&the_controller->lock, flags);
634643
return -EIDRM;
635644
}
636645

@@ -639,7 +648,7 @@ static int vhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
639648

640649
ret = usb_hcd_check_unlink_urb(hcd, urb, status);
641650
if (ret) {
642-
spin_unlock(&the_controller->lock);
651+
spin_unlock_irqrestore(&the_controller->lock, flags);
643652
return ret;
644653
}
645654
}
@@ -667,10 +676,10 @@ static int vhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
667676

668677
usb_hcd_unlink_urb_from_ep(hcd, urb);
669678

670-
spin_unlock(&the_controller->lock);
679+
spin_unlock_irqrestore(&the_controller->lock, flags);
671680
usb_hcd_giveback_urb(vhci_to_hcd(the_controller), urb,
672681
urb->status);
673-
spin_lock(&the_controller->lock);
682+
spin_lock_irqsave(&the_controller->lock, flags);
674683

675684
} else {
676685
/* tcp connection is alive */
@@ -682,7 +691,7 @@ static int vhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
682691
unlink = kzalloc(sizeof(struct vhci_unlink), GFP_ATOMIC);
683692
if (!unlink) {
684693
spin_unlock(&vdev->priv_lock);
685-
spin_unlock(&the_controller->lock);
694+
spin_unlock_irqrestore(&the_controller->lock, flags);
686695
usbip_event_add(&vdev->ud, VDEV_EVENT_ERROR_MALLOC);
687696
return -ENOMEM;
688697
}
@@ -703,7 +712,7 @@ static int vhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
703712
spin_unlock(&vdev->priv_lock);
704713
}
705714

706-
spin_unlock(&the_controller->lock);
715+
spin_unlock_irqrestore(&the_controller->lock, flags);
707716

708717
usbip_dbg_vhci_hc("leave\n");
709718
return 0;
@@ -712,8 +721,9 @@ static int vhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
712721
static void vhci_device_unlink_cleanup(struct vhci_device *vdev)
713722
{
714723
struct vhci_unlink *unlink, *tmp;
724+
unsigned long flags;
715725

716-
spin_lock(&the_controller->lock);
726+
spin_lock_irqsave(&the_controller->lock, flags);
717727
spin_lock(&vdev->priv_lock);
718728

719729
list_for_each_entry_safe(unlink, tmp, &vdev->unlink_tx, list) {
@@ -747,19 +757,19 @@ static void vhci_device_unlink_cleanup(struct vhci_device *vdev)
747757
list_del(&unlink->list);
748758

749759
spin_unlock(&vdev->priv_lock);
750-
spin_unlock(&the_controller->lock);
760+
spin_unlock_irqrestore(&the_controller->lock, flags);
751761

752762
usb_hcd_giveback_urb(vhci_to_hcd(the_controller), urb,
753763
urb->status);
754764

755-
spin_lock(&the_controller->lock);
765+
spin_lock_irqsave(&the_controller->lock, flags);
756766
spin_lock(&vdev->priv_lock);
757767

758768
kfree(unlink);
759769
}
760770

761771
spin_unlock(&vdev->priv_lock);
762-
spin_unlock(&the_controller->lock);
772+
spin_unlock_irqrestore(&the_controller->lock, flags);
763773
}
764774

765775
/*
@@ -826,8 +836,9 @@ static void vhci_shutdown_connection(struct usbip_device *ud)
826836
static void vhci_device_reset(struct usbip_device *ud)
827837
{
828838
struct vhci_device *vdev = container_of(ud, struct vhci_device, ud);
839+
unsigned long flags;
829840

830-
spin_lock(&ud->lock);
841+
spin_lock_irqsave(&ud->lock, flags);
831842

832843
vdev->speed = 0;
833844
vdev->devid = 0;
@@ -841,14 +852,16 @@ static void vhci_device_reset(struct usbip_device *ud)
841852
}
842853
ud->status = VDEV_ST_NULL;
843854

844-
spin_unlock(&ud->lock);
855+
spin_unlock_irqrestore(&ud->lock, flags);
845856
}
846857

847858
static void vhci_device_unusable(struct usbip_device *ud)
848859
{
849-
spin_lock(&ud->lock);
860+
unsigned long flags;
861+
862+
spin_lock_irqsave(&ud->lock, flags);
850863
ud->status = VDEV_ST_ERROR;
851-
spin_unlock(&ud->lock);
864+
spin_unlock_irqrestore(&ud->lock, flags);
852865
}
853866

854867
static void vhci_device_init(struct vhci_device *vdev)
@@ -938,12 +951,13 @@ static int vhci_get_frame_number(struct usb_hcd *hcd)
938951
static int vhci_bus_suspend(struct usb_hcd *hcd)
939952
{
940953
struct vhci_hcd *vhci = hcd_to_vhci(hcd);
954+
unsigned long flags;
941955

942956
dev_dbg(&hcd->self.root_hub->dev, "%s\n", __func__);
943957

944-
spin_lock(&vhci->lock);
958+
spin_lock_irqsave(&vhci->lock, flags);
945959
hcd->state = HC_STATE_SUSPENDED;
946-
spin_unlock(&vhci->lock);
960+
spin_unlock_irqrestore(&vhci->lock, flags);
947961

948962
return 0;
949963
}
@@ -952,15 +966,16 @@ static int vhci_bus_resume(struct usb_hcd *hcd)
952966
{
953967
struct vhci_hcd *vhci = hcd_to_vhci(hcd);
954968
int rc = 0;
969+
unsigned long flags;
955970

956971
dev_dbg(&hcd->self.root_hub->dev, "%s\n", __func__);
957972

958-
spin_lock(&vhci->lock);
973+
spin_lock_irqsave(&vhci->lock, flags);
959974
if (!HCD_HW_ACCESSIBLE(hcd))
960975
rc = -ESHUTDOWN;
961976
else
962977
hcd->state = HC_STATE_RUNNING;
963-
spin_unlock(&vhci->lock);
978+
spin_unlock_irqrestore(&vhci->lock, flags);
964979

965980
return rc;
966981
}
@@ -1058,17 +1073,18 @@ static int vhci_hcd_suspend(struct platform_device *pdev, pm_message_t state)
10581073
int rhport = 0;
10591074
int connected = 0;
10601075
int ret = 0;
1076+
unsigned long flags;
10611077

10621078
hcd = platform_get_drvdata(pdev);
10631079

1064-
spin_lock(&the_controller->lock);
1080+
spin_lock_irqsave(&the_controller->lock, flags);
10651081

10661082
for (rhport = 0; rhport < VHCI_NPORTS; rhport++)
10671083
if (the_controller->port_status[rhport] &
10681084
USB_PORT_STAT_CONNECTION)
10691085
connected += 1;
10701086

1071-
spin_unlock(&the_controller->lock);
1087+
spin_unlock_irqrestore(&the_controller->lock, flags);
10721088

10731089
if (connected > 0) {
10741090
dev_info(&pdev->dev,

0 commit comments

Comments
 (0)