diff mbox series

[v2,2/3] qla2xxx: on session delete return nvme cmd

Message ID 20190618181021.16547-3-hmadhani@marvell.com (mailing list archive)
State Superseded
Headers show
Series qla2xxx: Fix crashes with FC-NVMe devices | expand

Commit Message

Himanshu Madhani June 18, 2019, 6:10 p.m. UTC 
From: Quinn Tran <qutran@marvell.com>

- on session delete or chip reset, reject all NVME commands.
- on NVME command submission error, free srb resource.

Signed-off-by: Quinn Tran <qutran@marvell.com>
Signed-off-by: Himanshu Madhani <hmadhani@marvell.com>
---
 drivers/scsi/qla2xxx/qla_nvme.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

Comments

Dan Carpenter June 26, 2019, 6:20 a.m. UTC | #1 
Hi Himanshu,

url:    https://github.com/0day-ci/linux/commits/Himanshu-Madhani/qla2xxx-Fix-crashes-with-FC-NVMe-devices/20190619-074559
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
drivers/scsi/qla2xxx/qla_nvme.c:245 qla_nvme_ls_req() warn: variable dereferenced before check 'fcport' (see line 242)
drivers/scsi/qla2xxx/qla_nvme.c:496 qla_nvme_post_cmd() warn: variable dereferenced before check 'fcport' (see line 494)
drivers/scsi/qla2xxx/qla_nvme.c:510 qla_nvme_post_cmd() error: we previously assumed 'qpair' could be null (see line 496)

Old smatch warnings:
drivers/scsi/qla2xxx/qla_nvme.c:190 qla_nvme_abort_work() warn: variable dereferenced before check 'fcport' (see line 183)

# https://github.com/0day-ci/linux/commit/69efeca5ca5e394664f54ef8e349a31b0f424507
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 69efeca5ca5e394664f54ef8e349a31b0f424507
vim +/fcport +245 drivers/scsi/qla2xxx/qla_nvme.c

