From: Ranjan Kumar <ranjan.kumar@broadcom.com>
To: linux-scsi@vger.kernel.org, martin.petersen@oracle.com
Cc: sathya.prakash@broadcom.com, chandrakanth.patil@broadcom.com,
sweeti.vandure@broadcom.com, vishakhavc@google.com,
ipylypiv@google.com, Ranjan Kumar <ranjan.kumar@broadcom.com>,
Sashiko <sashiko-bot@kernel.org>
Subject: [PATCH 16/21] scsi: mpi3mr: Fix sense buffer use after free on BSG command completion
Date: Wed, 4 Nov 2026 17:30:57 +0530 [thread overview]
Message-ID: <20261104120103.503871-17-ranjan.kumar@broadcom.com> (raw)
In-Reply-To: <20261104120103.503871-1-ranjan.kumar@broadcom.com>
The interrupt handler writes the reply and sense data of a BSG command
into the command tracker while the ioctl can time out, clear the sense
buffer pointer and free the buffer at the same time.
Add a lock to the command tracker and take it in the handler, the reset
flush and the ioctl. The callbacks run under it and never sleep. Also
initialize the two PEL trackers, which never had their lock set up.
Fixes: f762326b2baa ("scsi: mpi3mr: Propagate sense data for admin queue SCSI I/O")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260818231426.58105-1-chandrakanth.patil@broadcom.com?part=1
Closes: https://sashiko.dev/#/patchset/20260825210411.301535-1-chandrakanth.patil@broadcom.com?part=1
Closes: https://sashiko.dev/#/patchset/20260825210411.301535-1-chandrakanth.patil@broadcom.com?part=4
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
drivers/scsi/mpi3mr/mpi3mr.h | 1 +
drivers/scsi/mpi3mr/mpi3mr_app.c | 35 ++++++++++++++++++++++++--------
drivers/scsi/mpi3mr/mpi3mr_fw.c | 8 ++++++++
drivers/scsi/mpi3mr/mpi3mr_os.c | 3 +++
4 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
index 8738fc2ce7e7..deb8a1eea917 100644
--- a/drivers/scsi/mpi3mr/mpi3mr.h
+++ b/drivers/scsi/mpi3mr/mpi3mr.h
@@ -898,6 +898,7 @@ struct mpi3mr_sdev_priv_data {
*/
struct mpi3mr_drv_cmd {
struct mutex mutex;
+ spinlock_t state_lock;
struct completion done;
void *reply;
u8 *sensebuf;
diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
index 30a01b02a9fc..fb54bd8093da 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_app.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
@@ -2447,7 +2447,9 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
u8 *sgl_iter = NULL, *sgl_din_iter = NULL, *sgl_dout_iter = NULL;
u16 rmc_size = 0, desc_count = 0;
size_t req_min, buf_left;
- bool prp_owned = false, timed_out = false;
+ bool prp_owned = false, timed_out = false, completed;
+ unsigned long flags;
+ u16 cmd_state;
bsg_req = job->request;
karg = (struct mpi3mr_bsg_mptcmd *)&bsg_req->cmd.mptcmd;
@@ -2795,11 +2797,13 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
block_seq = mpi3mr_tgtdev_block_io_inc(mrioc, tgtdev);
}
+ spin_lock_irqsave(&mrioc->bsg_cmds.state_lock, flags);
mrioc->bsg_cmds.state = MPI3MR_CMD_PENDING;
mrioc->bsg_cmds.is_waiting = 1;
mrioc->bsg_cmds.callback = NULL;
mrioc->bsg_cmds.is_sense = 0;
mrioc->bsg_cmds.sensebuf = sense_buff_k;
+ spin_unlock_irqrestore(&mrioc->bsg_cmds.state_lock, flags);
memset(mrioc->bsg_cmds.reply, 0, mrioc->reply_sz);
mpi_header->host_tag = cpu_to_le16(MPI3MR_HOSTTAG_BSG_CMDS);
if (mrioc->logging_level & MPI3_DEBUG_BSG_INFO) {
@@ -2820,7 +2824,9 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
if (rval) {
+ spin_lock_irqsave(&mrioc->bsg_cmds.state_lock, flags);
mrioc->bsg_cmds.is_waiting = 0;
+ spin_unlock_irqrestore(&mrioc->bsg_cmds.state_lock, flags);
dprint_bsg_err(mrioc,
"%s: posting bsg request is failed\n", __func__);
rval = -EAGAIN;
@@ -2836,11 +2842,16 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
mpi3mr_tgtdev_block_io_dec(mrioc, tgtdev, block_seq);
mpi3mr_tgtdev_put(tgtdev);
}
- if (!(mrioc->bsg_cmds.state & MPI3MR_CMD_COMPLETE)) {
- timed_out = true;
+ spin_lock_irqsave(&mrioc->bsg_cmds.state_lock, flags);
+ cmd_state = mrioc->bsg_cmds.state;
+ completed = cmd_state & MPI3MR_CMD_COMPLETE;
+ if (!completed)
mrioc->bsg_cmds.is_waiting = 0;
+ spin_unlock_irqrestore(&mrioc->bsg_cmds.state_lock, flags);
+ if (!completed) {
+ timed_out = true;
rval = -EAGAIN;
- if (mrioc->bsg_cmds.state & MPI3MR_CMD_RESET)
+ if (cmd_state & MPI3MR_CMD_RESET)
goto out_unlock;
if (((mpi_header->function != MPI3_FUNCTION_SCSI_IO) &&
(mpi_header->function != MPI3_FUNCTION_NVME_ENCAPSULATED))
@@ -2870,8 +2881,11 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
MPI3MR_HOSTTAG_BLK_TMS, MPI3MR_RESETTM_TIMEOUT,
&mrioc->host_tm_cmds, &resp_code, NULL);
}
- if (!(mrioc->bsg_cmds.state & MPI3MR_CMD_COMPLETE) &&
- !(mrioc->bsg_cmds.state & MPI3MR_CMD_RESET))
+ spin_lock_irqsave(&mrioc->bsg_cmds.state_lock, flags);
+ cmd_state = mrioc->bsg_cmds.state;
+ spin_unlock_irqrestore(&mrioc->bsg_cmds.state_lock, flags);
+ if (!(cmd_state & MPI3MR_CMD_COMPLETE) &&
+ !(cmd_state & MPI3MR_CMD_RESET))
mpi3mr_soft_reset_handler(mrioc,
MPI3MR_RESET_FROM_APP_TIMEOUT, 1);
goto out_unlock;
@@ -2947,14 +2961,17 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
out_unlock:
if (prp_owned) {
+ spin_lock_irqsave(&mrioc->bsg_cmds.state_lock, flags);
+ cmd_state = mrioc->bsg_cmds.state;
+ spin_unlock_irqrestore(&mrioc->bsg_cmds.state_lock, flags);
/*
* Do not free the PRP list if a timed out command may still be
* using it. Leaking it is safer than letting the controller read
* freed memory.
*/
if (!timed_out ||
- (mrioc->bsg_cmds.state & MPI3MR_CMD_COMPLETE) ||
- ((mrioc->bsg_cmds.state & MPI3MR_CMD_RESET) &&
+ (cmd_state & MPI3MR_CMD_COMPLETE) ||
+ ((cmd_state & MPI3MR_CMD_RESET) &&
!mrioc->unrecoverable))
dma_free_coherent(&mrioc->pdev->dev, mrioc->prp_sz,
mrioc->prp_list_virt, mrioc->prp_list_dma);
@@ -2966,9 +2983,11 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
job->reply_payload.sg_cnt,
din_buf, job->reply_payload.payload_len);
}
+ spin_lock_irqsave(&mrioc->bsg_cmds.state_lock, flags);
mrioc->bsg_cmds.is_sense = 0;
mrioc->bsg_cmds.sensebuf = NULL;
mrioc->bsg_cmds.state = MPI3MR_CMD_NOTUSED;
+ spin_unlock_irqrestore(&mrioc->bsg_cmds.state_lock, flags);
mutex_unlock(&mrioc->bsg_cmds.mutex);
out:
kfree(sense_buff_k);
diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
index eb3f41f50649..6d43112aebb7 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
@@ -442,6 +442,9 @@ static void mpi3mr_process_admin_reply_desc(struct mpi3mr_ioc *mrioc,
cmdptr = mpi3mr_get_drv_cmd(mrioc, host_tag, def_reply);
if (cmdptr) {
+ unsigned long flags;
+
+ spin_lock_irqsave(&cmdptr->state_lock, flags);
if (cmdptr->state & MPI3MR_CMD_PENDING) {
cmdptr->state |= MPI3MR_CMD_COMPLETE;
cmdptr->ioc_loginfo = ioc_loginfo;
@@ -466,6 +469,7 @@ static void mpi3mr_process_admin_reply_desc(struct mpi3mr_ioc *mrioc,
} else if (cmdptr->callback)
cmdptr->callback(mrioc, cmdptr);
}
+ spin_unlock_irqrestore(&cmdptr->state_lock, flags);
}
out:
if (sense_buf)
@@ -5333,6 +5337,9 @@ void mpi3mr_cleanup_ioc(struct mpi3mr_ioc *mrioc)
static inline void mpi3mr_drv_cmd_comp_reset(struct mpi3mr_ioc *mrioc,
struct mpi3mr_drv_cmd *cmdptr)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&cmdptr->state_lock, flags);
if (cmdptr->state & MPI3MR_CMD_PENDING) {
cmdptr->state |= MPI3MR_CMD_RESET;
cmdptr->state &= ~MPI3MR_CMD_PENDING;
@@ -5342,6 +5349,7 @@ static inline void mpi3mr_drv_cmd_comp_reset(struct mpi3mr_ioc *mrioc,
} else if (cmdptr->callback)
cmdptr->callback(mrioc, cmdptr);
}
+ spin_unlock_irqrestore(&cmdptr->state_lock, flags);
}
/**
diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
index 67d41e4a0a0a..ed9e83a4119c 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_os.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
@@ -5582,6 +5582,7 @@ static inline void mpi3mr_init_drv_cmd(struct mpi3mr_drv_cmd *cmdptr,
u16 host_tag)
{
mutex_init(&cmdptr->mutex);
+ spin_lock_init(&cmdptr->state_lock);
cmdptr->reply = NULL;
cmdptr->state = MPI3MR_CMD_NOTUSED;
cmdptr->dev_handle = MPI3MR_INVALID_DEV_HANDLE;
@@ -5732,6 +5733,8 @@ mpi3mr_probe(struct pci_dev *pdev, const struct pci_device_id *id)
mpi3mr_init_drv_cmd(&mrioc->cfg_cmds, MPI3MR_HOSTTAG_CFG_CMDS);
mpi3mr_init_drv_cmd(&mrioc->transport_cmds,
MPI3MR_HOSTTAG_TRANSPORT_CMDS);
+ mpi3mr_init_drv_cmd(&mrioc->pel_cmds, MPI3MR_HOSTTAG_PEL_WAIT);
+ mpi3mr_init_drv_cmd(&mrioc->pel_abort_cmd, MPI3MR_HOSTTAG_PEL_ABORT);
for (i = 0; i < MPI3MR_NUM_DEVRMCMD; i++)
mpi3mr_init_drv_cmd(&mrioc->dev_rmhs_cmds[i],
--
2.47.3
next prev parent reply other threads:[~2026-11-04 12:10 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-04 12:00 [PATCH 00/21] scsi: mpi3mr: Fix issues reported by Sashiko Ranjan Kumar
2026-11-04 12:00 ` [PATCH 01/21] scsi: mpi3mr: Fix out-of-bounds read of the BSG request Ranjan Kumar
2026-11-04 12:00 ` [PATCH 02/21] scsi: mpi3mr: Fix integer wrap in BSG buffer entry lengths Ranjan Kumar
2026-11-04 12:00 ` [PATCH 03/21] scsi: mpi3mr: Remove redundant BSG buffer length checks Ranjan Kumar
2026-11-04 12:00 ` [PATCH 04/21] scsi: mpi3mr: Fix PRP list leak in BSG passthrough Ranjan Kumar
2026-11-04 12:00 ` [PATCH 05/21] scsi: mpi3mr: Fix reposting of invalid reply address on the admin queue Ranjan Kumar
2026-11-04 12:00 ` [PATCH 06/21] scsi: mpi3mr: Fix endianness of device handles in BSG requests Ranjan Kumar
2026-11-04 12:00 ` [PATCH 07/21] scsi: mpi3mr: Fix integer overflow in the HDB upload bounds check Ranjan Kumar
2026-11-04 12:00 ` [PATCH 08/21] scsi: mpi3mr: Fix integer overflow in the log data size calculation Ranjan Kumar
2026-11-04 12:00 ` [PATCH 09/21] scsi: mpi3mr: Queue DEVICE_INFO_CHANGED events to the bottom half again Ranjan Kumar
2026-11-04 12:00 ` [PATCH 10/21] scsi: mpi3mr: Fix target device reference leaks Ranjan Kumar
2026-11-04 12:00 ` [PATCH 11/21] scsi: mpi3mr: Fix the 32 bit shift when building the phy mask Ranjan Kumar
2026-11-04 12:00 ` [PATCH 12/21] scsi: mpi3mr: Fix empty device list race in the all targets query Ranjan Kumar
2026-11-04 12:00 ` [PATCH 13/21] scsi: mpi3mr: Fix the lowest phy number for phys 32 and above Ranjan Kumar
2026-11-04 12:00 ` [PATCH 14/21] scsi: mpi3mr: Fix target private data use after free in BSG passthrough Ranjan Kumar
2026-11-04 12:28 ` sashiko-bot
2026-11-04 12:00 ` [PATCH 15/21] scsi: mpi3mr: Fix block_io accounting when a target is re-created during BSG Ranjan Kumar
2026-11-04 12:28 ` sashiko-bot
2026-11-04 12:00 ` Ranjan Kumar [this message]
2026-11-04 12:26 ` [PATCH 16/21] scsi: mpi3mr: Fix sense buffer use after free on BSG command completion sashiko-bot
2026-11-04 12:00 ` [PATCH 17/21] scsi: mpi3mr: Reject a too small reply size when enabling log data Ranjan Kumar
2026-11-04 12:00 ` [PATCH 18/21] scsi: mpi3mr: Fix partly written and lost entries in the log data buffer Ranjan Kumar
2026-11-04 12:01 ` [PATCH 19/21] scsi: mpi3mr: Fix byte order handling in the NVMe passthrough commands Ranjan Kumar
2026-11-04 12:01 ` [PATCH 20/21] scsi: mpi3mr: Fix byte order of the BSG status reply Ranjan Kumar
2026-11-04 12:01 ` [PATCH 21/21] scsi: mpi3mr: Use the correct conversion for the diag buffer post request Ranjan Kumar
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=20261104120103.503871-17-ranjan.kumar@broadcom.com \
--to=ranjan.kumar@broadcom.com \
--cc=chandrakanth.patil@broadcom.com \
--cc=ipylypiv@google.com \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=sashiko-bot@kernel.org \
--cc=sathya.prakash@broadcom.com \
--cc=sweeti.vandure@broadcom.com \
--cc=vishakhavc@google.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox