From: Dan Carpenter <dan.carpenter@oracle.com>
To: james.smart@emulex.com
Cc: linux-scsi@vger.kernel.org
Subject: Re: [bug report] [SCSI] lpfc 8.3.24: Extend BSG infrastructure and add link diagnostics
Date: Wed, 13 Jan 2021 12:46:57 +0300 [thread overview]
Message-ID: <20210113094657.GH5105@kadam> (raw)
In-Reply-To: <X/61pr0UpP0M45ME@mwanda>
On Wed, Jan 13, 2021 at 11:56:06AM +0300, Dan Carpenter wrote:
> Hello James Smart,
>
> The patch 7ad20aa9d39a: "[SCSI] lpfc 8.3.24: Extend BSG
> infrastructure and add link diagnostics" from May 24, 2011, leads to
> the following static checker warning:
>
> drivers/scsi/lpfc/lpfc_bsg.c:4989 lpfc_bsg_issue_mbox()
> warn: 'dmabuf' was already freed.
>
> The problem is that lpfc_bsg_issue_mbox() call lpfc_bsg_handle_sli_cfg_ext()
> which calls lpfc_bsg_handle_sli_cfg_ebuf() which is where the bug really
> is, I think.
>
> drivers/scsi/lpfc/lpfc_bsg.c
> 4584 static int
> 4585 lpfc_bsg_handle_sli_cfg_ebuf(struct lpfc_hba *phba, struct bsg_job *job,
> 4586 struct lpfc_dmabuf *dmabuf)
> 4587 {
> 4588 int rc;
> 4589
> 4590 lpfc_printf_log(phba, KERN_INFO, LOG_LIBDFC,
> 4591 "2971 SLI_CONFIG buffer (type:x%x)\n",
> 4592 phba->mbox_ext_buf_ctx.mboxType);
> 4593
> 4594 if (phba->mbox_ext_buf_ctx.mboxType == mbox_rd) {
> 4595 if (phba->mbox_ext_buf_ctx.state != LPFC_BSG_MBOX_DONE) {
> 4596 lpfc_printf_log(phba, KERN_ERR, LOG_LIBDFC,
> 4597 "2972 SLI_CONFIG rd buffer state "
> 4598 "mismatch:x%x\n",
> 4599 phba->mbox_ext_buf_ctx.state);
> 4600 lpfc_bsg_mbox_ext_abort(phba);
> 4601 return -EPIPE;
> 4602 }
> 4603 rc = lpfc_bsg_read_ebuf_get(phba, job);
> 4604 if (rc == SLI_CONFIG_HANDLED)
> 4605 lpfc_bsg_dma_page_free(phba, dmabuf);
>
> I think this path is correct.
>
> 4606 } else { /* phba->mbox_ext_buf_ctx.mboxType == mbox_wr */
> 4607 if (phba->mbox_ext_buf_ctx.state != LPFC_BSG_MBOX_HOST) {
> 4608 lpfc_printf_log(phba, KERN_ERR, LOG_LIBDFC,
> 4609 "2973 SLI_CONFIG wr buffer state "
> 4610 "mismatch:x%x\n",
> 4611 phba->mbox_ext_buf_ctx.state);
> 4612 lpfc_bsg_mbox_ext_abort(phba);
> 4613 return -EPIPE;
> 4614 }
> 4615 rc = lpfc_bsg_write_ebuf_set(phba, job, dmabuf);
>
> But this path has two bugs. If lpfc_bsg_write_ebuf_set() then it frees
> three things but it should not free anything. This leads to the double
> free bug which Smatch is complaining about. (Smatch only catches one of
> the problems).
Actually the other two are local variables so freeing them was correct.
But I've added the mempool_free() to Smatch as a free function since it
wasn't tracking that before.
regards,
dan carpenter
prev parent reply other threads:[~2021-01-13 9:55 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-13 8:56 [bug report] [SCSI] lpfc 8.3.24: Extend BSG infrastructure and add link diagnostics Dan Carpenter
2021-01-13 9:46 ` Dan Carpenter [this message]
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=20210113094657.GH5105@kadam \
--to=dan.carpenter@oracle.com \
--cc=james.smart@emulex.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.