e84067d7 Duane Grigsby               2017-06-21  230  static int qla_nvme_ls_req(struct nvme_fc_local_port *lport,
e84067d7 Duane Grigsby               2017-06-21  231      struct nvme_fc_remote_port *rport, struct nvmefc_ls_req *fd)
e84067d7 Duane Grigsby               2017-06-21  232  {
9dd9686b Darren Trapp                2018-03-20  233  	struct qla_nvme_rport *qla_rport = rport->private;
9dd9686b Darren Trapp                2018-03-20  234  	fc_port_t *fcport = qla_rport->fcport;
e84067d7 Duane Grigsby               2017-06-21  235  	struct srb_iocb   *nvme;
e84067d7 Duane Grigsby               2017-06-21  236  	struct nvme_private *priv = fd->private;
e84067d7 Duane Grigsby               2017-06-21  237  	struct scsi_qla_host *vha;
e84067d7 Duane Grigsby               2017-06-21  238  	int     rval = QLA_FUNCTION_FAILED;
e84067d7 Duane Grigsby               2017-06-21  239  	struct qla_hw_data *ha;
e84067d7 Duane Grigsby               2017-06-21  240  	srb_t           *sp;
e84067d7 Duane Grigsby               2017-06-21  241  
e84067d7 Duane Grigsby               2017-06-21 @242  	vha = fcport->vha;
                                                              ^^^^^^^^^^^
Dereference.

e84067d7 Duane Grigsby               2017-06-21  243  	ha = vha->hw;
69efeca5 Quinn Tran                  2019-06-18  244  
69efeca5 Quinn Tran                  2019-06-18 @245  	if (!ha->flags.fw_started || (fcport && fcport->deleted))
                                                                                      ^^^^^^
Check for NULL is too late.

69efeca5 Quinn Tran                  2019-06-18  246  		return rval;
69efeca5 Quinn Tran                  2019-06-18  247  
e84067d7 Duane Grigsby               2017-06-21  248  	/* Alloc SRB structure */
e84067d7 Duane Grigsby               2017-06-21  249  	sp = qla2x00_get_sp(vha, fcport, GFP_ATOMIC);
e84067d7 Duane Grigsby               2017-06-21  250  	if (!sp)
e84067d7 Duane Grigsby               2017-06-21  251  		return rval;
e84067d7 Duane Grigsby               2017-06-21  252  
e84067d7 Duane Grigsby               2017-06-21  253  	sp->type = SRB_NVME_LS;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Arun Easi June 26, 2019, 7:17 p.m. UTC | #2 
Thanks Dan for reporting the issue.

This is already fixed in v3 of the patch:
	https://marc.info/?l=linux-scsi&m=156113583805280&w=2

Regards,
-Arun

On Tue, 25 Jun 2019, 11:20pm, Dan Carpenter wrote:

> Hi Himanshu,
> 
> url:    https://github.com/0day-ci/linux/commits/Himanshu-Madhani/qla2xxx-Fix-crashes-with-FC-NVMe-devices/20190619-074559
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
> 
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> New smatch warnings:
> drivers/scsi/qla2xxx/qla_nvme.c:245 qla_nvme_ls_req() warn: variable dereferenced before check 'fcport' (see line 242)
> drivers/scsi/qla2xxx/qla_nvme.c:496 qla_nvme_post_cmd() warn: variable dereferenced before check 'fcport' (see line 494)
> drivers/scsi/qla2xxx/qla_nvme.c:510 qla_nvme_post_cmd() error: we previously assumed 'qpair' could be null (see line 496)
> 
> Old smatch warnings:
> drivers/scsi/qla2xxx/qla_nvme.c:190 qla_nvme_abort_work() warn: variable dereferenced before check 'fcport' (see line 183)
> 
> # https://github.com/0day-ci/linux/commit/69efeca5ca5e394664f54ef8e349a31b0f424507
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout 69efeca5ca5e394664f54ef8e349a31b0f424507
> vim +/fcport +245 drivers/scsi/qla2xxx/qla_nvme.c
> 
> e84067d7 Duane Grigsby               2017-06-21  230  static int qla_nvme_ls_req(struct nvme_fc_local_port *lport,
> e84067d7 Duane Grigsby               2017-06-21  231      struct nvme_fc_remote_port *rport, struct nvmefc_ls_req *fd)
> e84067d7 Duane Grigsby               2017-06-21  232  {
> 9dd9686b Darren Trapp                2018-03-20  233  	struct qla_nvme_rport *qla_rport = rport->private;
> 9dd9686b Darren Trapp                2018-03-20  234  	fc_port_t *fcport = qla_rport->fcport;
> e84067d7 Duane Grigsby               2017-06-21  235  	struct srb_iocb   *nvme;
> e84067d7 Duane Grigsby               2017-06-21  236  	struct nvme_private *priv = fd->private;
> e84067d7 Duane Grigsby               2017-06-21  237  	struct scsi_qla_host *vha;
> e84067d7 Duane Grigsby               2017-06-21  238  	int     rval = QLA_FUNCTION_FAILED;
> e84067d7 Duane Grigsby               2017-06-21  239  	struct qla_hw_data *ha;
> e84067d7 Duane Grigsby               2017-06-21  240  	srb_t           *sp;
> e84067d7 Duane Grigsby               2017-06-21  241  
> e84067d7 Duane Grigsby               2017-06-21 @242  	vha = fcport->vha;
>                                                               ^^^^^^^^^^^
> Dereference.
> 
> e84067d7 Duane Grigsby               2017-06-21  243  	ha = vha->hw;
> 69efeca5 Quinn Tran                  2019-06-18  244  
> 69efeca5 Quinn Tran                  2019-06-18 @245  	if (!ha->flags.fw_started || (fcport && fcport->deleted))
>                                                                                       ^^^^^^
> Check for NULL is too late.
> 
> 69efeca5 Quinn Tran                  2019-06-18  246  		return rval;
> 69efeca5 Quinn Tran                  2019-06-18  247  
> e84067d7 Duane Grigsby               2017-06-21  248  	/* Alloc SRB structure */
> e84067d7 Duane Grigsby               2017-06-21  249  	sp = qla2x00_get_sp(vha, fcport, GFP_ATOMIC);
> e84067d7 Duane Grigsby               2017-06-21  250  	if (!sp)
> e84067d7 Duane Grigsby               2017-06-21  251  		return rval;
> e84067d7 Duane Grigsby               2017-06-21  252  
> e84067d7 Duane Grigsby               2017-06-21  253  	sp->type = SRB_NVME_LS;
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>
diff mbox series

Patch

diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
index b43c62758cec..2c64457ce713 100644
--- a/drivers/scsi/qla2xxx/qla_nvme.c
+++ b/drivers/scsi/qla2xxx/qla_nvme.c
@@ -241,6 +241,10 @@  static int qla_nvme_ls_req(struct nvme_fc_local_port *lport,
 
 	vha = fcport->vha;
 	ha = vha->hw;
+
+	if (!ha->flags.fw_started || (fcport && fcport->deleted))
+		return rval;
+
 	/* Alloc SRB structure */
 	sp = qla2x00_get_sp(vha, fcport, GFP_ATOMIC);
 	if (!sp)
@@ -272,6 +276,7 @@  static int qla_nvme_ls_req(struct nvme_fc_local_port *lport,
 		    "qla2x00_start_sp failed = %d\n", rval);
 		atomic_dec(&sp->ref_count);
 		wake_up(&sp->nvme_ls_waitq);
+		sp->free(sp);
 		return rval;
 	}
 
@@ -488,7 +493,7 @@  static int qla_nvme_post_cmd(struct nvme_fc_local_port *lport,
 
 	vha = fcport->vha;
 
-	if (test_bit(ABORT_ISP_ACTIVE, &vha->dpc_flags))
+	if ((qpair && !qpair->fw_started) || (fcport && fcport->deleted))
 		return rval;
 
 	/*
@@ -523,6 +528,7 @@  static int qla_nvme_post_cmd(struct nvme_fc_local_port *lport,
 		    "qla2x00_start_nvme_mq failed = %d\n", rval);
 		atomic_dec(&sp->ref_count);
 		wake_up(&sp->nvme_ls_waitq);
+		sp->free(sp);
 	}
 
 	return rval;
@@ -549,14 +555,13 @@  static void qla_nvme_remoteport_delete(struct nvme_fc_remote_port *rport)
 
 	complete(&fcport->nvme_del_done);
 
-	if (!test_bit(UNLOADING, &fcport->vha->dpc_flags)) {
-		INIT_WORK(&fcport->free_work, qlt_free_session_done);
-		schedule_work(&fcport->free_work);
-	}
+	INIT_WORK(&fcport->free_work, qlt_free_session_done);
+	schedule_work(&fcport->free_work);
 
 	fcport->nvme_flag &= ~NVME_FLAG_DELETING;
 	ql_log(ql_log_info, fcport->vha, 0x2110,
-	    "remoteport_delete of %p completed.\n", fcport);
+	    "remoteport_delete of %p %8phN completed.\n",
+	    fcport, fcport->port_name);
 }
 
 static struct nvme_fc_port_template qla_nvme_fc_transport = {
@@ -588,7 +593,8 @@  static void qla_nvme_unregister_remote_port(struct work_struct *work)
 		return;
 
 	ql_log(ql_log_warn, NULL, 0x2112,
-	    "%s: unregister remoteport on %p\n",__func__, fcport);
+	    "%s: unregister remoteport on %p %8phN\n",
+	    __func__, fcport, fcport->port_name);
 
 	nvme_fc_set_remoteport_devloss(fcport->nvme_remote_port, 0);
 	init_completion(&fcport->nvme_del_done);