Commit 0c9a5f3e authored by Quinn Tran's avatar Quinn Tran Committed by Martin K. Petersen

scsi: qla2xxx: Fix unsafe removal from linked list

On NPIV delete, the VPort is taken off a linked list in an unsafe manner.
The check for VPort refcount should be done behind lock before taking off
the element.

[ 2733.016907] general protection fault: 0000 [#1] SMP NOPTI
[ 2733.016908] qla2xxx [0000:22:00.1]-7088:27: VP[4] deleted.
[ 2733.016912] CPU: 22 PID: 23481 Comm: qla2xxx_15_dpc Kdump: loaded Tainted:
G           OE KX    5.3.18-47-default #1 SLE15-SP3
[ 2733.016914] Hardware name: Dell Inc. PowerEdge R7525/0PYVT1, BIOS 2.1.4 02/17/2021
[ 2733.016929] RIP: 0010:qla2x00_abort_isp+0x90/0x850 [qla2xxx]
[ 2733.016933] RSP: 0018:ffffb9cfc91efe98 EFLAGS: 00010087
[ 2733.016935] RAX: 0000000000000292 RBX: dead000000000100 RCX: 0000000000000000
[ 2733.016936] RDX: 0000000000000001 RSI: ffff944bfeb99558 RDI: ffff944bfc4b4488
[ 2733.016937] RBP: ffff944bfc4b2868 R08: 00000000000187a2 R09: 0000000000000001
[ 2733.016937] R10: ffffb9cfc91efcc8 R11: 0000000000000001 R12: ffff944bfc4b4000
[ 2733.016938] R13: ffff944bfc4b4870 R14: ffff944bfc4b4488 R15: ffff944bda895c80
[ 2733.016939] FS:  0000000000000000(0000) GS:ffff944bfeb80000(0000) knlGS:0000000000000000
[ 2733.016940] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 2733.016940] CR2: 00007fc173e74458 CR3: 0000001ff57de000 CR4: 0000000000350ee0
[ 2733.016941] Call Trace:
[ 2733.016951]   qla2xxx_pci_error_detected+0x190/0x190 [qla2xxx]
[ 2733.016957]   qla2x00_do_dpc+0x560/0xa10 [qla2xxx]
[ 2733.016962]   kthread+0x10d/0x130
[ 2733.016963]   kthread_park+0xa0/0xa0
[ 2733.016966]   ret_from_fork+0x22/0x40

