All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <error27@gmail.com>
To: james.smart@broadcom.com
Cc: linux-scsi@vger.kernel.org
Subject: [bug report] scsi: lpfc: Rework locations of ndlp reference taking
Date: Tue, 15 Nov 2022 17:13:18 +0300	[thread overview]
Message-ID: <Y3OefhyyJNKH/iaf@kili> (raw)

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

             reply	other threads:[~2022-11-15 14:13 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-15 14:13 Dan Carpenter [this message]
  -- strict thread matches above, loose matches on Subject: below --
2020-11-27  9:54 [bug report] scsi: lpfc: Rework locations of ndlp reference taking Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y3OefhyyJNKH/iaf@kili \
    --to=error27@gmail.com \
    --cc=james.smart@broadcom.com \
    --cc=linux-scsi@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.