Skip to content

Commit

Permalink
scsi: lpfc: Fix NULL pointer access in lpfc_nvme_info_show
Browse files Browse the repository at this point in the history
After making remoteport unregister requests, the ndlp nrport pointer was
stale.

Track when waiting for waiting for unregister completion callback and
adjust nldp pointer assignment.  Add a few safety checks for NULL
pointer values.

Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <james.smart@broadcom.com>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
  • Loading branch information
James Smart authored and Martin K. Petersen committed Apr 18, 2018
1 parent 0cdb84e commit 0146602
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 10 deletions.
16 changes: 12 additions & 4 deletions drivers/scsi/lpfc/lpfc_attr.c
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ lpfc_nvme_info_show(struct device *dev, struct device_attribute *attr,
struct lpfc_nvmet_tgtport *tgtp;
struct nvme_fc_local_port *localport;
struct lpfc_nvme_lport *lport;
struct lpfc_nvme_rport *rport;
struct lpfc_nodelist *ndlp;
struct nvme_fc_remote_port *nrport;
struct lpfc_nvme_ctrl_stat *cstat;
Expand Down Expand Up @@ -312,11 +313,14 @@ lpfc_nvme_info_show(struct device *dev, struct device_attribute *attr,
localport->port_id, statep);

list_for_each_entry(ndlp, &vport->fc_nodes, nlp_listp) {
if (!ndlp->nrport)
rport = lpfc_ndlp_get_nrport(ndlp);
if (!rport)
continue;

/* local short-hand pointer. */
nrport = ndlp->nrport->remoteport;
nrport = rport->remoteport;
if (!nrport)
continue;

/* Port state is only one of two values for now. */
switch (nrport->port_state) {
Expand Down Expand Up @@ -3290,6 +3294,9 @@ lpfc_update_rport_devloss_tmo(struct lpfc_vport *vport)
{
struct Scsi_Host *shost;
struct lpfc_nodelist *ndlp;
#if (IS_ENABLED(CONFIG_NVME_FC))
struct lpfc_nvme_rport *rport;
#endif

shost = lpfc_shost_from_vport(vport);
spin_lock_irq(shost->host_lock);
Expand All @@ -3299,8 +3306,9 @@ lpfc_update_rport_devloss_tmo(struct lpfc_vport *vport)
if (ndlp->rport)
ndlp->rport->dev_loss_tmo = vport->cfg_devloss_tmo;
#if (IS_ENABLED(CONFIG_NVME_FC))
if (ndlp->nrport)
nvme_fc_set_remoteport_devloss(ndlp->nrport->remoteport,
rport = lpfc_ndlp_get_nrport(ndlp);
if (rport)
nvme_fc_set_remoteport_devloss(rport->remoteport,
vport->cfg_devloss_tmo);
#endif
}
Expand Down
8 changes: 6 additions & 2 deletions drivers/scsi/lpfc/lpfc_debugfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,7 @@ lpfc_debugfs_nodelist_data(struct lpfc_vport *vport, char *buf, int size)
struct nvme_fc_local_port *localport;
struct lpfc_nvmet_tgtport *tgtp;
struct nvme_fc_remote_port *nrport;
struct lpfc_nvme_rport *rport;

cnt = (LPFC_NODELIST_SIZE / LPFC_NODELIST_ENTRY_SIZE);
outio = 0;
Expand Down Expand Up @@ -695,10 +696,13 @@ lpfc_debugfs_nodelist_data(struct lpfc_vport *vport, char *buf, int size)
len += snprintf(buf + len, size - len, "\tRport List:\n");
list_for_each_entry(ndlp, &vport->fc_nodes, nlp_listp) {
/* local short-hand pointer. */
if (!ndlp->nrport)
rport = lpfc_ndlp_get_nrport(ndlp);
if (!rport)
continue;

nrport = ndlp->nrport->remoteport;
nrport = rport->remoteport;
if (!nrport)
continue;

/* Port state is only one of two values for now. */
switch (nrport->port_state) {
Expand Down
13 changes: 9 additions & 4 deletions drivers/scsi/lpfc/lpfc_nvme.c
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,7 @@ lpfc_nvme_remoteport_delete(struct nvme_fc_remote_port *remoteport)
remoteport);
spin_lock_irq(&vport->phba->hbalock);
ndlp->nrport = NULL;
ndlp->upcall_flags &= ~NLP_WAIT_FOR_UNREG;
spin_unlock_irq(&vport->phba->hbalock);

/* Remove original register reference. The host transport
Expand Down Expand Up @@ -2646,6 +2647,7 @@ lpfc_nvme_register_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
struct nvme_fc_local_port *localport;
struct lpfc_nvme_lport *lport;
struct lpfc_nvme_rport *rport;
struct lpfc_nvme_rport *oldrport;
struct nvme_fc_remote_port *remote_port;
struct nvme_fc_port_info rpinfo;
struct lpfc_nodelist *prev_ndlp;
Expand Down Expand Up @@ -2678,7 +2680,9 @@ lpfc_nvme_register_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)

rpinfo.port_name = wwn_to_u64(ndlp->nlp_portname.u.wwn);
rpinfo.node_name = wwn_to_u64(ndlp->nlp_nodename.u.wwn);
if (!ndlp->nrport)

oldrport = lpfc_ndlp_get_nrport(ndlp);
if (!oldrport)
lpfc_nlp_get(ndlp);

ret = nvme_fc_register_remoteport(localport, &rpinfo, &remote_port);
Expand All @@ -2688,8 +2692,8 @@ lpfc_nvme_register_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
* new rport.
*/
rport = remote_port->private;
if (ndlp->nrport) {
if (ndlp->nrport == remote_port->private) {
if (oldrport) {
if (oldrport == remote_port->private) {
/* Same remoteport. Just reuse. */
lpfc_printf_vlog(ndlp->vport, KERN_INFO,
LOG_NVME_DISC,
Expand All @@ -2713,6 +2717,7 @@ lpfc_nvme_register_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
*/
spin_lock_irq(&vport->phba->hbalock);
ndlp->nrport = NULL;
ndlp->upcall_flags &= ~NLP_WAIT_FOR_UNREG;
spin_unlock_irq(&vport->phba->hbalock);
rport->ndlp = NULL;
rport->remoteport = NULL;
Expand Down Expand Up @@ -2785,7 +2790,7 @@ lpfc_nvme_unregister_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
if (!lport)
goto input_err;

rport = ndlp->nrport;
rport = lpfc_ndlp_get_nrport(ndlp);
if (!rport)
goto input_err;

Expand Down
4 changes: 4 additions & 0 deletions drivers/scsi/lpfc/lpfc_nvme.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@
#define LPFC_NVME_FB_SHIFT 9
#define LPFC_NVME_MAX_FB (1 << 20) /* 1M */

#define lpfc_ndlp_get_nrport(ndlp) \
((!ndlp->nrport || (ndlp->upcall_flags & NLP_WAIT_FOR_UNREG)) \
? NULL : ndlp->nrport)

struct lpfc_nvme_qhandle {
uint32_t index; /* WQ index to use */
uint32_t qidx; /* queue index passed to create */
Expand Down

0 comments on commit 0146602

Please sign in to comment.