Skip to content

Commit 1a19755

Browse files
Ychamemartinkpetersen
authored andcommitted
scsi: fcoe: Fix potential deadlock on &fip->ctlr_lock
There is a long call chain that &fip->ctlr_lock is acquired by isr fnic_isr_msix_wq_copy() under hard IRQ context. Thus other process context code acquiring the lock should disable IRQ, otherwise deadlock could happen if the IRQ preempts the execution while the lock is held in process context on the same CPU. [ISR] fnic_isr_msix_wq_copy() -> fnic_wq_copy_cmpl_handler() -> fnic_fcpio_cmpl_handler() -> fnic_fcpio_flogi_reg_cmpl_handler() -> fnic_flush_tx() -> fnic_send_frame() -> fcoe_ctlr_els_send() -> spin_lock_bh(&fip->ctlr_lock) [Process Context] 1. fcoe_ctlr_timer_work() -> fcoe_ctlr_flogi_send() -> spin_lock_bh(&fip->ctlr_lock) 2. fcoe_ctlr_recv_work() -> fcoe_ctlr_recv_handler() -> fcoe_ctlr_recv_els() -> fcoe_ctlr_announce() -> spin_lock_bh(&fip->ctlr_lock) 3. fcoe_ctlr_recv_work() -> fcoe_ctlr_recv_handler() -> fcoe_ctlr_recv_els() -> fcoe_ctlr_flogi_retry() -> spin_lock_bh(&fip->ctlr_lock) 4. -> fcoe_xmit() -> fcoe_ctlr_els_send() -> spin_lock_bh(&fip->ctlr_lock) spin_lock_bh() is not enough since fnic_isr_msix_wq_copy() is a hardirq. These flaws were found by an experimental static analysis tool I am developing for irq-related deadlock. The patch fix the potential deadlocks by spin_lock_irqsave() to disable hard irq. Fixes: 794d98e ("[SCSI] libfcoe: retry rejected FLOGI to another FCF if possible") Signed-off-by: Chengfeng Ye <[email protected]> Link: https://lore.kernel.org/r/[email protected] Reviewed-by: Davidlohr Bueso <[email protected]> Signed-off-by: Martin K. Petersen <[email protected]>
1 parent 2d6f70f commit 1a19755

File tree

1 file changed

+12
-8
lines changed

1 file changed

+12
-8
lines changed

drivers/scsi/fcoe/fcoe_ctlr.c

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -319,16 +319,17 @@ static void fcoe_ctlr_announce(struct fcoe_ctlr *fip)
319319
{
320320
struct fcoe_fcf *sel;
321321
struct fcoe_fcf *fcf;
322+
unsigned long flags;
322323

323324
mutex_lock(&fip->ctlr_mutex);
324-
spin_lock_bh(&fip->ctlr_lock);
325+
spin_lock_irqsave(&fip->ctlr_lock, flags);
325326

326327
kfree_skb(fip->flogi_req);
327328
fip->flogi_req = NULL;
328329
list_for_each_entry(fcf, &fip->fcfs, list)
329330
fcf->flogi_sent = 0;
330331

331-
spin_unlock_bh(&fip->ctlr_lock);
332+
spin_unlock_irqrestore(&fip->ctlr_lock, flags);
332333
sel = fip->sel_fcf;
333334

334335
if (sel && ether_addr_equal(sel->fcf_mac, fip->dest_addr))
@@ -699,6 +700,7 @@ int fcoe_ctlr_els_send(struct fcoe_ctlr *fip, struct fc_lport *lport,
699700
{
700701
struct fc_frame *fp;
701702
struct fc_frame_header *fh;
703+
unsigned long flags;
702704
u16 old_xid;
703705
u8 op;
704706
u8 mac[ETH_ALEN];
@@ -732,11 +734,11 @@ int fcoe_ctlr_els_send(struct fcoe_ctlr *fip, struct fc_lport *lport,
732734
op = FIP_DT_FLOGI;
733735
if (fip->mode == FIP_MODE_VN2VN)
734736
break;
735-
spin_lock_bh(&fip->ctlr_lock);
737+
spin_lock_irqsave(&fip->ctlr_lock, flags);
736738
kfree_skb(fip->flogi_req);
737739
fip->flogi_req = skb;
738740
fip->flogi_req_send = 1;
739-
spin_unlock_bh(&fip->ctlr_lock);
741+
spin_unlock_irqrestore(&fip->ctlr_lock, flags);
740742
schedule_work(&fip->timer_work);
741743
return -EINPROGRESS;
742744
case ELS_FDISC:
@@ -1705,10 +1707,11 @@ static int fcoe_ctlr_flogi_send_locked(struct fcoe_ctlr *fip)
17051707
static int fcoe_ctlr_flogi_retry(struct fcoe_ctlr *fip)
17061708
{
17071709
struct fcoe_fcf *fcf;
1710+
unsigned long flags;
17081711
int error;
17091712

17101713
mutex_lock(&fip->ctlr_mutex);
1711-
spin_lock_bh(&fip->ctlr_lock);
1714+
spin_lock_irqsave(&fip->ctlr_lock, flags);
17121715
LIBFCOE_FIP_DBG(fip, "re-sending FLOGI - reselect\n");
17131716
fcf = fcoe_ctlr_select(fip);
17141717
if (!fcf || fcf->flogi_sent) {
@@ -1719,7 +1722,7 @@ static int fcoe_ctlr_flogi_retry(struct fcoe_ctlr *fip)
17191722
fcoe_ctlr_solicit(fip, NULL);
17201723
error = fcoe_ctlr_flogi_send_locked(fip);
17211724
}
1722-
spin_unlock_bh(&fip->ctlr_lock);
1725+
spin_unlock_irqrestore(&fip->ctlr_lock, flags);
17231726
mutex_unlock(&fip->ctlr_mutex);
17241727
return error;
17251728
}
@@ -1736,8 +1739,9 @@ static int fcoe_ctlr_flogi_retry(struct fcoe_ctlr *fip)
17361739
static void fcoe_ctlr_flogi_send(struct fcoe_ctlr *fip)
17371740
{
17381741
struct fcoe_fcf *fcf;
1742+
unsigned long flags;
17391743

1740-
spin_lock_bh(&fip->ctlr_lock);
1744+
spin_lock_irqsave(&fip->ctlr_lock, flags);
17411745
fcf = fip->sel_fcf;
17421746
if (!fcf || !fip->flogi_req_send)
17431747
goto unlock;
@@ -1764,7 +1768,7 @@ static void fcoe_ctlr_flogi_send(struct fcoe_ctlr *fip)
17641768
} else /* XXX */
17651769
LIBFCOE_FIP_DBG(fip, "No FCF selected - defer send\n");
17661770
unlock:
1767-
spin_unlock_bh(&fip->ctlr_lock);
1771+
spin_unlock_irqrestore(&fip->ctlr_lock, flags);
17681772
}
17691773

17701774
/**

0 commit comments

Comments
 (0)