scsi: fcoe: Fix potential deadlock on &fip->ctlr_lock
[ Upstream commit 1a1975551943f681772720f639ff42fbaa746212 ]
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: 794d98e77f
("[SCSI] libfcoe: retry rejected FLOGI to another FCF if possible")
Signed-off-by: Chengfeng Ye <dg573847474@gmail.com>
Link: https://lore.kernel.org/r/20230817074708.7509-1-dg573847474@gmail.com
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
parent
b1e3199bad
commit
d2bf25674c
@ -318,16 +318,17 @@ static void fcoe_ctlr_announce(struct fcoe_ctlr *fip)
|
|||||||
{
|
{
|
||||||
struct fcoe_fcf *sel;
|
struct fcoe_fcf *sel;
|
||||||
struct fcoe_fcf *fcf;
|
struct fcoe_fcf *fcf;
|
||||||
|
unsigned long flags;
|
||||||
|
|
||||||
mutex_lock(&fip->ctlr_mutex);
|
mutex_lock(&fip->ctlr_mutex);
|
||||||
spin_lock_bh(&fip->ctlr_lock);
|
spin_lock_irqsave(&fip->ctlr_lock, flags);
|
||||||
|
|
||||||
kfree_skb(fip->flogi_req);
|
kfree_skb(fip->flogi_req);
|
||||||
fip->flogi_req = NULL;
|
fip->flogi_req = NULL;
|
||||||
list_for_each_entry(fcf, &fip->fcfs, list)
|
list_for_each_entry(fcf, &fip->fcfs, list)
|
||||||
fcf->flogi_sent = 0;
|
fcf->flogi_sent = 0;
|
||||||
|
|
||||||
spin_unlock_bh(&fip->ctlr_lock);
|
spin_unlock_irqrestore(&fip->ctlr_lock, flags);
|
||||||
sel = fip->sel_fcf;
|
sel = fip->sel_fcf;
|
||||||
|
|
||||||
if (sel && ether_addr_equal(sel->fcf_mac, fip->dest_addr))
|
if (sel && ether_addr_equal(sel->fcf_mac, fip->dest_addr))
|
||||||
@ -697,6 +698,7 @@ int fcoe_ctlr_els_send(struct fcoe_ctlr *fip, struct fc_lport *lport,
|
|||||||
{
|
{
|
||||||
struct fc_frame *fp;
|
struct fc_frame *fp;
|
||||||
struct fc_frame_header *fh;
|
struct fc_frame_header *fh;
|
||||||
|
unsigned long flags;
|
||||||
u16 old_xid;
|
u16 old_xid;
|
||||||
u8 op;
|
u8 op;
|
||||||
u8 mac[ETH_ALEN];
|
u8 mac[ETH_ALEN];
|
||||||
@ -730,11 +732,11 @@ int fcoe_ctlr_els_send(struct fcoe_ctlr *fip, struct fc_lport *lport,
|
|||||||
op = FIP_DT_FLOGI;
|
op = FIP_DT_FLOGI;
|
||||||
if (fip->mode == FIP_MODE_VN2VN)
|
if (fip->mode == FIP_MODE_VN2VN)
|
||||||
break;
|
break;
|
||||||
spin_lock_bh(&fip->ctlr_lock);
|
spin_lock_irqsave(&fip->ctlr_lock, flags);
|
||||||
kfree_skb(fip->flogi_req);
|
kfree_skb(fip->flogi_req);
|
||||||
fip->flogi_req = skb;
|
fip->flogi_req = skb;
|
||||||
fip->flogi_req_send = 1;
|
fip->flogi_req_send = 1;
|
||||||
spin_unlock_bh(&fip->ctlr_lock);
|
spin_unlock_irqrestore(&fip->ctlr_lock, flags);
|
||||||
schedule_work(&fip->timer_work);
|
schedule_work(&fip->timer_work);
|
||||||
return -EINPROGRESS;
|
return -EINPROGRESS;
|
||||||
case ELS_FDISC:
|
case ELS_FDISC:
|
||||||
@ -1711,10 +1713,11 @@ static int fcoe_ctlr_flogi_send_locked(struct fcoe_ctlr *fip)
|
|||||||
static int fcoe_ctlr_flogi_retry(struct fcoe_ctlr *fip)
|
static int fcoe_ctlr_flogi_retry(struct fcoe_ctlr *fip)
|
||||||
{
|
{
|
||||||
struct fcoe_fcf *fcf;
|
struct fcoe_fcf *fcf;
|
||||||
|
unsigned long flags;
|
||||||
int error;
|
int error;
|
||||||
|
|
||||||
mutex_lock(&fip->ctlr_mutex);
|
mutex_lock(&fip->ctlr_mutex);
|
||||||
spin_lock_bh(&fip->ctlr_lock);
|
spin_lock_irqsave(&fip->ctlr_lock, flags);
|
||||||
LIBFCOE_FIP_DBG(fip, "re-sending FLOGI - reselect\n");
|
LIBFCOE_FIP_DBG(fip, "re-sending FLOGI - reselect\n");
|
||||||
fcf = fcoe_ctlr_select(fip);
|
fcf = fcoe_ctlr_select(fip);
|
||||||
if (!fcf || fcf->flogi_sent) {
|
if (!fcf || fcf->flogi_sent) {
|
||||||
@ -1725,7 +1728,7 @@ static int fcoe_ctlr_flogi_retry(struct fcoe_ctlr *fip)
|
|||||||
fcoe_ctlr_solicit(fip, NULL);
|
fcoe_ctlr_solicit(fip, NULL);
|
||||||
error = fcoe_ctlr_flogi_send_locked(fip);
|
error = fcoe_ctlr_flogi_send_locked(fip);
|
||||||
}
|
}
|
||||||
spin_unlock_bh(&fip->ctlr_lock);
|
spin_unlock_irqrestore(&fip->ctlr_lock, flags);
|
||||||
mutex_unlock(&fip->ctlr_mutex);
|
mutex_unlock(&fip->ctlr_mutex);
|
||||||
return error;
|
return error;
|
||||||
}
|
}
|
||||||
@ -1742,8 +1745,9 @@ static int fcoe_ctlr_flogi_retry(struct fcoe_ctlr *fip)
|
|||||||
static void fcoe_ctlr_flogi_send(struct fcoe_ctlr *fip)
|
static void fcoe_ctlr_flogi_send(struct fcoe_ctlr *fip)
|
||||||
{
|
{
|
||||||
struct fcoe_fcf *fcf;
|
struct fcoe_fcf *fcf;
|
||||||
|
unsigned long flags;
|
||||||
|
|
||||||
spin_lock_bh(&fip->ctlr_lock);
|
spin_lock_irqsave(&fip->ctlr_lock, flags);
|
||||||
fcf = fip->sel_fcf;
|
fcf = fip->sel_fcf;
|
||||||
if (!fcf || !fip->flogi_req_send)
|
if (!fcf || !fip->flogi_req_send)
|
||||||
goto unlock;
|
goto unlock;
|
||||||
@ -1770,7 +1774,7 @@ static void fcoe_ctlr_flogi_send(struct fcoe_ctlr *fip)
|
|||||||
} else /* XXX */
|
} else /* XXX */
|
||||||
LIBFCOE_FIP_DBG(fip, "No FCF selected - defer send\n");
|
LIBFCOE_FIP_DBG(fip, "No FCF selected - defer send\n");
|
||||||
unlock:
|
unlock:
|
||||||
spin_unlock_bh(&fip->ctlr_lock);
|
spin_unlock_irqrestore(&fip->ctlr_lock, flags);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
Loading…
Reference in New Issue
Block a user