Link: https://lore.kernel.org/r/20210810043720.1137-9-njavali@marvell.comReviewed-by: default avatarHimanshu Madhani <himanshu.madhani@oracle.com>
Signed-off-by: default avatarQuinn Tran <qutran@marvell.com>
Signed-off-by: default avatarNilesh Javali <njavali@marvell.com>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent 01c97f2d
...@@ -6573,13 +6573,13 @@ void ...@@ -6573,13 +6573,13 @@ void
qla2x00_update_fcports(scsi_qla_host_t *base_vha) qla2x00_update_fcports(scsi_qla_host_t *base_vha)
{ {
fc_port_t *fcport; fc_port_t *fcport;
struct scsi_qla_host *vha; struct scsi_qla_host *vha, *tvp;
struct qla_hw_data *ha = base_vha->hw; struct qla_hw_data *ha = base_vha->hw;
unsigned long flags; unsigned long flags;
spin_lock_irqsave(&ha->vport_slock, flags); spin_lock_irqsave(&ha->vport_slock, flags);
/* Go with deferred removal of rport references. */ /* Go with deferred removal of rport references. */
list_for_each_entry(vha, &base_vha->hw->vp_list, list) { list_for_each_entry_safe(vha, tvp, &base_vha->hw->vp_list, list) {
atomic_inc(&vha->vref_count); atomic_inc(&vha->vref_count);
list_for_each_entry(fcport, &vha->vp_fcports, list) { list_for_each_entry(fcport, &vha->vp_fcports, list) {
if (fcport->drport && if (fcport->drport &&
...@@ -6924,7 +6924,8 @@ void ...@@ -6924,7 +6924,8 @@ void
qla2x00_quiesce_io(scsi_qla_host_t *vha) qla2x00_quiesce_io(scsi_qla_host_t *vha)
{ {
struct qla_hw_data *ha = vha->hw; struct qla_hw_data *ha = vha->hw;
struct scsi_qla_host *vp; struct scsi_qla_host *vp, *tvp;
unsigned long flags;
ql_dbg(ql_dbg_dpc, vha, 0x401d, ql_dbg(ql_dbg_dpc, vha, 0x401d,
"Quiescing I/O - ha=%p.\n", ha); "Quiescing I/O - ha=%p.\n", ha);
...@@ -6933,8 +6934,18 @@ qla2x00_quiesce_io(scsi_qla_host_t *vha) ...@@ -6933,8 +6934,18 @@ qla2x00_quiesce_io(scsi_qla_host_t *vha)
if (atomic_read(&vha->loop_state) != LOOP_DOWN) { if (atomic_read(&vha->loop_state) != LOOP_DOWN) {
atomic_set(&vha->loop_state, LOOP_DOWN); atomic_set(&vha->loop_state, LOOP_DOWN);
qla2x00_mark_all_devices_lost(vha); qla2x00_mark_all_devices_lost(vha);
list_for_each_entry(vp, &ha->vp_list, list)
spin_lock_irqsave(&ha->vport_slock, flags);
list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) {
atomic_inc(&vp->vref_count);
spin_unlock_irqrestore(&ha->vport_slock, flags);
qla2x00_mark_all_devices_lost(vp); qla2x00_mark_all_devices_lost(vp);
spin_lock_irqsave(&ha->vport_slock, flags);
atomic_dec(&vp->vref_count);
}
spin_unlock_irqrestore(&ha->vport_slock, flags);
} else { } else {
if (!atomic_read(&vha->loop_down_timer)) if (!atomic_read(&vha->loop_down_timer))
atomic_set(&vha->loop_down_timer, atomic_set(&vha->loop_down_timer,
...@@ -6949,7 +6960,7 @@ void ...@@ -6949,7 +6960,7 @@ void
qla2x00_abort_isp_cleanup(scsi_qla_host_t *vha) qla2x00_abort_isp_cleanup(scsi_qla_host_t *vha)
{ {
struct qla_hw_data *ha = vha->hw; struct qla_hw_data *ha = vha->hw;
struct scsi_qla_host *vp; struct scsi_qla_host *vp, *tvp;
unsigned long flags; unsigned long flags;
fc_port_t *fcport; fc_port_t *fcport;
u16 i; u16 i;
...@@ -7017,7 +7028,7 @@ qla2x00_abort_isp_cleanup(scsi_qla_host_t *vha) ...@@ -7017,7 +7028,7 @@ qla2x00_abort_isp_cleanup(scsi_qla_host_t *vha)
qla2x00_mark_all_devices_lost(vha); qla2x00_mark_all_devices_lost(vha);
spin_lock_irqsave(&ha->vport_slock, flags); spin_lock_irqsave(&ha->vport_slock, flags);
list_for_each_entry(vp, &ha->vp_list, list) { list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) {
atomic_inc(&vp->vref_count); atomic_inc(&vp->vref_count);
spin_unlock_irqrestore(&ha->vport_slock, flags); spin_unlock_irqrestore(&ha->vport_slock, flags);
...@@ -7039,7 +7050,7 @@ qla2x00_abort_isp_cleanup(scsi_qla_host_t *vha) ...@@ -7039,7 +7050,7 @@ qla2x00_abort_isp_cleanup(scsi_qla_host_t *vha)
fcport->scan_state = 0; fcport->scan_state = 0;
} }
spin_lock_irqsave(&ha->vport_slock, flags); spin_lock_irqsave(&ha->vport_slock, flags);
list_for_each_entry(vp, &ha->vp_list, list) { list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) {
atomic_inc(&vp->vref_count); atomic_inc(&vp->vref_count);
spin_unlock_irqrestore(&ha->vport_slock, flags); spin_unlock_irqrestore(&ha->vport_slock, flags);
...@@ -7083,7 +7094,7 @@ qla2x00_abort_isp(scsi_qla_host_t *vha) ...@@ -7083,7 +7094,7 @@ qla2x00_abort_isp(scsi_qla_host_t *vha)
int rval; int rval;
uint8_t status = 0; uint8_t status = 0;
struct qla_hw_data *ha = vha->hw; struct qla_hw_data *ha = vha->hw;
struct scsi_qla_host *vp; struct scsi_qla_host *vp, *tvp;
struct req_que *req = ha->req_q_map[0]; struct req_que *req = ha->req_q_map[0];
unsigned long flags; unsigned long flags;
...@@ -7239,7 +7250,7 @@ qla2x00_abort_isp(scsi_qla_host_t *vha) ...@@ -7239,7 +7250,7 @@ qla2x00_abort_isp(scsi_qla_host_t *vha)
ql_dbg(ql_dbg_taskm, vha, 0x8022, "%s succeeded.\n", __func__); ql_dbg(ql_dbg_taskm, vha, 0x8022, "%s succeeded.\n", __func__);
qla2x00_configure_hba(vha); qla2x00_configure_hba(vha);
spin_lock_irqsave(&ha->vport_slock, flags); spin_lock_irqsave(&ha->vport_slock, flags);
list_for_each_entry(vp, &ha->vp_list, list) { list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) {
if (vp->vp_idx) { if (vp->vp_idx) {
atomic_inc(&vp->vref_count); atomic_inc(&vp->vref_count);
spin_unlock_irqrestore(&ha->vport_slock, flags); spin_unlock_irqrestore(&ha->vport_slock, flags);
...@@ -8924,7 +8935,7 @@ qla82xx_restart_isp(scsi_qla_host_t *vha) ...@@ -8924,7 +8935,7 @@ qla82xx_restart_isp(scsi_qla_host_t *vha)
{ {
int status, rval; int status, rval;
struct qla_hw_data *ha = vha->hw; struct qla_hw_data *ha = vha->hw;
struct scsi_qla_host *vp; struct scsi_qla_host *vp, *tvp;
unsigned long flags; unsigned long flags;
status = qla2x00_init_rings(vha); status = qla2x00_init_rings(vha);
...@@ -8996,7 +9007,7 @@ qla82xx_restart_isp(scsi_qla_host_t *vha) ...@@ -8996,7 +9007,7 @@ qla82xx_restart_isp(scsi_qla_host_t *vha)
"qla82xx_restart_isp succeeded.\n"); "qla82xx_restart_isp succeeded.\n");
spin_lock_irqsave(&ha->vport_slock, flags); spin_lock_irqsave(&ha->vport_slock, flags);
list_for_each_entry(vp, &ha->vp_list, list) { list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) {
if (vp->vp_idx) { if (vp->vp_idx) {
atomic_inc(&vp->vref_count); atomic_inc(&vp->vref_count);
spin_unlock_irqrestore(&ha->vport_slock, flags); spin_unlock_irqrestore(&ha->vport_slock, flags);
......
...@@ -65,7 +65,7 @@ qla24xx_deallocate_vp_id(scsi_qla_host_t *vha) ...@@ -65,7 +65,7 @@ qla24xx_deallocate_vp_id(scsi_qla_host_t *vha)
uint16_t vp_id; uint16_t vp_id;
struct qla_hw_data *ha = vha->hw; struct qla_hw_data *ha = vha->hw;
unsigned long flags = 0; unsigned long flags = 0;
u8 i; u32 i, bailout;
mutex_lock(&ha->vport_lock); mutex_lock(&ha->vport_lock);
/* /*
...@@ -75,21 +75,29 @@ qla24xx_deallocate_vp_id(scsi_qla_host_t *vha) ...@@ -75,21 +75,29 @@ qla24xx_deallocate_vp_id(scsi_qla_host_t *vha)
* ensures no active vp_list traversal while the vport is removed * ensures no active vp_list traversal while the vport is removed
* from the queue) * from the queue)
*/ */
for (i = 0; i < 10; i++) { bailout = 0;
if (wait_event_timeout(vha->vref_waitq, for (i = 0; i < 500; i++) {
!atomic_read(&vha->vref_count), HZ) > 0) spin_lock_irqsave(&ha->vport_slock, flags);
if (atomic_read(&vha->vref_count) == 0) {
list_del(&vha->list);
qlt_update_vp_map(vha, RESET_VP_IDX);
bailout = 1;
}
spin_unlock_irqrestore(&ha->vport_slock, flags);
if (bailout)
break; break;
else
msleep(20);
} }
if (!bailout) {
spin_lock_irqsave(&ha->vport_slock, flags); ql_log(ql_log_info, vha, 0xfffa,
if (atomic_read(&vha->vref_count)) { "vha->vref_count=%u timeout\n", vha->vref_count.counter);
ql_dbg(ql_dbg_vport, vha, 0xfffa, spin_lock_irqsave(&ha->vport_slock, flags);
"vha->vref_count=%u timeout\n", vha->vref_count.counter); list_del(&vha->list);
vha->vref_count = (atomic_t)ATOMIC_INIT(0); qlt_update_vp_map(vha, RESET_VP_IDX);
spin_unlock_irqrestore(&ha->vport_slock, flags);
} }
list_del(&vha->list);
qlt_update_vp_map(vha, RESET_VP_IDX);
spin_unlock_irqrestore(&ha->vport_slock, flags);
vp_id = vha->vp_idx; vp_id = vha->vp_idx;
ha->num_vhosts--; ha->num_vhosts--;
...@@ -262,13 +270,13 @@ qla24xx_configure_vp(scsi_qla_host_t *vha) ...@@ -262,13 +270,13 @@ qla24xx_configure_vp(scsi_qla_host_t *vha)
void void
qla2x00_alert_all_vps(struct rsp_que *rsp, uint16_t *mb) qla2x00_alert_all_vps(struct rsp_que *rsp, uint16_t *mb)
{ {
scsi_qla_host_t *vha; scsi_qla_host_t *vha, *tvp;
struct qla_hw_data *ha = rsp->hw; struct qla_hw_data *ha = rsp->hw;
int i = 0; int i = 0;
unsigned long flags; unsigned long flags;
spin_lock_irqsave(&ha->vport_slock, flags); spin_lock_irqsave(&ha->vport_slock, flags);
list_for_each_entry(vha, &ha->vp_list, list) { list_for_each_entry_safe(vha, tvp, &ha->vp_list, list) {
if (vha->vp_idx) { if (vha->vp_idx) {
if (test_bit(VPORT_DELETE, &vha->dpc_flags)) if (test_bit(VPORT_DELETE, &vha->dpc_flags))
continue; continue;
...@@ -421,7 +429,7 @@ void ...@@ -421,7 +429,7 @@ void
qla2x00_do_dpc_all_vps(scsi_qla_host_t *vha) qla2x00_do_dpc_all_vps(scsi_qla_host_t *vha)
{ {
struct qla_hw_data *ha = vha->hw; struct qla_hw_data *ha = vha->hw;
scsi_qla_host_t *vp; scsi_qla_host_t *vp, *tvp;
unsigned long flags = 0; unsigned long flags = 0;
if (vha->vp_idx) if (vha->vp_idx)
...@@ -435,7 +443,7 @@ qla2x00_do_dpc_all_vps(scsi_qla_host_t *vha) ...@@ -435,7 +443,7 @@ qla2x00_do_dpc_all_vps(scsi_qla_host_t *vha)
return; return;
spin_lock_irqsave(&ha->vport_slock, flags); spin_lock_irqsave(&ha->vport_slock, flags);
list_for_each_entry(vp, &ha->vp_list, list) { list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) {
if (vp->vp_idx) { if (vp->vp_idx) {
atomic_inc(&vp->vref_count); atomic_inc(&vp->vref_count);
spin_unlock_irqrestore(&ha->vport_slock, flags); spin_unlock_irqrestore(&ha->vport_slock, flags);
......
...@@ -7533,7 +7533,7 @@ static void qla_pci_error_cleanup(scsi_qla_host_t *vha) ...@@ -7533,7 +7533,7 @@ static void qla_pci_error_cleanup(scsi_qla_host_t *vha)
struct qla_hw_data *ha = vha->hw; struct qla_hw_data *ha = vha->hw;
scsi_qla_host_t *base_vha = pci_get_drvdata(ha->pdev); scsi_qla_host_t *base_vha = pci_get_drvdata(ha->pdev);
struct qla_qpair *qpair = NULL; struct qla_qpair *qpair = NULL;
struct scsi_qla_host *vp; struct scsi_qla_host *vp, *tvp;
fc_port_t *fcport; fc_port_t *fcport;
int i; int i;
unsigned long flags; unsigned long flags;
...@@ -7564,7 +7564,7 @@ static void qla_pci_error_cleanup(scsi_qla_host_t *vha) ...@@ -7564,7 +7564,7 @@ static void qla_pci_error_cleanup(scsi_qla_host_t *vha)
qla2x00_mark_all_devices_lost(vha); qla2x00_mark_all_devices_lost(vha);
spin_lock_irqsave(&ha->vport_slock, flags); spin_lock_irqsave(&ha->vport_slock, flags);
list_for_each_entry(vp, &ha->vp_list, list) { list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) {
atomic_inc(&vp->vref_count); atomic_inc(&vp->vref_count);
spin_unlock_irqrestore(&ha->vport_slock, flags); spin_unlock_irqrestore(&ha->vport_slock, flags);
qla2x00_mark_all_devices_lost(vp); qla2x00_mark_all_devices_lost(vp);
...@@ -7578,7 +7578,7 @@ static void qla_pci_error_cleanup(scsi_qla_host_t *vha) ...@@ -7578,7 +7578,7 @@ static void qla_pci_error_cleanup(scsi_qla_host_t *vha)
fcport->flags &= ~(FCF_LOGIN_NEEDED | FCF_ASYNC_SENT); fcport->flags &= ~(FCF_LOGIN_NEEDED | FCF_ASYNC_SENT);
spin_lock_irqsave(&ha->vport_slock, flags); spin_lock_irqsave(&ha->vport_slock, flags);
list_for_each_entry(vp, &ha->vp_list, list) { list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) {
atomic_inc(&vp->vref_count); atomic_inc(&vp->vref_count);
spin_unlock_irqrestore(&ha->vport_slock, flags); spin_unlock_irqrestore(&ha->vport_slock, flags);
list_for_each_entry(fcport, &vp->vp_fcports, list) list_for_each_entry(fcport, &vp->vp_fcports, list)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment