* [bug report] scsi: lpfc: Rework locations of ndlp reference taking
@ 2020-11-27 9:54 Dan Carpenter
0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2020-11-27 9:54 UTC (permalink / raw)
To: james.smart; +Cc: linux-scsi
Hello James Smart,
This is a semi-automatic email about new static checker warnings.
The patch 4430f7fd09ec: "scsi: lpfc: Rework locations of ndlp
reference taking" from Nov 15, 2020, leads to the following Smatch
complaint:
drivers/scsi/lpfc/lpfc_els.c:2043 lpfc_cmpl_els_plogi()
error: we previously assumed 'ndlp' could be null (see line 1942)
drivers/scsi/lpfc/lpfc_els.c
1941 ndlp = lpfc_findnode_did(vport, irsp->un.elsreq64.remoteID);
1942 if (!ndlp) {
^^^^
1943 lpfc_printf_vlog(vport, KERN_ERR, LOG_TRACE_EVENT,
1944 "0136 PLOGI completes to NPort x%x "
1945 "with no ndlp. Data: x%x x%x x%x\n",
1946 irsp->un.elsreq64.remoteID,
1947 irsp->ulpStatus, irsp->un.ulpWord[4],
1948 irsp->ulpIoTag);
1949 goto out;
^^^^^^^^
"ndlp" is NULL
1950 }
1951
1952 /* Since ndlp can be freed in the disc state machine, note if this node
1953 * is being used during discovery.
1954 */
1955 spin_lock_irq(&ndlp->lock);
1956 disc = (ndlp->nlp_flag & NLP_NPR_2B_DISC);
1957 ndlp->nlp_flag &= ~NLP_NPR_2B_DISC;
1958 spin_unlock_irq(&ndlp->lock);
1959
1960 /* PLOGI completes to NPort <nlp_DID> */
1961 lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS,
1962 "0102 PLOGI completes to NPort x%06x "
1963 "Data: x%x x%x x%x x%x x%x\n",
1964 ndlp->nlp_DID, ndlp->nlp_fc4_type,
1965 irsp->ulpStatus, irsp->un.ulpWord[4],
1966 disc, vport->num_disc_nodes);
1967
1968 /* Check to see if link went down during discovery */
1969 if (lpfc_els_chk_latt(vport)) {
1970 spin_lock_irq(&ndlp->lock);
1971 ndlp->nlp_flag |= NLP_NPR_2B_DISC;
1972 spin_unlock_irq(&ndlp->lock);
1973 goto out;
1974 }
1975
1976 if (irsp->ulpStatus) {
1977 /* Check for retry */
1978 if (lpfc_els_retry(phba, cmdiocb, rspiocb)) {
1979 /* ELS command is being retried */
1980 if (disc) {
1981 spin_lock_irq(&ndlp->lock);
1982 ndlp->nlp_flag |= NLP_NPR_2B_DISC;
1983 spin_unlock_irq(&ndlp->lock);
1984 }
1985 goto out;
1986 }
1987 /* PLOGI failed Don't print the vport to vport rjts */
1988 if (irsp->ulpStatus != IOSTAT_LS_RJT ||
1989 (((irsp->un.ulpWord[4]) >> 16 != LSRJT_INVALID_CMD) &&
1990 ((irsp->un.ulpWord[4]) >> 16 != LSRJT_UNABLE_TPC)) ||
1991 (phba)->pport->cfg_log_verbose & LOG_ELS)
1992 lpfc_printf_vlog(vport, KERN_ERR, LOG_TRACE_EVENT,
1993 "2753 PLOGI failure DID:%06X Status:x%x/x%x\n",
1994 ndlp->nlp_DID, irsp->ulpStatus,
1995 irsp->un.ulpWord[4]);
1996
1997 /* Do not call DSM for lpfc_els_abort'ed ELS cmds */
1998 if (lpfc_error_lost_link(irsp))
1999 goto check_plogi;
2000 else
2001 lpfc_disc_state_machine(vport, ndlp, cmdiocb,
2002 NLP_EVT_CMPL_PLOGI);
2003
2004 /* As long as this node is not registered with the scsi or nvme
2005 * transport, it is no longer an active node. Otherwise
2006 * devloss handles the final cleanup.
2007 */
2008 if (!(ndlp->fc4_xpt_flags & (SCSI_XPT_REGD | NVME_XPT_REGD))) {
2009 spin_lock_irq(&ndlp->lock);
2010 ndlp->nlp_flag &= ~NLP_NPR_2B_DISC;
2011 spin_unlock_irq(&ndlp->lock);
2012 lpfc_disc_state_machine(vport, ndlp, cmdiocb,
2013 NLP_EVT_DEVICE_RM);
2014 }
2015 } else {
2016 /* Good status, call state machine */
2017 prsp = list_entry(((struct lpfc_dmabuf *)
2018 cmdiocb->context2)->list.next,
2019 struct lpfc_dmabuf, list);
2020 ndlp = lpfc_plogi_confirm_nport(phba, prsp->virt, ndlp);
2021 lpfc_disc_state_machine(vport, ndlp, cmdiocb,
2022 NLP_EVT_CMPL_PLOGI);
2023 }
2024
2025 check_plogi:
2026 if (disc && vport->num_disc_nodes) {
2027 /* Check to see if there are more PLOGIs to be sent */
2028 lpfc_more_plogi(vport);
2029
2030 if (vport->num_disc_nodes == 0) {
2031 spin_lock_irq(shost->host_lock);
2032 vport->fc_flag &= ~FC_NDISC_ACTIVE;
2033 spin_unlock_irq(shost->host_lock);
2034
2035 lpfc_can_disctmo(vport);
2036 lpfc_end_rscn(vport);
2037 }
2038 }
2039
2040 out:
2041 lpfc_debugfs_disc_trc(vport, LPFC_DISC_TRC_NODE,
2042 "PLOGI Cmpl PUT: did:x%x refcnt %d",
2043 ndlp->nlp_DID, kref_read(&ndlp->kref), 0);
^^^^^^^^^^^^^ ^^^^^^^^^^^
Dereferenced here.
2044
2045 /* Release the reference on the original I/O request. */
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* [bug report] scsi: lpfc: Rework locations of ndlp reference taking
@ 2022-11-15 14:13 Dan Carpenter
0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2022-11-15 14:13 UTC (permalink / raw)
To: james.smart; +Cc: linux-scsi
Hello James Smart,
The patch 4430f7fd09ec: "scsi: lpfc: Rework locations of ndlp
reference taking" from Nov 15, 2020, leads to the following Smatch
static checker warning:
drivers/scsi/lpfc/lpfc_els.c:5221 lpfc_cmpl_els_logo_acc()
warn: 'ndlp' was already freed.
drivers/scsi/lpfc/lpfc_els.c
5162 static void
5163 lpfc_cmpl_els_logo_acc(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
5164 struct lpfc_iocbq *rspiocb)
5165 {
5166 struct lpfc_nodelist *ndlp = cmdiocb->ndlp;
5167 struct lpfc_vport *vport = cmdiocb->vport;
5168 u32 ulp_status, ulp_word4;
5169
5170 ulp_status = get_job_ulpstatus(phba, rspiocb);
5171 ulp_word4 = get_job_word4(phba, rspiocb);
5172
5173 lpfc_debugfs_disc_trc(vport, LPFC_DISC_TRC_ELS_RSP,
5174 "ACC LOGO cmpl: status:x%x/x%x did:x%x",
5175 ulp_status, ulp_word4, ndlp->nlp_DID);
5176 /* ACC to LOGO completes to NPort <nlp_DID> */
5177 lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS,
5178 "0109 ACC to LOGO completes to NPort x%x refcnt %d "
5179 "Data: x%x x%x x%x\n",
5180 ndlp->nlp_DID, kref_read(&ndlp->kref), ndlp->nlp_flag,
5181 ndlp->nlp_state, ndlp->nlp_rpi);
5182
5183 /* This clause allows the LOGO ACC to complete and free resources
5184 * for the Fabric Domain Controller. It does deliberately skip
5185 * the unreg_rpi and release rpi because some fabrics send RDP
5186 * requests after logging out from the initiator.
5187 */
5188 if (ndlp->nlp_type & NLP_FABRIC &&
5189 ((ndlp->nlp_DID & WELL_KNOWN_DID_MASK) != WELL_KNOWN_DID_MASK))
5190 goto out;
5191
5192 if (ndlp->nlp_state == NLP_STE_NPR_NODE) {
5193 /* If PLOGI is being retried, PLOGI completion will cleanup the
5194 * node. The NLP_NPR_2B_DISC flag needs to be retained to make
5195 * progress on nodes discovered from last RSCN.
5196 */
5197 if ((ndlp->nlp_flag & NLP_DELAY_TMO) &&
5198 (ndlp->nlp_last_elscmd == ELS_CMD_PLOGI))
5199 goto out;
5200
5201 /* NPort Recovery mode or node is just allocated */
5202 if (!lpfc_nlp_not_used(ndlp)) {
^^^^
lpfc_nlp_not_used() is a nightmare function from 2007 that frees ndlp if
it's holding the last reference.
5203 /* A LOGO is completing and the node is in NPR state.
5204 * Just unregister the RPI because the node is still
5205 * required.
5206 */
5207 lpfc_unreg_rpi(vport, ndlp);
5208 } else {
5209 /* Indicate the node has already released, should
^^^^^^^^^^^^^^^^^^^^^^^^^
Node already released on this path.
5210 * not reference to it from within lpfc_els_free_iocb.
5211 */
5212 cmdiocb->ndlp = NULL;
5213 }
5214 }
5215 out:
5216 /*
5217 * The driver received a LOGO from the rport and has ACK'd it.
5218 * At this point, the driver is done so release the IOCB
5219 */
5220 lpfc_els_free_iocb(phba, cmdiocb);
--> 5221 lpfc_nlp_put(ndlp);
^^^^
Double free.
5222 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-11-15 14:13 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-27 9:54 [bug report] scsi: lpfc: Rework locations of ndlp reference taking Dan Carpenter
2022-11-15 14:13 Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).