[PATCH 00/21] scsi: mpi3mr: Fix issues reported by Sashiko

[PATCH 00/21] scsi: mpi3mr: Fix issues reported by Sashiko

[Ranjan Kumar]

This series fixes a number of issues in the mpi3mr driver that were
reported by Sashiko in its reviews of recent mpi3mr patches.

The patches cover the following areas:

 - BSG request validation: the request length, integer wrap in the buffer
   entry lengths, redundant checks and the leak of the NVMe PRP list
 - Admin reply reposting, endianness of device handles and integer
   overflows in the HDB upload and log data size checks
 - The DEVICE_INFO_CHANGED event, target device reference leaks, the phy
   mask shifts and the all targets query
 - BSG use after free fixes, log data size and locking, and the byte
   order of the NVMe passthrough, the status reply and the diag buffer
   post request

Later patches build on earlier ones, so please apply the series in order.

Ranjan Kumar (21):
  scsi: mpi3mr: Fix out-of-bounds read of the BSG request
  scsi: mpi3mr: Fix integer wrap in BSG buffer entry lengths
  scsi: mpi3mr: Remove redundant BSG buffer length checks
  scsi: mpi3mr: Fix PRP list leak in BSG passthrough
  scsi: mpi3mr: Fix reposting of invalid reply address on the admin
    queue
  scsi: mpi3mr: Fix endianness of device handles in BSG requests
  scsi: mpi3mr: Fix integer overflow in the HDB upload bounds check
  scsi: mpi3mr: Fix integer overflow in the log data size calculation
  scsi: mpi3mr: Queue DEVICE_INFO_CHANGED events to the bottom half
    again
  scsi: mpi3mr: Fix target device reference leaks
  scsi: mpi3mr: Fix the 32 bit shift when building the phy mask
  scsi: mpi3mr: Fix empty device list race in the all targets query
  scsi: mpi3mr: Fix the lowest phy number for phys 32 and above
  scsi: mpi3mr: Fix target private data use after free in BSG
    passthrough
  scsi: mpi3mr: Fix block_io accounting when a target is re-created
    during BSG
  scsi: mpi3mr: Fix sense buffer use after free on BSG command
    completion
  scsi: mpi3mr: Reject a too small reply size when enabling log data
  scsi: mpi3mr: Fix partly written and lost entries in the log data
    buffer
  scsi: mpi3mr: Fix byte order handling in the NVMe passthrough commands
  scsi: mpi3mr: Fix byte order of the BSG status reply
  scsi: mpi3mr: Use the correct conversion for the diag buffer post
    request

 drivers/scsi/mpi3mr/mpi3mr.h           |   8 +
 drivers/scsi/mpi3mr/mpi3mr_app.c       | 250 +++++++++++++++++--------
 drivers/scsi/mpi3mr/mpi3mr_fw.c        |  20 +-
 drivers/scsi/mpi3mr/mpi3mr_os.c        |  83 +++++++-
 drivers/scsi/mpi3mr/mpi3mr_transport.c |  34 ++--
 5 files changed, 301 insertions(+), 94 deletions(-)


-- 
2.47.3

[PATCH 01/21] scsi: mpi3mr: Fix out-of-bounds read of the BSG request

[Ranjan Kumar]

The driver reads fields from a BSG request without checking its length. A
short request, or one that declares more buffer entries than it contains,
makes the driver read past the end of the buffer.

Return -EINVAL when the request is too short for the command or for the
number of buffer entries it declares.

Fixes: 4268fa751365 ("scsi: mpi3mr: Add bsg device support")
Fixes: f5e6d5a34376 ("scsi: mpi3mr: Add support for driver commands")
Fixes: 506bc1a0d6ba ("scsi: mpi3mr: Add support for MPT commands")
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/20260818231426.58105-1-chandrakanth.patil@broadcom.com?part=2
Closes: https://sashiko.dev/#/patchset/20260818231426.58105-1-chandrakanth.patil@broadcom.com?part=4
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=2
Closes: https://sashiko.dev/#/patchset/20260825210411.301535-1-chandrakanth.patil@broadcom.com?part=3
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
 drivers/scsi/mpi3mr/mpi3mr_app.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
index eadad89bfcfb..ba70226f9cee 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_app.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
@@ -1690,6 +1690,13 @@ static long mpi3mr_bsg_process_drv_cmds(struct bsg_job *job)
 	struct mpi3mr_bsg_packet *bsg_req = NULL;
 	struct mpi3mr_bsg_drv_cmd *drvrcmd = NULL;
 
+	if (job->request_len <
+	    offsetofend(struct mpi3mr_bsg_packet, cmd.drvrcmd.opcode)) {
+		pr_err("%s: invalid BSG driver command length(%u)\n",
+		    MPI3MR_DRIVER_NAME, job->request_len);
+		return -EINVAL;
+	}
+
 	bsg_req = job->request;
 	drvrcmd = &bsg_req->cmd.drvrcmd;
 
@@ -2416,10 +2423,22 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 	u8 *din_buf = NULL, *dout_buf = NULL;
 	u8 *sgl_iter = NULL, *sgl_din_iter = NULL, *sgl_dout_iter = NULL;
 	u16 rmc_size  = 0, desc_count = 0;
+	size_t req_min;
 
 	bsg_req = job->request;
 	karg = (struct mpi3mr_bsg_mptcmd *)&bsg_req->cmd.mptcmd;
 
+	req_min = offsetof(struct mpi3mr_bsg_packet,
+	    cmd.mptcmd.buf_entry_list.buf_entry);
+	if (job->request_len >= req_min)
+		req_min += karg->buf_entry_list.num_of_entries *
+		    sizeof(struct mpi3mr_buf_entry);
+	if (job->request_len < req_min) {
+		pr_err("%s: invalid BSG MPT command length(%u), need %zu\n",
+		    MPI3MR_DRIVER_NAME, job->request_len, req_min);
+		return -EINVAL;
+	}
+
 	mrioc = mpi3mr_bsg_verify_adapter(karg->mrioc_id);
 	if (!mrioc)
 		return -ENODEV;
@@ -2995,6 +3014,12 @@ static int mpi3mr_bsg_request(struct bsg_job *job)
 
 	struct mpi3mr_bsg_packet *bsg_req = job->request;
 
+	if (job->request_len < offsetofend(struct mpi3mr_bsg_packet, cmd_type)) {
+		pr_err("%s: invalid BSG request length(%u)\n",
+		    MPI3MR_DRIVER_NAME, job->request_len);
+		goto out;
+	}
+
 	switch (bsg_req->cmd_type) {
 	case MPI3MR_DRV_CMD:
 		rval = mpi3mr_bsg_process_drv_cmds(job);
@@ -3008,6 +3033,7 @@ static int mpi3mr_bsg_request(struct bsg_job *job)
 		break;
 	}
 
+out:
 	bsg_job_done(job, rval, reply_payload_rcv_len);
 
 	return 0;
-- 
2.47.3

[PATCH 02/21] scsi: mpi3mr: Fix integer wrap in BSG buffer entry lengths

[Ranjan Kumar]

The length of each BSG buffer entry is added to a pointer and the result is
compared with the end of the payload. On a 32 bit system a large length
makes the sum wrap around and pass the check, so the driver goes on to use
a buffer much larger than the payload.

Compare each entry length with the number of bytes left in the payload
before the pointer is moved.

Fixes: 506bc1a0d6ba ("scsi: mpi3mr: Add support for MPT commands")
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/20260818231426.58105-1-chandrakanth.patil@broadcom.com?part=2
Closes: https://sashiko.dev/#/patchset/20260825210411.301535-1-chandrakanth.patil@broadcom.com?part=1
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
 drivers/scsi/mpi3mr/mpi3mr_app.c | 35 +++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
index ba70226f9cee..6bdd43b05e20 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_app.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
@@ -2379,6 +2379,26 @@ static int mpi3mr_map_data_buffer_dma(struct mpi3mr_ioc *mrioc,
 	drv_buf->num_dma_desc = needed_desc;
 	return 0;
 }
+
+/**
+ * mpi3mr_bsg_is_dout_be - Check if a buffer entry uses the data out buffer
+ * @buf_type: Buffer entry type
+ *
+ * Return: true if the entry is taken from the data out buffer and
+ * false if it is taken from the data in buffer.
+ */
+static bool mpi3mr_bsg_is_dout_be(u8 buf_type)
+{
+	switch (buf_type) {
+	case MPI3MR_BSG_BUFTYPE_RAIDMGMT_CMD:
+	case MPI3MR_BSG_BUFTYPE_DATA_OUT:
+	case MPI3MR_BSG_BUFTYPE_MPI_REQUEST:
+		return true;
+	default:
+		return false;
+	}
+}
+
 /**
  * mpi3mr_bsg_process_mpt_cmds - MPI Pass through BSG handler
  * @job: BSG job reference
@@ -2423,7 +2443,7 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 	u8 *din_buf = NULL, *dout_buf = NULL;
 	u8 *sgl_iter = NULL, *sgl_din_iter = NULL, *sgl_dout_iter = NULL;
 	u16 rmc_size  = 0, desc_count = 0;
-	size_t req_min;
+	size_t req_min, buf_left;
 
 	bsg_req = job->request;
 	karg = (struct mpi3mr_bsg_mptcmd *)&bsg_req->cmd.mptcmd;
@@ -2503,6 +2523,19 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 	drv_buf_iter = drv_bufs;
 
 	for (count = 0; count < bufcnt; count++, buf_entries++, drv_buf_iter++) {
+		if (mpi3mr_bsg_is_dout_be(buf_entries->buf_type))
+			buf_left = job->request_payload.payload_len -
+			    (sgl_dout_iter - dout_buf);
+		else
+			buf_left = job->reply_payload.payload_len -
+			    (sgl_din_iter - din_buf);
+		if (buf_entries->buf_len > buf_left) {
+			dprint_bsg_err(mrioc, "%s: buffer entry %u exceeds the payload\n",
+			    __func__, count);
+			mutex_unlock(&mrioc->bsg_cmds.mutex);
+			rval = -EINVAL;
+			goto out;
+		}
 
 		switch (buf_entries->buf_type) {
 		case MPI3MR_BSG_BUFTYPE_RAIDMGMT_CMD:
-- 
2.47.3

[PATCH 03/21] scsi: mpi3mr: Remove redundant BSG buffer length checks

[Ranjan Kumar]

Buffer entry lengths are now checked before the pointer moves, which makes
the later pointer comparisons dead code.

Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
 drivers/scsi/mpi3mr/mpi3mr_app.c | 23 -----------------------
 1 file changed, 23 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
index 6bdd43b05e20..9ab17eab191d 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_app.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
@@ -2601,14 +2601,6 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 				rval = -EINVAL;
 				goto out;
 			}
-			if (sgl_iter + mpi_msg_size >
-			    dout_buf + job->request_payload.payload_len) {
-				dprint_bsg_err(mrioc, "%s: MPI request buf exceeds dout_buf\n",
-					       __func__);
-				mutex_unlock(&mrioc->bsg_cmds.mutex);
-				rval = -EINVAL;
-				goto out;
-			}
 			memcpy(mpi_req, sgl_iter, mpi_msg_size);
 			break;
 		default:
@@ -2623,21 +2615,6 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 			goto out;
 		}
 
-		if (sgl_dout_iter > (dout_buf + job->request_payload.payload_len)) {
-			dprint_bsg_err(mrioc, "%s: data_out buffer length mismatch\n",
-				       __func__);
-			mutex_unlock(&mrioc->bsg_cmds.mutex);
-			rval = -EINVAL;
-			goto out;
-		}
-		if (sgl_din_iter > (din_buf + job->reply_payload.payload_len)) {
-			dprint_bsg_err(mrioc, "%s: data_in buffer length mismatch\n",
-				       __func__);
-			mutex_unlock(&mrioc->bsg_cmds.mutex);
-			rval = -EINVAL;
-			goto out;
-		}
-
 		drv_buf_iter->bsg_buf = sgl_iter;
 		drv_buf_iter->bsg_buf_len = buf_entries->buf_len;
 	}
-- 
2.47.3

[PATCH 04/21] scsi: mpi3mr: Fix PRP list leak in BSG passthrough

[Ranjan Kumar]

The PRP list built for an NVMe passthrough request is freed only after the
command completes. If posting the request fails or the command times out,
the list is never freed. The next request then overwrites the pointer and
the old buffer is lost.

Free the PRP list on every exit from the request handler.

If a timed out command is still pending after recovery, the controller may
still use its list. Accept a small leak in that case instead of freeing
memory the controller may still read.

Fixes: 7dbd0dd8cde3 ("scsi: mpi3mr: Add support for NVMe passthrough")
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/20260818231426.58105-1-chandrakanth.patil@broadcom.com?part=2
Closes: https://sashiko.dev/#/patchset/20260825210411.301535-1-chandrakanth.patil@broadcom.com?part=1
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
 drivers/scsi/mpi3mr/mpi3mr_app.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
index 9ab17eab191d..a559c99e3b30 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_app.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
@@ -2444,6 +2444,7 @@ 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;
 
 	bsg_req = job->request;
 	karg = (struct mpi3mr_bsg_mptcmd *)&bsg_req->cmd.mptcmd;
@@ -2748,6 +2749,8 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 				mutex_unlock(&mrioc->bsg_cmds.mutex);
 				goto out;
 			}
+			if (mrioc->prp_list_virt)
+				prp_owned = true;
 		} else if (nvme_fmt == MPI3MR_NVME_DATA_FORMAT_SGL1 ||
 			nvme_fmt == MPI3MR_NVME_DATA_FORMAT_SGL2) {
 			if (mpi3mr_build_nvme_sgl(mrioc,
@@ -2833,6 +2836,7 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 	if (block_io && stgt_priv)
 		atomic_dec(&stgt_priv->block_io);
 	if (!(mrioc->bsg_cmds.state & MPI3MR_CMD_COMPLETE)) {
+		timed_out = true;
 		mrioc->bsg_cmds.is_waiting = 0;
 		rval = -EAGAIN;
 		if (mrioc->bsg_cmds.state & MPI3MR_CMD_RESET)
@@ -2872,12 +2876,6 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 	}
 	dprint_bsg_info(mrioc, "%s: bsg request is completed\n", __func__);
 
-	if (mrioc->prp_list_virt) {
-		dma_free_coherent(&mrioc->pdev->dev, mrioc->prp_sz,
-		    mrioc->prp_list_virt, mrioc->prp_list_dma);
-		mrioc->prp_list_virt = NULL;
-	}
-
 	if ((mrioc->bsg_cmds.ioc_status & MPI3_IOCSTATUS_STATUS_MASK)
 	     != MPI3_IOCSTATUS_SUCCESS) {
 		dprint_bsg_info(mrioc,
@@ -2946,6 +2944,20 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 	}
 
 out_unlock:
+	if (prp_owned) {
+		/*
+		 * 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) &&
+		    !mrioc->unrecoverable))
+			dma_free_coherent(&mrioc->pdev->dev, mrioc->prp_sz,
+			    mrioc->prp_list_virt, mrioc->prp_list_dma);
+		mrioc->prp_list_virt = NULL;
+	}
 	if (din_buf) {
 		job->reply_payload_rcv_len =
 			sg_copy_from_buffer(job->reply_payload.sg_list,
-- 
2.47.3

[PATCH 05/21] scsi: mpi3mr: Fix reposting of invalid reply address on the admin queue

[Ranjan Kumar]

An admin reply whose reply frame address is not one of the driver's reply
buffers is still reposted to the hardware reply free queue. Clear the
address when the lookup fails so it is not reposted, as the operational
reply queues already do.

Fixes: 824a156633df ("scsi: mpi3mr: Base driver code")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260825210411.301535-1-chandrakanth.patil@broadcom.com?part=7
Closes: https://sashiko.dev/#/patchset/20260805110634.346670-1-ranjan.kumar@broadcom.com?part=5
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
 drivers/scsi/mpi3mr/mpi3mr_fw.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
index f0d3cd398dd0..eb3f41f50649 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
@@ -402,8 +402,15 @@ static void mpi3mr_process_admin_reply_desc(struct mpi3mr_ioc *mrioc,
 		addr_desc = (struct mpi3_address_reply_descriptor *)reply_desc;
 		*reply_dma = le64_to_cpu(addr_desc->reply_frame_address);
 		def_reply = mpi3mr_get_reply_virt_addr(mrioc, *reply_dma);
-		if (!def_reply)
+		if (!def_reply) {
+			ioc_err(mrioc, "def_reply is NULL, invalid reply_frame_address\n");
+			/*
+			 * Do not let the caller repost an address that
+			 * failed virt-addr lookup back to the hardware.
+			 */
+			*reply_dma = 0;
 			goto out;
+		}
 		host_tag = le16_to_cpu(def_reply->host_tag);
 		ioc_status = le16_to_cpu(def_reply->ioc_status);
 		if (ioc_status &
-- 
2.47.3

[PATCH 06/21] scsi: mpi3mr: Fix endianness of device handles in BSG requests

[Ranjan Kumar]

The device handle of a task management request, an NVMe encapsulated
request and the target reset sent after a timeout is read from a little
endian field and used as a CPU order value. On a big endian system this
gives the wrong device handle.

Convert the handle to CPU byte order before it is used.

Fixes: 506bc1a0d6ba ("scsi: mpi3mr: Add support for MPT commands")
Fixes: 7dbd0dd8cde3 ("scsi: mpi3mr: Add support for NVMe passthrough")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260818231426.58105-1-chandrakanth.patil@broadcom.com?part=4
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_app.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
index a559c99e3b30..83f798c4e9af 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_app.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
@@ -2082,7 +2082,7 @@ static int mpi3mr_build_nvme_prp(struct mpi3mr_ioc *mrioc,
 			    mrioc->facts.sge_mod_shift) << 32);
 	u64 sgemod_val = ((u64)(mrioc->facts.sge_mod_value) <<
 			  mrioc->facts.sge_mod_shift) << 32;
-	u16 dev_handle = nvme_encap_request->dev_handle;
+	u16 dev_handle = le16_to_cpu(nvme_encap_request->dev_handle);
 	struct mpi3mr_tgt_dev *tgtdev;
 	u16 desc_count = 0;
 
@@ -2782,7 +2782,7 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 		tm_req = (struct mpi3_scsi_task_mgmt_request *)mpi_req;
 		if (tm_req->task_type !=
 		    MPI3_SCSITASKMGMT_TASKTYPE_ABORT_TASK) {
-			dev_handle = tm_req->dev_handle;
+			dev_handle = le16_to_cpu(tm_req->dev_handle);
 			block_io = 1;
 		}
 	}
@@ -2861,10 +2861,11 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 			(mpi_header->function == MPI3_BSG_FUNCTION_SCSI_IO)) {
 			dprint_bsg_err(mrioc, "%s: bsg request timedout after %d seconds,\n"
 				"issuing target reset to (0x%04x)\n", __func__,
-				karg->timeout, mpi_header->function_dependent);
+				karg->timeout,
+				le16_to_cpu(mpi_header->function_dependent));
 			mpi3mr_issue_tm(mrioc,
 			    MPI3_SCSITASKMGMT_TASKTYPE_TARGET_RESET,
-			    mpi_header->function_dependent, 0,
+			    le16_to_cpu(mpi_header->function_dependent), 0,
 			    MPI3MR_HOSTTAG_BLK_TMS, MPI3MR_RESETTM_TIMEOUT,
 			    &mrioc->host_tm_cmds, &resp_code, NULL);
 		}
-- 
2.47.3

[PATCH 07/21] scsi: mpi3mr: Fix integer overflow in the HDB upload bounds check

[Ranjan Kumar]

The start offset and the length of an HDB upload request are added in 32
bits and compared with the buffer size. A large offset makes the sum wrap
around and pass the check, so the copy reads far outside the buffer.

Reject the request if the sum overflows.

Fixes: 78b506984ebe ("scsi: mpi3mr: Add ioctl support for HDB")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260818231426.58105-1-chandrakanth.patil@broadcom.com?part=15
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
 drivers/scsi/mpi3mr/mpi3mr_app.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
index 83f798c4e9af..a68552c9dafa 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_app.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
@@ -1117,6 +1117,7 @@ static long mpi3mr_bsg_upload_hdb(struct mpi3mr_ioc *mrioc,
 	struct diag_buffer_desc *diag_buffer;
 	uint32_t data_out_size;
 	uint32_t data_in_size;
+	u32 end;
 
 	data_out_size = job->request_payload.payload_len;
 	data_in_size = job->reply_payload.payload_len;
@@ -1151,7 +1152,8 @@ static long mpi3mr_bsg_upload_hdb(struct mpi3mr_ioc *mrioc,
 		return -EINVAL;
 	}
 
-	if ((upload_hdb.start_offset + upload_hdb.length) > diag_buffer->size) {
+	if (check_add_overflow(upload_hdb.start_offset, upload_hdb.length, &end) ||
+	    end > diag_buffer->size) {
 		dprint_bsg_err(mrioc,
 		    "%s: invalid start offset %d, length %d for type %d\n",
 		    __func__, upload_hdb.start_offset, upload_hdb.length,
-- 
2.47.3

[PATCH 08/21] scsi: mpi3mr: Fix integer overflow in the log data size calculation

[Ranjan Kumar]

The number of log entries and their total size are held in 16 bits. With a
large entry size the product wraps around, so the driver copies only part
of the log data and still reports success.

Use 32 bit variables for both values.

Fixes: f5e6d5a34376 ("scsi: mpi3mr: Add support for driver commands")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260825210411.301535-1-chandrakanth.patil@broadcom.com?part=5
Closes: https://sashiko.dev/#/patchset/20260825210411.301535-1-chandrakanth.patil@broadcom.com?part=13
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
 drivers/scsi/mpi3mr/mpi3mr_app.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
index a68552c9dafa..bd105efdd9cb 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_app.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
@@ -1346,7 +1346,8 @@ static long mpi3mr_enable_logdata(struct mpi3mr_ioc *mrioc,
 static long mpi3mr_get_logdata(struct mpi3mr_ioc *mrioc,
 	struct bsg_job *job)
 {
-	u16 num_entries, sz, entry_sz = mrioc->logdata_entry_sz;
+	u16 entry_sz = mrioc->logdata_entry_sz;
+	u32 num_entries, sz;
 
 	if ((!mrioc->logdata_buf) || (job->request_payload.payload_len < entry_sz))
 		return -EINVAL;
-- 
2.47.3

[PATCH 09/21] scsi: mpi3mr: Queue DEVICE_INFO_CHANGED events to the bottom half again

[Ranjan Kumar]

The DEVICE_INFO_CHANGED event shares its case with the LOG_DATA event.
When the log data handling was added to that case, DEVICE_INFO_CHANGED
stopped being queued to the bottom half, so device information changes
are no longer applied and the event is saved as log data instead.

Give DEVICE_INFO_CHANGED its own case that queues the bottom half.

Fixes: d0d19250ed81 ("scsi: mpi3mr: Rename log data save helper to reflect threaded/BH context")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260825210411.301535-1-chandrakanth.patil@broadcom.com?part=5
Closes: https://sashiko.dev/#/patchset/20260818231426.58105-1-chandrakanth.patil@broadcom.com?part=11
Closes: https://sashiko.dev/#/patchset/20260825210411.301535-1-chandrakanth.patil@broadcom.com?part=11
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
 drivers/scsi/mpi3mr/mpi3mr_os.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
index 6b0156e54d45..7de8f20e396a 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_os.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
@@ -3273,6 +3273,10 @@ void mpi3mr_os_handle_events(struct mpi3mr_ioc *mrioc,
 		break;
 	}
 	case MPI3_EVENT_DEVICE_INFO_CHANGED:
+	{
+		process_evt_bh = 1;
+		break;
+	}
 	case MPI3_EVENT_LOG_DATA:
 
 		if (mrioc->reply_sz > offsetof(struct mpi3_event_notification_reply, event_data))
-- 
2.47.3

[PATCH 10/21] scsi: mpi3mr: Fix target device reference leaks

[Ranjan Kumar]

Looking up a target device takes a reference. When a target allocation is
refused, or when a device has no HBA port, the reference is not released,
so the device is never freed and its count grows with every rescan.

Release the reference on these paths.

Fixes: 13ef29ea4aa0 ("scsi: mpi3mr: Add support for device add/remove event handling")
Fixes: 626665e9c38d ("scsi: mpi3mr: Get target object based on rphy")
Fixes: 8e45183978d6 ("scsi: mpi3mr: Bad drive in topology results kernel crash")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260818231426.58105-1-chandrakanth.patil@broadcom.com?part=9
Closes: https://sashiko.dev/#/patchset/20260825210411.301535-1-chandrakanth.patil@broadcom.com?part=9
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
 drivers/scsi/mpi3mr/mpi3mr_os.c        | 10 ++++++++--
 drivers/scsi/mpi3mr/mpi3mr_transport.c |  7 ++++---
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
index 7de8f20e396a..99928fcc5bd7 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_os.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
@@ -5114,8 +5114,11 @@ static int mpi3mr_target_alloc(struct scsi_target *starget)
 			scsi_tgt_priv_data->wslen = tgt_dev->wslen;
 			if (tgt_dev->dev_type == MPI3_DEVICE_DEVFORM_VD)
 				scsi_tgt_priv_data->throttle_group = tgt_dev->dev_spec.vd_inf.tg;
-		} else
+		} else {
+			if (tgt_dev)
+				mpi3mr_tgtdev_put(tgt_dev);
 			retval = -ENXIO;
+		}
 	} else if (mrioc->sas_transport_enabled && !starget->channel) {
 		rphy = dev_to_rphy(starget->dev.parent);
 		tgt_dev = __mpi3mr_get_tgtdev_by_addr_and_rphy(mrioc,
@@ -5132,8 +5135,11 @@ static int mpi3mr_target_alloc(struct scsi_target *starget)
 			tgt_dev->starget = starget;
 			atomic_set(&scsi_tgt_priv_data->block_io, 0);
 			retval = 0;
-		} else
+		} else {
+			if (tgt_dev)
+				mpi3mr_tgtdev_put(tgt_dev);
 			retval = -ENXIO;
+		}
 	}
 	spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
 
diff --git a/drivers/scsi/mpi3mr/mpi3mr_transport.c b/drivers/scsi/mpi3mr/mpi3mr_transport.c
index f1e34329f5a2..26dee121bc43 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_transport.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_transport.c
@@ -2575,9 +2575,10 @@ static u8 mpi3mr_get_port_id_by_rphy(struct mpi3mr_ioc *mrioc, struct sas_rphy *
 
 		tgtdev = __mpi3mr_get_tgtdev_by_addr_and_rphy(mrioc,
 			    rphy->identify.sas_address, rphy);
-		if (tgtdev && tgtdev->dev_spec.sas_sata_inf.hba_port) {
-			port_id =
-				tgtdev->dev_spec.sas_sata_inf.hba_port->port_id;
+		if (tgtdev) {
+			if (tgtdev->dev_spec.sas_sata_inf.hba_port)
+				port_id =
+					tgtdev->dev_spec.sas_sata_inf.hba_port->port_id;
 			mpi3mr_tgtdev_put(tgtdev);
 		}
 		spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
-- 
2.47.3

[PATCH 11/21] scsi: mpi3mr: Fix the 32 bit shift when building the phy mask

[Ranjan Kumar]

The phy mask is 64 bits wide but the bit for a phy is built with a 32 bit
shift. For phy numbers 31 and above the result is wrong or undefined, so
the mask gets the wrong bits for those phys.

Build the bit with a 64 bit shift.

Fixes: cb5b60894602 ("scsi: mpi3mr: Increase maximum number of PHYs to 64 from 32")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260818231426.58105-1-chandrakanth.patil@broadcom.com?part=12
Closes: https://sashiko.dev/#/patchset/20260825210411.301535-1-chandrakanth.patil@broadcom.com?part=12
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
 drivers/scsi/mpi3mr/mpi3mr_transport.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_transport.c b/drivers/scsi/mpi3mr/mpi3mr_transport.c
index 26dee121bc43..ecf312f007dc 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_transport.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_transport.c
@@ -610,7 +610,7 @@ static void mpi3mr_delete_sas_phy(struct mpi3mr_ioc *mrioc,
 	mr_sas_port->num_phys--;
 
 	if (host_node) {
-		mr_sas_port->phy_mask &= ~(1 << mr_sas_phy->phy_id);
+		mr_sas_port->phy_mask &= ~BIT_ULL(mr_sas_phy->phy_id);
 
 		if (mr_sas_port->lowest_phy == mr_sas_phy->phy_id)
 			mr_sas_port->lowest_phy = ffs(mr_sas_port->phy_mask) - 1;
@@ -641,7 +641,7 @@ static void mpi3mr_add_sas_phy(struct mpi3mr_ioc *mrioc,
 	list_add_tail(&mr_sas_phy->port_siblings, &mr_sas_port->phy_list);
 	mr_sas_port->num_phys++;
 	if (host_node) {
-		mr_sas_port->phy_mask |= (1 << mr_sas_phy->phy_id);
+		mr_sas_port->phy_mask |= BIT_ULL(mr_sas_phy->phy_id);
 
 		if (mr_sas_phy->phy_id < mr_sas_port->lowest_phy)
 			mr_sas_port->lowest_phy = ffs(mr_sas_port->phy_mask) - 1;
@@ -1417,7 +1417,7 @@ static struct mpi3mr_sas_port *mpi3mr_sas_port_add(struct mpi3mr_ioc *mrioc,
 		    &mr_sas_port->phy_list);
 		mr_sas_port->num_phys++;
 		if (mr_sas_node->host_node)
-			mr_sas_port->phy_mask |= (1 << i);
+			mr_sas_port->phy_mask |= BIT_ULL(i);
 	}
 
 	if (!mr_sas_port->num_phys) {
@@ -1814,7 +1814,7 @@ mpi3mr_refresh_sas_ports(struct mpi3mr_ioc *mrioc)
 		found = 0;
 		for (j = 0; j < host_port_count; j++) {
 			if (h_port[j].handle == attached_handle) {
-				h_port[j].phy_mask |= (1 << i);
+				h_port[j].phy_mask |= BIT_ULL(i);
 				found = 1;
 				break;
 			}
@@ -1841,7 +1841,7 @@ mpi3mr_refresh_sas_ports(struct mpi3mr_ioc *mrioc)
 		port_idx = host_port_count;
 		h_port[port_idx].sas_address = le64_to_cpu(sasinf->sas_address);
 		h_port[port_idx].handle = attached_handle;
-		h_port[port_idx].phy_mask = (1 << i);
+		h_port[port_idx].phy_mask = BIT_ULL(i);
 		h_port[port_idx].iounit_port_id = sas_io_unit_pg0->phy_data[i].io_unit_port;
 		h_port[port_idx].lowest_phy = sasinf->phy_num;
 		h_port[port_idx].used = 0;
-- 
2.47.3

[PATCH 12/21] scsi: mpi3mr: Fix empty device list race in the all targets query

[Ranjan Kumar]

The number of target devices is counted under the target lock but the list
is tested for emptiness later without it. If the devices go away in
between, the user is given a count with no device list.

Decide from the count that was taken under the lock.

Fixes: f5e6d5a34376 ("scsi: mpi3mr: Add support for driver commands")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260825210411.301535-1-chandrakanth.patil@broadcom.com?part=13
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
 drivers/scsi/mpi3mr/mpi3mr_app.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
index bd105efdd9cb..db27bff58c27 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_app.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
@@ -1499,7 +1499,7 @@ static long mpi3mr_get_all_tgt_info(struct mpi3mr_ioc *mrioc,
 	spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
 
 	if ((job->request_payload.payload_len <= sizeof(u64)) ||
-		list_empty(&mrioc->tgtdev_list)) {
+		!num_devices) {
 		sg_copy_from_buffer(job->request_payload.sg_list,
 				    job->request_payload.sg_cnt,
 				    &num_devices, sizeof(num_devices));
-- 
2.47.3

[PATCH 13/21] scsi: mpi3mr: Fix the lowest phy number for phys 32 and above

[Ranjan Kumar]

The phy mask is 64 bits wide but the lowest phy of a port is found with a
call that takes a 32 bit value, so only the low 32 bits of the mask are
looked at. For a port with only phys 32 and above the lowest phy comes out
as -1.

Find the lowest phy in the whole mask.

Fixes: cb5b60894602 ("scsi: mpi3mr: Increase maximum number of PHYs to 64 from 32")
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
 drivers/scsi/mpi3mr/mpi3mr_transport.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_transport.c b/drivers/scsi/mpi3mr/mpi3mr_transport.c
index ecf312f007dc..87f2e1841ce3 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_transport.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_transport.c
@@ -587,6 +587,17 @@ static enum sas_linkrate mpi3mr_convert_phy_link_rate(u8 link_rate)
 	return rc;
 }
 
+/**
+ * mpi3mr_lowest_phy - Get the lowest phy number in a phy mask
+ * @phy_mask: Phy mask of the port
+ *
+ * Return: The lowest phy number, or -1 if the mask is empty.
+ */
+static int mpi3mr_lowest_phy(u64 phy_mask)
+{
+	return phy_mask ? __ffs64(phy_mask) : -1;
+}
+
 /**
  * mpi3mr_delete_sas_phy - Remove a single phy from port
  * @mrioc: Adapter instance reference
@@ -613,7 +624,7 @@ static void mpi3mr_delete_sas_phy(struct mpi3mr_ioc *mrioc,
 		mr_sas_port->phy_mask &= ~BIT_ULL(mr_sas_phy->phy_id);
 
 		if (mr_sas_port->lowest_phy == mr_sas_phy->phy_id)
-			mr_sas_port->lowest_phy = ffs(mr_sas_port->phy_mask) - 1;
+			mr_sas_port->lowest_phy = mpi3mr_lowest_phy(mr_sas_port->phy_mask);
 	}
 	sas_port_delete_phy(mr_sas_port->port, mr_sas_phy->phy);
 	mr_sas_phy->phy_belongs_to_port = 0;
@@ -644,7 +655,7 @@ static void mpi3mr_add_sas_phy(struct mpi3mr_ioc *mrioc,
 		mr_sas_port->phy_mask |= BIT_ULL(mr_sas_phy->phy_id);
 
 		if (mr_sas_phy->phy_id < mr_sas_port->lowest_phy)
-			mr_sas_port->lowest_phy = ffs(mr_sas_port->phy_mask) - 1;
+			mr_sas_port->lowest_phy = mpi3mr_lowest_phy(mr_sas_port->phy_mask);
 	}
 	sas_port_add_phy(mr_sas_port->port, mr_sas_phy->phy);
 	mr_sas_phy->phy_belongs_to_port = 1;
@@ -1427,7 +1438,7 @@ static struct mpi3mr_sas_port *mpi3mr_sas_port_add(struct mpi3mr_ioc *mrioc,
 	}
 
 	if (mr_sas_node->host_node)
-		mr_sas_port->lowest_phy = ffs(mr_sas_port->phy_mask) - 1;
+		mr_sas_port->lowest_phy = mpi3mr_lowest_phy(mr_sas_port->phy_mask);
 
 	if (mr_sas_port->remote_identify.device_type == SAS_END_DEVICE) {
 		tgtdev = mpi3mr_get_tgtdev_by_addr(mrioc,
-- 
2.47.3

[PATCH 14/21] scsi: mpi3mr: Fix target private data use after free in BSG passthrough

[Ranjan Kumar]

A BSG task management request raises block_io on the SCSI target private
data and lowers it after the command completes. The pointer is read
without a lock and the target device reference is dropped before the
wait. If the device is removed meanwhile, the private data is freed and
block_io is decremented in freed memory.

Do the lookup and the update under the target device lock, free the
private data under the same lock, and keep the target device reference
until the count is released.

Fixes: 506bc1a0d6ba ("scsi: mpi3mr: Add support for MPT commands")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260818231426.58105-1-chandrakanth.patil@broadcom.com?part=3
Closes: https://sashiko.dev/#/patchset/20260818231426.58105-1-chandrakanth.patil@broadcom.com?part=4
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=3
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     |  5 +++
 drivers/scsi/mpi3mr/mpi3mr_app.c | 24 +++++++-------
 drivers/scsi/mpi3mr/mpi3mr_os.c  | 55 ++++++++++++++++++++++++++++++--
 3 files changed, 69 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
index d6e16707fd97..da47100d895e 100644
--- a/drivers/scsi/mpi3mr/mpi3mr.h
+++ b/drivers/scsi/mpi3mr/mpi3mr.h
@@ -1533,6 +1533,11 @@ int mpi3mr_issue_tm(struct mpi3mr_ioc *mrioc, u8 tm_type,
 	u16 handle, uint lun, u16 htag, ulong timeout,
 	struct mpi3mr_drv_cmd *drv_cmd,
 	u8 *resp_code, struct scsi_cmnd *scmd);
+struct mpi3mr_stgt_priv_data *mpi3mr_tgtdev_block_io_inc(struct mpi3mr_ioc *mrioc,
+							 struct mpi3mr_tgt_dev *tgtdev);
+void mpi3mr_tgtdev_block_io_dec(struct mpi3mr_ioc *mrioc,
+				struct mpi3mr_tgt_dev *tgtdev,
+				struct mpi3mr_stgt_priv_data *stgt_priv);
 struct mpi3mr_tgt_dev *mpi3mr_get_tgtdev_by_handle(
 	struct mpi3mr_ioc *mrioc, u16 handle);
 void mpi3mr_pel_get_seqnum_complete(struct mpi3mr_ioc *mrioc,
diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
index db27bff58c27..4df0dc81358b 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_app.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
@@ -2439,7 +2439,7 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 	struct mpi3_scsi_task_mgmt_request *tm_req;
 	u32 erbsz = MPI3MR_SENSE_BUF_SZ, tmplen;
 	u16 dev_handle;
-	struct mpi3mr_tgt_dev *tgtdev;
+	struct mpi3mr_tgt_dev *tgtdev = NULL;
 	struct mpi3mr_stgt_priv_data *stgt_priv = NULL;
 	struct mpi3mr_bsg_in_reply_buf *bsg_reply_buf = NULL;
 	u32 din_size = 0, dout_size = 0;
@@ -2791,14 +2791,8 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 	}
 	if (block_io) {
 		tgtdev = mpi3mr_get_tgtdev_by_handle(mrioc, dev_handle);
-		if (tgtdev) {
-			if (tgtdev->starget && tgtdev->starget->hostdata) {
-				stgt_priv = (struct mpi3mr_stgt_priv_data *)
-				    tgtdev->starget->hostdata;
-				atomic_inc(&stgt_priv->block_io);
-			}
-			mpi3mr_tgtdev_put(tgtdev);
-		}
+		if (tgtdev)
+			stgt_priv = mpi3mr_tgtdev_block_io_inc(mrioc, tgtdev);
 	}
 
 	mrioc->bsg_cmds.state = MPI3MR_CMD_PENDING;
@@ -2830,14 +2824,18 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 		dprint_bsg_err(mrioc,
 		    "%s: posting bsg request is failed\n", __func__);
 		rval = -EAGAIN;
-		if (block_io && stgt_priv)
-			atomic_dec(&stgt_priv->block_io);
+		if (tgtdev) {
+			mpi3mr_tgtdev_block_io_dec(mrioc, tgtdev, stgt_priv);
+			mpi3mr_tgtdev_put(tgtdev);
+		}
 		goto out_unlock;
 	}
 	wait_for_completion_timeout(&mrioc->bsg_cmds.done,
 	    (karg->timeout * HZ));
-	if (block_io && stgt_priv)
-		atomic_dec(&stgt_priv->block_io);
+	if (tgtdev) {
+		mpi3mr_tgtdev_block_io_dec(mrioc, tgtdev, stgt_priv);
+		mpi3mr_tgtdev_put(tgtdev);
+	}
 	if (!(mrioc->bsg_cmds.state & MPI3MR_CMD_COMPLETE)) {
 		timed_out = true;
 		mrioc->bsg_cmds.is_waiting = 0;
diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
index 99928fcc5bd7..e7833b24c509 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_os.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
@@ -788,6 +788,58 @@ struct mpi3mr_tgt_dev *mpi3mr_get_tgtdev_by_handle(
 	return tgtdev;
 }
 
+/**
+ * mpi3mr_tgtdev_block_io_inc - Block I/O on the SCSI target of a tgtdev
+ * @mrioc: Adapter instance reference
+ * @tgtdev: Target device reference, held by the caller
+ *
+ * Increment block_io of the SCSI target private data. The lookup and the
+ * increment are done under tgtdev_lock, which mpi3mr_target_destroy holds
+ * while it frees the private data.
+ *
+ * Return: The private data that was incremented, only to be handed back to
+ * mpi3mr_tgtdev_block_io_dec. NULL if the tgtdev has no SCSI target.
+ */
+struct mpi3mr_stgt_priv_data *mpi3mr_tgtdev_block_io_inc(struct mpi3mr_ioc *mrioc,
+							 struct mpi3mr_tgt_dev *tgtdev)
+{
+	struct mpi3mr_stgt_priv_data *stgt_priv = NULL;
+	unsigned long flags;
+
+	spin_lock_irqsave(&mrioc->tgtdev_lock, flags);
+	if (tgtdev->starget && tgtdev->starget->hostdata) {
+		stgt_priv = tgtdev->starget->hostdata;
+		atomic_inc(&stgt_priv->block_io);
+	}
+	spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
+	return stgt_priv;
+}
+
+/**
+ * mpi3mr_tgtdev_block_io_dec - Undo mpi3mr_tgtdev_block_io_inc
+ * @mrioc: Adapter instance reference
+ * @tgtdev: Target device reference, held by the caller
+ * @stgt_priv: Value returned by mpi3mr_tgtdev_block_io_inc
+ *
+ * Decrement block_io only if the SCSI target still has the same private
+ * data. If the target was removed meanwhile the private data is gone
+ * and there is nothing to undo.
+ */
+void mpi3mr_tgtdev_block_io_dec(struct mpi3mr_ioc *mrioc,
+				struct mpi3mr_tgt_dev *tgtdev,
+				struct mpi3mr_stgt_priv_data *stgt_priv)
+{
+	unsigned long flags;
+
+	if (!stgt_priv)
+		return;
+
+	spin_lock_irqsave(&mrioc->tgtdev_lock, flags);
+	if (tgtdev->starget && tgtdev->starget->hostdata == stgt_priv)
+		atomic_dec_if_positive(&stgt_priv->block_io);
+	spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
+}
+
 /**
  * __mpi3mr_get_tgtdev_by_perst_id -Get tgtdev from persist ID
  * @mrioc: Adapter instance reference
@@ -4953,10 +5005,9 @@ static void mpi3mr_target_destroy(struct scsi_target *starget)
 		mpi3mr_tgtdev_put(tgt_dev);
 		mpi3mr_tgtdev_put(tgt_dev);
 	}
-	spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
-
 	kfree(starget->hostdata);
 	starget->hostdata = NULL;
+	spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
 }
 
 /**
-- 
2.47.3

[PATCH 15/21] scsi: mpi3mr: Fix block_io accounting when a target is re-created during BSG

[Ranjan Kumar]

A BSG task management request raises block_io on the SCSI target private
data when it starts and lowers it when it completes. If the target is
removed and created again while the command waits, the new private data
can be allocated at the address of the old one. The address check then
passes and the command lowers block_io of the new target, which may be
held by another path.

Give each private data a sequence number and lower block_io only when the
number is the same as when the command started.

Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
 drivers/scsi/mpi3mr/mpi3mr.h     |  8 +++----
 drivers/scsi/mpi3mr/mpi3mr_app.c |  8 +++----
 drivers/scsi/mpi3mr/mpi3mr_os.c  | 40 ++++++++++++++++++++------------
 3 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
index da47100d895e..8738fc2ce7e7 100644
--- a/drivers/scsi/mpi3mr/mpi3mr.h
+++ b/drivers/scsi/mpi3mr/mpi3mr.h
@@ -847,6 +847,7 @@ struct mpi3mr_stgt_priv_data {
 	u16 perst_id;
 	u32 num_luns;
 	atomic_t block_io;
+	u64 seq;
 	u8 dev_removed;
 	u8 dev_removedelay;
 	u8 dev_type;
@@ -1533,11 +1534,10 @@ int mpi3mr_issue_tm(struct mpi3mr_ioc *mrioc, u8 tm_type,
 	u16 handle, uint lun, u16 htag, ulong timeout,
 	struct mpi3mr_drv_cmd *drv_cmd,
 	u8 *resp_code, struct scsi_cmnd *scmd);
-struct mpi3mr_stgt_priv_data *mpi3mr_tgtdev_block_io_inc(struct mpi3mr_ioc *mrioc,
-							 struct mpi3mr_tgt_dev *tgtdev);
+u64 mpi3mr_tgtdev_block_io_inc(struct mpi3mr_ioc *mrioc,
+			       struct mpi3mr_tgt_dev *tgtdev);
 void mpi3mr_tgtdev_block_io_dec(struct mpi3mr_ioc *mrioc,
-				struct mpi3mr_tgt_dev *tgtdev,
-				struct mpi3mr_stgt_priv_data *stgt_priv);
+				struct mpi3mr_tgt_dev *tgtdev, u64 seq);
 struct mpi3mr_tgt_dev *mpi3mr_get_tgtdev_by_handle(
 	struct mpi3mr_ioc *mrioc, u16 handle);
 void mpi3mr_pel_get_seqnum_complete(struct mpi3mr_ioc *mrioc,
diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
index 4df0dc81358b..30a01b02a9fc 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_app.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
@@ -2440,7 +2440,7 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 	u32 erbsz = MPI3MR_SENSE_BUF_SZ, tmplen;
 	u16 dev_handle;
 	struct mpi3mr_tgt_dev *tgtdev = NULL;
-	struct mpi3mr_stgt_priv_data *stgt_priv = NULL;
+	u64 block_seq = 0;
 	struct mpi3mr_bsg_in_reply_buf *bsg_reply_buf = NULL;
 	u32 din_size = 0, dout_size = 0;
 	u8 *din_buf = NULL, *dout_buf = NULL;
@@ -2792,7 +2792,7 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 	if (block_io) {
 		tgtdev = mpi3mr_get_tgtdev_by_handle(mrioc, dev_handle);
 		if (tgtdev)
-			stgt_priv = mpi3mr_tgtdev_block_io_inc(mrioc, tgtdev);
+			block_seq = mpi3mr_tgtdev_block_io_inc(mrioc, tgtdev);
 	}
 
 	mrioc->bsg_cmds.state = MPI3MR_CMD_PENDING;
@@ -2825,7 +2825,7 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 		    "%s: posting bsg request is failed\n", __func__);
 		rval = -EAGAIN;
 		if (tgtdev) {
-			mpi3mr_tgtdev_block_io_dec(mrioc, tgtdev, stgt_priv);
+			mpi3mr_tgtdev_block_io_dec(mrioc, tgtdev, block_seq);
 			mpi3mr_tgtdev_put(tgtdev);
 		}
 		goto out_unlock;
@@ -2833,7 +2833,7 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 	wait_for_completion_timeout(&mrioc->bsg_cmds.done,
 	    (karg->timeout * HZ));
 	if (tgtdev) {
-		mpi3mr_tgtdev_block_io_dec(mrioc, tgtdev, stgt_priv);
+		mpi3mr_tgtdev_block_io_dec(mrioc, tgtdev, block_seq);
 		mpi3mr_tgtdev_put(tgtdev);
 	}
 	if (!(mrioc->bsg_cmds.state & MPI3MR_CMD_COMPLETE)) {
diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
index e7833b24c509..67d41e4a0a0a 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_os.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
@@ -788,6 +788,8 @@ struct mpi3mr_tgt_dev *mpi3mr_get_tgtdev_by_handle(
 	return tgtdev;
 }
 
+static atomic64_t mpi3mr_stgt_seq = ATOMIC64_INIT(0);
+
 /**
  * mpi3mr_tgtdev_block_io_inc - Block I/O on the SCSI target of a tgtdev
  * @mrioc: Adapter instance reference
@@ -797,46 +799,52 @@ struct mpi3mr_tgt_dev *mpi3mr_get_tgtdev_by_handle(
  * increment are done under tgtdev_lock, which mpi3mr_target_destroy holds
  * while it frees the private data.
  *
- * Return: The private data that was incremented, only to be handed back to
- * mpi3mr_tgtdev_block_io_dec. NULL if the tgtdev has no SCSI target.
+ * Return: Sequence number of the private data that was incremented, to be
+ * handed back to mpi3mr_tgtdev_block_io_dec. 0 if the tgtdev has no SCSI
+ * target.
  */
-struct mpi3mr_stgt_priv_data *mpi3mr_tgtdev_block_io_inc(struct mpi3mr_ioc *mrioc,
-							 struct mpi3mr_tgt_dev *tgtdev)
+u64 mpi3mr_tgtdev_block_io_inc(struct mpi3mr_ioc *mrioc,
+			       struct mpi3mr_tgt_dev *tgtdev)
 {
-	struct mpi3mr_stgt_priv_data *stgt_priv = NULL;
+	struct mpi3mr_stgt_priv_data *stgt_priv;
 	unsigned long flags;
+	u64 seq = 0;
 
 	spin_lock_irqsave(&mrioc->tgtdev_lock, flags);
 	if (tgtdev->starget && tgtdev->starget->hostdata) {
 		stgt_priv = tgtdev->starget->hostdata;
 		atomic_inc(&stgt_priv->block_io);
+		seq = stgt_priv->seq;
 	}
 	spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
-	return stgt_priv;
+	return seq;
 }
 
 /**
  * mpi3mr_tgtdev_block_io_dec - Undo mpi3mr_tgtdev_block_io_inc
  * @mrioc: Adapter instance reference
  * @tgtdev: Target device reference, held by the caller
- * @stgt_priv: Value returned by mpi3mr_tgtdev_block_io_inc
+ * @seq: Value returned by mpi3mr_tgtdev_block_io_inc
  *
- * Decrement block_io only if the SCSI target still has the same private
- * data. If the target was removed meanwhile the private data is gone
- * and there is nothing to undo.
+ * Decrement block_io only if the SCSI target still has the private data
+ * that was incremented. A private data allocated later at the same address
+ * has a different sequence number and is left alone.
  */
 void mpi3mr_tgtdev_block_io_dec(struct mpi3mr_ioc *mrioc,
-				struct mpi3mr_tgt_dev *tgtdev,
-				struct mpi3mr_stgt_priv_data *stgt_priv)
+				struct mpi3mr_tgt_dev *tgtdev, u64 seq)
 {
+	struct mpi3mr_stgt_priv_data *stgt_priv;
 	unsigned long flags;
 
-	if (!stgt_priv)
+	if (!seq)
 		return;
 
 	spin_lock_irqsave(&mrioc->tgtdev_lock, flags);
-	if (tgtdev->starget && tgtdev->starget->hostdata == stgt_priv)
-		atomic_dec_if_positive(&stgt_priv->block_io);
+	if (tgtdev->starget && tgtdev->starget->hostdata) {
+		stgt_priv = tgtdev->starget->hostdata;
+		if (stgt_priv->seq == seq)
+			atomic_dec_if_positive(&stgt_priv->block_io);
+	}
 	spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
 }
 
@@ -5139,6 +5147,8 @@ static int mpi3mr_target_alloc(struct scsi_target *starget)
 	if (!scsi_tgt_priv_data)
 		return -ENOMEM;
 
+	scsi_tgt_priv_data->seq = atomic64_inc_return(&mpi3mr_stgt_seq);
+
 	starget->hostdata = scsi_tgt_priv_data;
 
 	spin_lock_irqsave(&mrioc->tgtdev_lock, flags);
-- 
2.47.3

[PATCH 16/21] scsi: mpi3mr: Fix sense buffer use after free on BSG command completion

[Ranjan Kumar]

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

[PATCH 17/21] scsi: mpi3mr: Reject a too small reply size when enabling log data

[Ranjan Kumar]

The log data entry size is computed from the reply frame size that the
firmware reports. If that size is 20 the entry size becomes 0, the buffer
pointer is a zero size pointer, and the next log event or read uses it.
Smaller sizes wrap to a huge entry size.

Refuse to enable log data when the reply frame cannot hold the event
header.

Fixes: f5e6d5a34376 ("scsi: mpi3mr: Add support for driver commands")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260825210411.301535-1-chandrakanth.patil@broadcom.com?part=5
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
 drivers/scsi/mpi3mr/mpi3mr_app.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
index fb54bd8093da..bf2c8c413a80 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_app.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
@@ -1311,6 +1311,9 @@ static long mpi3mr_enable_logdata(struct mpi3mr_ioc *mrioc,
 	struct mpi3mr_logdata_enable logdata_enable;
 
 	if (!mrioc->logdata_buf) {
+		if (mrioc->reply_sz <=
+		    (sizeof(struct mpi3_event_notification_reply) - 4))
+			return -EINVAL;
 		mrioc->logdata_entry_sz =
 		    (mrioc->reply_sz - (sizeof(struct mpi3_event_notification_reply) - 4))
 		    + MPI3MR_BSG_LOGDATA_ENTRY_HEADER_SZ;
-- 
2.47.3

[PATCH 18/21] scsi: mpi3mr: Fix partly written and lost entries in the log data buffer

[Ranjan Kumar]

Log data events are saved from the event handlers while the get and enable
ioctls read and publish the same buffer, index and entry size, all without
a lock. A read can return an entry that is only partly written, and two
events handled at the same time can take the same index so one is lost.

Add a lock for the log data buffer and take it in the writer and in both
ioctls.

Fixes: f5e6d5a34376 ("scsi: mpi3mr: Add support for driver commands")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260825210411.301535-1-chandrakanth.patil@broadcom.com?part=5
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
 drivers/scsi/mpi3mr/mpi3mr.h     |  2 ++
 drivers/scsi/mpi3mr/mpi3mr_app.c | 43 +++++++++++++++++++++++---------
 drivers/scsi/mpi3mr/mpi3mr_fw.c  |  3 +++
 drivers/scsi/mpi3mr/mpi3mr_os.c  |  1 +
 4 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
index deb8a1eea917..c87c85adb680 100644
--- a/drivers/scsi/mpi3mr/mpi3mr.h
+++ b/drivers/scsi/mpi3mr/mpi3mr.h
@@ -1165,6 +1165,7 @@ struct scmd_priv {
  * @logdata_buf: Circular buffer to store log data entries
  * @logdata_buf_idx: Index of entry in buffer to store
  * @logdata_entry_sz: log data entry size
+ * @logdata_lock: Lock for the log data buffer, index and entry size
  * @adm_req_q_bar_writeq_lock: Admin request queue lock
  * @adm_reply_q_bar_writeq_lock: Admin reply queue lock
  * @pend_large_data_sz: Counter to track pending large data
@@ -1374,6 +1375,7 @@ struct mpi3mr_ioc {
 	u8 *logdata_buf;
 	u16 logdata_buf_idx;
 	u16 logdata_entry_sz;
+	spinlock_t logdata_lock;
 	spinlock_t adm_req_q_bar_writeq_lock;
 	spinlock_t adm_reply_q_bar_writeq_lock;
 
diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
index bf2c8c413a80..310ead2c644e 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_app.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
@@ -1309,20 +1309,27 @@ static long mpi3mr_enable_logdata(struct mpi3mr_ioc *mrioc,
 	struct bsg_job *job)
 {
 	struct mpi3mr_logdata_enable logdata_enable;
+	unsigned long flags;
+	u16 entry_sz;
+	u8 *buf;
 
 	if (!mrioc->logdata_buf) {
 		if (mrioc->reply_sz <=
 		    (sizeof(struct mpi3_event_notification_reply) - 4))
 			return -EINVAL;
-		mrioc->logdata_entry_sz =
+		entry_sz =
 		    (mrioc->reply_sz - (sizeof(struct mpi3_event_notification_reply) - 4))
 		    + MPI3MR_BSG_LOGDATA_ENTRY_HEADER_SZ;
-		mrioc->logdata_buf_idx = 0;
-		mrioc->logdata_buf = kcalloc(MPI3MR_BSG_LOGDATA_MAX_ENTRIES,
-		    mrioc->logdata_entry_sz, GFP_KERNEL);
-
-		if (!mrioc->logdata_buf)
+		buf = kcalloc(MPI3MR_BSG_LOGDATA_MAX_ENTRIES, entry_sz,
+			      GFP_KERNEL);
+		if (!buf)
 			return -ENOMEM;
+
+		spin_lock_irqsave(&mrioc->logdata_lock, flags);
+		mrioc->logdata_entry_sz = entry_sz;
+		mrioc->logdata_buf_idx = 0;
+		mrioc->logdata_buf = buf;
+		spin_unlock_irqrestore(&mrioc->logdata_lock, flags);
 	}
 
 	memset(&logdata_enable, 0, sizeof(logdata_enable));
@@ -1349,11 +1356,15 @@ static long mpi3mr_enable_logdata(struct mpi3mr_ioc *mrioc,
 static long mpi3mr_get_logdata(struct mpi3mr_ioc *mrioc,
 	struct bsg_job *job)
 {
-	u16 entry_sz = mrioc->logdata_entry_sz;
+	unsigned long flags;
+	u16 entry_sz;
 	u32 num_entries, sz;
+	long rval = -EINVAL;
 
+	spin_lock_irqsave(&mrioc->logdata_lock, flags);
+	entry_sz = mrioc->logdata_entry_sz;
 	if ((!mrioc->logdata_buf) || (job->request_payload.payload_len < entry_sz))
-		return -EINVAL;
+		goto out;
 
 	num_entries = job->request_payload.payload_len / entry_sz;
 	if (num_entries > MPI3MR_BSG_LOGDATA_MAX_ENTRIES)
@@ -1364,9 +1375,11 @@ static long mpi3mr_get_logdata(struct mpi3mr_ioc *mrioc,
 		sg_copy_from_buffer(job->request_payload.sg_list,
 				    job->request_payload.sg_cnt,
 				    mrioc->logdata_buf, sz);
-		return 0;
+		rval = 0;
 	}
-	return -EINVAL;
+out:
+	spin_unlock_irqrestore(&mrioc->logdata_lock, flags);
+	return rval;
 }
 
 /**
@@ -3023,13 +3036,18 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 void mpi3mr_app_save_logdata_th(struct mpi3mr_ioc *mrioc, char *event_data,
 	u16 event_data_size)
 {
-	u32 index = mrioc->logdata_buf_idx;
+	u32 index;
 	size_t entry_payload_len, sz;
 	struct mpi3mr_logdata_entry *entry;
+	unsigned long flags;
 
-	if (!(mrioc->logdata_buf))
+	spin_lock_irqsave(&mrioc->logdata_lock, flags);
+	if (!(mrioc->logdata_buf)) {
+		spin_unlock_irqrestore(&mrioc->logdata_lock, flags);
 		return;
+	}
 
+	index = mrioc->logdata_buf_idx;
 	entry = (struct mpi3mr_logdata_entry *)
 		(mrioc->logdata_buf + (index * mrioc->logdata_entry_sz));
 	entry->valid_entry = 1;
@@ -3042,6 +3060,7 @@ void mpi3mr_app_save_logdata_th(struct mpi3mr_ioc *mrioc, char *event_data,
 	memcpy(entry->data, event_data, sz);
 	mrioc->logdata_buf_idx =
 		((++index) % MPI3MR_BSG_LOGDATA_MAX_ENTRIES);
+	spin_unlock_irqrestore(&mrioc->logdata_lock, flags);
 	atomic64_inc(&event_counter);
 }
 
diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
index 6d43112aebb7..637cf575e6bb 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
@@ -5029,6 +5029,7 @@ void mpi3mr_memset_buffers(struct mpi3mr_ioc *mrioc)
 void mpi3mr_free_mem(struct mpi3mr_ioc *mrioc)
 {
 	u16 i, j;
+	unsigned long flags;
 	struct mpi3mr_intr_info *intr_info;
 	struct diag_buffer_desc *diag_buffer;
 
@@ -5197,8 +5198,10 @@ void mpi3mr_free_mem(struct mpi3mr_ioc *mrioc)
 	kfree(mrioc->throttle_groups);
 	mrioc->throttle_groups = NULL;
 
+	spin_lock_irqsave(&mrioc->logdata_lock, flags);
 	kfree(mrioc->logdata_buf);
 	mrioc->logdata_buf = NULL;
+	spin_unlock_irqrestore(&mrioc->logdata_lock, flags);
 
 }
 
diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
index ed9e83a4119c..7a7ef199d3cd 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_os.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
@@ -5717,6 +5717,7 @@ mpi3mr_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	spin_lock_init(&mrioc->adm_reply_q_bar_writeq_lock);
 	spin_lock_init(&mrioc->sas_node_lock);
 	spin_lock_init(&mrioc->trigger_lock);
+	spin_lock_init(&mrioc->logdata_lock);
 
 	INIT_LIST_HEAD(&mrioc->fwevt_list);
 	INIT_LIST_HEAD(&mrioc->tgtdev_list);
-- 
2.47.3

[PATCH 19/21] scsi: mpi3mr: Fix byte order handling in the NVMe passthrough commands

[Ranjan Kumar]

The NVMe passthrough code reads the command dword that selects PRP or SGL
and applies the SGE modifier to the addresses without converting between
CPU and little endian byte order. On a big endian host the wrong data
format is chosen and the modifier bits end up in the wrong place of the
address.

Convert the command dword when reading it, and keep the modifier mask and
value in little endian form like the address fields they are applied to.

Fixes: 7dbd0dd8cde3 ("scsi: mpi3mr: Add support for NVMe passthrough")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260825210411.301535-1-chandrakanth.patil@broadcom.com?part=2
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
 drivers/scsi/mpi3mr/mpi3mr_app.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
index 310ead2c644e..2dc6fad7a37b 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_app.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
@@ -1969,7 +1969,7 @@ static unsigned int mpi3mr_get_nvme_data_fmt(
 {
 	u8 format = 0;
 
-	format = ((nvme_encap_request->command[0] & 0xc000) >> 14);
+	format = ((le32_to_cpu(nvme_encap_request->command[0]) & 0xc000) >> 14);
 	return format;
 
 }
@@ -1998,10 +1998,10 @@ static int mpi3mr_build_nvme_sgl(struct mpi3mr_ioc *mrioc,
 	u16 available_sges = 0, i;
 	u32 sge_element_size = sizeof(struct mpi3mr_nvme_pt_sge);
 	struct mpi3mr_buf_map *drv_buf_iter = drv_bufs;
-	u64 sgemod_mask = ((u64)((mrioc->facts.sge_mod_mask) <<
-			    mrioc->facts.sge_mod_shift) << 32);
-	u64 sgemod_val = ((u64)(mrioc->facts.sge_mod_value) <<
-			  mrioc->facts.sge_mod_shift) << 32;
+	__le64 sgemod_mask = cpu_to_le64(((u64)((mrioc->facts.sge_mod_mask) <<
+			    mrioc->facts.sge_mod_shift) << 32));
+	__le64 sgemod_val = cpu_to_le64(((u64)(mrioc->facts.sge_mod_value) <<
+			  mrioc->facts.sge_mod_shift) << 32);
 	u32 size;
 
 	nvme_sgl = (struct mpi3mr_nvme_pt_sge *)
@@ -2097,10 +2097,10 @@ static int mpi3mr_build_nvme_prp(struct mpi3mr_ioc *mrioc,
 	size_t length = 0, desc_len;
 	u8 count;
 	struct mpi3mr_buf_map *drv_buf_iter = drv_bufs;
-	u64 sgemod_mask = ((u64)((mrioc->facts.sge_mod_mask) <<
-			    mrioc->facts.sge_mod_shift) << 32);
-	u64 sgemod_val = ((u64)(mrioc->facts.sge_mod_value) <<
-			  mrioc->facts.sge_mod_shift) << 32;
+	__le64 sgemod_mask = cpu_to_le64(((u64)((mrioc->facts.sge_mod_mask) <<
+			    mrioc->facts.sge_mod_shift) << 32));
+	__le64 sgemod_val = cpu_to_le64(((u64)(mrioc->facts.sge_mod_value) <<
+			  mrioc->facts.sge_mod_shift) << 32);
 	u16 dev_handle = le16_to_cpu(nvme_encap_request->dev_handle);
 	struct mpi3mr_tgt_dev *tgtdev;
 	u16 desc_count = 0;
-- 
2.47.3

[PATCH 20/21] scsi: mpi3mr: Fix byte order of the BSG status reply

[Ranjan Kumar]

The status reply returned for a BSG MPT command stores the IOC status and
log info in little endian fields without converting them from CPU byte
order, so a big endian host returns swapped values to the application.

Convert both values to little endian.

Fixes: 506bc1a0d6ba ("scsi: mpi3mr: Add support for MPT commands")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260825210411.301535-1-chandrakanth.patil@broadcom.com?part=2
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
 drivers/scsi/mpi3mr/mpi3mr_app.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
index 2dc6fad7a37b..e38208241707 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_app.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
@@ -2938,8 +2938,10 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 				MPI3MR_BSG_MPI_REPLY_BUFTYPE_STATUS;
 			status_desc = (struct mpi3_status_reply_descriptor *)
 			    bsg_reply_buf->reply_buf;
-			status_desc->ioc_status = mrioc->bsg_cmds.ioc_status;
-			status_desc->ioc_log_info = mrioc->bsg_cmds.ioc_loginfo;
+			status_desc->ioc_status =
+			    cpu_to_le16(mrioc->bsg_cmds.ioc_status);
+			status_desc->ioc_log_info =
+			    cpu_to_le32(mrioc->bsg_cmds.ioc_loginfo);
 		}
 		tmplen = min(drv_buf_iter->kern_buf_len,
 			drv_buf_iter->bsg_buf_len);
-- 
2.47.3

[PATCH 21/21] scsi: mpi3mr: Use the correct conversion for the diag buffer post request

[Ranjan Kumar]

The diag buffer post request is filled in with le64_to_cpu and le32_to_cpu
where cpu_to_le64 and cpu_to_le32 are meant. Both swap bytes the same way,
so the result does not change, but sparse reports the wrong direction.

Use the helpers that match the direction.

Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
 drivers/scsi/mpi3mr/mpi3mr_app.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
index e38208241707..792c34ab5bdf 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_app.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
@@ -266,8 +266,8 @@ int mpi3mr_issue_diag_buf_post(struct mpi3mr_ioc *mrioc,
 	diag_buf_post_req.host_tag = cpu_to_le16(MPI3MR_HOSTTAG_INITCMDS);
 	diag_buf_post_req.function = MPI3_FUNCTION_DIAG_BUFFER_POST;
 	diag_buf_post_req.type = diag_buffer->type;
-	diag_buf_post_req.address = le64_to_cpu(diag_buffer->dma_addr);
-	diag_buf_post_req.length = le32_to_cpu(diag_buffer->size);
+	diag_buf_post_req.address = cpu_to_le64(diag_buffer->dma_addr);
+	diag_buf_post_req.length = cpu_to_le32(diag_buffer->size);
 
 	if (diag_buffer->is_segmented)
 		diag_buf_post_req.msg_flags |= MPI3_DIAG_BUFFER_POST_MSGFLAGS_SEGMENTED;
-- 
2.47.3

Re: [PATCH 16/21] scsi: mpi3mr: Fix sense buffer use after free on BSG command completion

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] scsi: mpi3mr: missing kernel-doc for state_lock in struct mpi3mr_drv_cmd
--

commit c6a057ccd37372310fe900ce7c3174bca6fb48f5
Author: Ranjan Kumar <ranjan.kumar@broadcom.com>

scsi: mpi3mr: Fix sense buffer use after free on BSG command completion

This commit adds a spinlock (state_lock) to struct mpi3mr_drv_cmd to
synchronize access to command tracker state and sense buffer pointers
between the BSG ioctl path, the interrupt reply handler, and reset
flushing, and initializes the PEL command trackers in mpi3mr_probe().

> diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
> index 8738fc2ce7e77..deb8a1eea9177 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;

[Severity: Low]
This isn't a bug, but struct mpi3mr_drv_cmd in drivers/scsi/mpi3mr/mpi3mr.h
has a kernel-doc comment block right above it describing all of its members.
Should @state_lock be added to that kernel-doc comment as well so that
running scripts/kernel-doc or building with W=1 does not warn about an
undocumented struct member?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104120103.503871-1-ranjan.kumar@broadcom.com?part=16

Re: [PATCH 14/21] scsi: mpi3mr: Fix target private data use after free in BSG passthrough

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] scsi: mpi3mr: scsi_target UAF in mpi3mr_tgtdev_block_io_dec() for SAS targets
- [High] scsi: mpi3mr: leaked block_io in mpi3mr_tgtdev_block_io_dec() on LUN removal
--

commit 5bf2d57a06044ae754274ac7045b1778169812fc
Author: Ranjan Kumar <ranjan.kumar@broadcom.com>

scsi: mpi3mr: Fix target private data use after free in BSG passthrough

This commit introduces mpi3mr_tgtdev_block_io_inc() and
mpi3mr_tgtdev_block_io_dec() to update block_io on the SCSI target private
data under mrioc->tgtdev_lock while holding a reference to tgtdev across the
BSG command wait, and moves kfree(starget->hostdata) inside mrioc->tgtdev_lock
in mpi3mr_target_destroy().

> diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
> index 99928fcc5bd79..e7833b24c5097 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr_os.c
> +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
> @@ -788,6 +788,58 @@ struct mpi3mr_tgt_dev *mpi3mr_get_tgtdev_by_handle(
>  	return tgtdev;
>  }
>  
> +/**
> + * mpi3mr_tgtdev_block_io_inc - Block I/O on the SCSI target of a tgtdev
> + * @mrioc: Adapter instance reference
> + * @tgtdev: Target device reference, held by the caller
> + *
> + * Increment block_io of the SCSI target private data. The lookup and the
> + * increment are done under tgtdev_lock, which mpi3mr_target_destroy holds
> + * while it frees the private data.
> + *
> + * Return: The private data that was incremented, only to be handed back to
> + * mpi3mr_tgtdev_block_io_dec. NULL if the tgtdev has no SCSI target.
> + */
> +struct mpi3mr_stgt_priv_data *mpi3mr_tgtdev_block_io_inc(struct mpi3mr_ioc *mrioc,
> +							 struct mpi3mr_tgt_dev *tgtdev)
> +{
> +	struct mpi3mr_stgt_priv_data *stgt_priv = NULL;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&mrioc->tgtdev_lock, flags);
> +	if (tgtdev->starget && tgtdev->starget->hostdata) {
> +		stgt_priv = tgtdev->starget->hostdata;
> +		atomic_inc(&stgt_priv->block_io);
> +	}
> +	spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
> +	return stgt_priv;
> +}
> +
> +/**
> + * mpi3mr_tgtdev_block_io_dec - Undo mpi3mr_tgtdev_block_io_inc
> + * @mrioc: Adapter instance reference
> + * @tgtdev: Target device reference, held by the caller
> + * @stgt_priv: Value returned by mpi3mr_tgtdev_block_io_inc
> + *
> + * Decrement block_io only if the SCSI target still has the same private
> + * data. If the target was removed meanwhile the private data is gone
> + * and there is nothing to undo.
> + */
> +void mpi3mr_tgtdev_block_io_dec(struct mpi3mr_ioc *mrioc,
> +				struct mpi3mr_tgt_dev *tgtdev,
> +				struct mpi3mr_stgt_priv_data *stgt_priv)
> +{
> +	unsigned long flags;
> +
> +	if (!stgt_priv)
> +		return;
> +
> +	spin_lock_irqsave(&mrioc->tgtdev_lock, flags);
> +	if (tgtdev->starget && tgtdev->starget->hostdata == stgt_priv)
> +		atomic_dec_if_positive(&stgt_priv->block_io);
> +	spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
> +}

[Severity: High]
Can mpi3mr_tgtdev_block_io_dec() leak a block_io count and permanently wedge
I/O to the target when the last LUN is removed while a BSG task management
command is in flight?

In mpi3mr_sdev_destroy(), when the last LUN on a target is destroyed,
tgt_dev->starget is cleared even though starget and stgt_priv remain alive:

drivers/scsi/mpi3mr/mpi3mr_os.c:mpi3mr_sdev_destroy() {
	...
	if (tgt_dev && (!scsi_tgt_priv_data->num_luns))
		tgt_dev->starget = NULL;
	...
}

For example, during __scsi_scan_target(), probing LUN 0 can find peripheral
qualifier 1 or 3 and call __scsi_remove_device() on the temporary LUN 0
scsi_device before scanning the remaining LUNs, or the last LUN can be
removed while a concurrent scan holds starget->reap_ref.

If mpi3mr_bsg_process_mpt_cmds() increments stgt_priv->block_io via
mpi3mr_tgtdev_block_io_inc() before mpi3mr_sdev_destroy() sets
tgt_dev->starget = NULL, and then completes while tgt_dev->starget is NULL,
mpi3mr_tgtdev_block_io_dec() will see tgtdev->starget == NULL and skip
atomic_dec_if_positive(&stgt_priv->block_io) (a regression that remains even
after later commit 23e409e23e8d fixes the separate stgt_priv slab reuse).

When mpi3mr_sdev_init() subsequently initializes another LUN on the same
starget, it reattaches starget without resetting block_io:

drivers/scsi/mpi3mr/mpi3mr_os.c:mpi3mr_sdev_init() {
	...
	if (tgt_dev) {
		if (tgt_dev->starget == NULL)
			tgt_dev->starget = starget;
	...
}

Wouldn't that leave stgt_priv->block_io permanently elevated and cause
mpi3mr_qcmd() to reject all subsequent commands with
SCSI_MLQUEUE_DEVICE_BUSY?

[ ... ]

> @@ -4953,10 +5005,9 @@ static void mpi3mr_target_destroy(struct scsi_target *starget)
>  		mpi3mr_tgtdev_put(tgt_dev);
>  		mpi3mr_tgtdev_put(tgt_dev);
>  	}
> -	spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
> -
>  	kfree(starget->hostdata);
>  	starget->hostdata = NULL;
> +	spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
>  }

[Severity: High]
Could mpi3mr_tgtdev_block_io_dec() and mpi3mr_tgtdev_block_io_inc()
dereference a freed struct scsi_target via tgtdev->starget->hostdata for SAS
transport targets?

In mpi3mr_target_alloc(), SAS transport targets (where
mrioc->sas_transport_enabled && !starget->channel) have tgt_dev->starget =
starget assigned, but starget->id is assigned by scsi_transport_sas
(rphy->scsi_target_id) rather than tgt_dev->perst_id.

When a SAS device is removed in mpi3mr_remove_device_by_sas_address(), tgtdev
is removed from mrioc->tgtdev_list before mpi3mr_remove_tgtdev_from_host()
runs:

drivers/scsi/mpi3mr/mpi3mr_transport.c:mpi3mr_remove_device_by_sas_address() {
	...
	if (tgtdev) {
		if (!list_empty(&tgtdev->list)) {
			was_on_tgtdev_list = 1;
			if (tgtdev->state == MPI3MR_DEV_REMOVE_HS_STARTED) {
				list_del_init(&tgtdev->list);
				mpi3mr_tgtdev_put(tgtdev);
			}
		}
	}
	spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
	if (was_on_tgtdev_list) {
		if (tgtdev->host_exposed)
			mpi3mr_remove_tgtdev_from_host(mrioc, tgtdev);
	...
}

Because tgtdev is no longer on mrioc->tgtdev_list,
__mpi3mr_get_tgtdev_by_addr_and_rphy() in mpi3mr_sdev_destroy() returns NULL
and does not clear tgt_dev->starget (or if target scanning fails before adding
a LUN, mpi3mr_sdev_destroy() never runs).

Then, when scsi_target_destroy() calls mpi3mr_target_destroy(),
tgt_dev->starget is only cleared when tgt_dev->perst_id == starget->id:

drivers/scsi/mpi3mr/mpi3mr_os.c:mpi3mr_target_destroy() {
	...
	tgt_dev = __mpi3mr_get_tgtdev_from_tgtpriv(mrioc, scsi_tgt_priv_data);
	if (tgt_dev && (tgt_dev->starget == starget) &&
	    (tgt_dev->perst_id == starget->id))
		tgt_dev->starget = NULL;
	...
}

When tgt_dev->perst_id != starget->id, tgt_dev->starget is left pointing to
starget after scsi_target_destroy() drops the final device reference and frees
starget. While mpi3mr_bsg_process_mpt_cmds() holds its reference to tgtdev
across wait_for_completion_timeout(), won't evaluating
tgtdev->starget->hostdata in mpi3mr_tgtdev_block_io_dec() (or in
mpi3mr_tgtdev_block_io_inc() after a failed target scan) read from the freed
struct scsi_target?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104120103.503871-1-ranjan.kumar@broadcom.com?part=14

Re: [PATCH 15/21] scsi: mpi3mr: Fix block_io accounting when a target is re-created during BSG

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] scsi: mpi3mr: leaked block_io count in mpi3mr_tgtdev_block_io_dec()
- [Low] scsi: mpi3mr: undocumented seq member in struct mpi3mr_stgt_priv_data
--

commit 23e409e23e8d123d95d4c65a026b00b68a047101
Author: Ranjan Kumar <ranjan.kumar@broadcom.com>

scsi: mpi3mr: Fix block_io accounting when a target is re-created during BSG

This commit introduces a 64-bit sequence number on struct
mpi3mr_stgt_priv_data so that mpi3mr_tgtdev_block_io_dec() only decrements
block_io if the target private data matches the instance that was
incremented by mpi3mr_tgtdev_block_io_inc() when the BSG task management
command started.

> diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
> index da47100d895e7..8738fc2ce7e77 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr.h
> +++ b/drivers/scsi/mpi3mr/mpi3mr.h
> @@ -847,6 +847,7 @@ struct mpi3mr_stgt_priv_data {
>  	u16 perst_id;
>  	u32 num_luns;
>  	atomic_t block_io;
> +	u64 seq;

[Severity: Low]
This isn't a bug, but struct mpi3mr_stgt_priv_data has a kernel-doc comment
block describing all of its members. Should a @seq entry be added there as
well so W=1 builds and scripts/kernel-doc do not warn about an undocumented
struct member?

[ ... ]

> diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
> index e7833b24c5097..67d41e4a0a0ab 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr_os.c
> +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
> @@ -788,6 +788,8 @@ struct mpi3mr_tgt_dev *mpi3mr_get_tgtdev_by_handle(
>  	return tgtdev;
>  }
>  
> +static atomic64_t mpi3mr_stgt_seq = ATOMIC64_INIT(0);
> +
>  /**
>   * mpi3mr_tgtdev_block_io_inc - Block I/O on the SCSI target of a tgtdev
>   * @mrioc: Adapter instance reference
> @@ -797,46 +799,52 @@ struct mpi3mr_tgt_dev *mpi3mr_get_tgtdev_by_handle(
>   * increment are done under tgtdev_lock, which mpi3mr_target_destroy holds
>   * while it frees the private data.
>   *
> - * Return: The private data that was incremented, only to be handed back to
> - * mpi3mr_tgtdev_block_io_dec. NULL if the tgtdev has no SCSI target.
> + * Return: Sequence number of the private data that was incremented, to be
> + * handed back to mpi3mr_tgtdev_block_io_dec. 0 if the tgtdev has no SCSI
> + * target.
>   */
> -struct mpi3mr_stgt_priv_data *mpi3mr_tgtdev_block_io_inc(struct mpi3mr_ioc *mrioc,
> -							 struct mpi3mr_tgt_dev *tgtdev)
> +u64 mpi3mr_tgtdev_block_io_inc(struct mpi3mr_ioc *mrioc,
> +			       struct mpi3mr_tgt_dev *tgtdev)
>  {
> -	struct mpi3mr_stgt_priv_data *stgt_priv = NULL;
> +	struct mpi3mr_stgt_priv_data *stgt_priv;
>  	unsigned long flags;
> +	u64 seq = 0;
>  
>  	spin_lock_irqsave(&mrioc->tgtdev_lock, flags);
>  	if (tgtdev->starget && tgtdev->starget->hostdata) {
>  		stgt_priv = tgtdev->starget->hostdata;
>  		atomic_inc(&stgt_priv->block_io);
> +		seq = stgt_priv->seq;
>  	}
>  	spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
> -	return stgt_priv;
> +	return seq;
>  }
>  
>  /**
>   * mpi3mr_tgtdev_block_io_dec - Undo mpi3mr_tgtdev_block_io_inc
>   * @mrioc: Adapter instance reference
>   * @tgtdev: Target device reference, held by the caller
> - * @stgt_priv: Value returned by mpi3mr_tgtdev_block_io_inc
> + * @seq: Value returned by mpi3mr_tgtdev_block_io_inc
>   *
> - * Decrement block_io only if the SCSI target still has the same private
> - * data. If the target was removed meanwhile the private data is gone
> - * and there is nothing to undo.
> + * Decrement block_io only if the SCSI target still has the private data
> + * that was incremented. A private data allocated later at the same address
> + * has a different sequence number and is left alone.
>   */
>  void mpi3mr_tgtdev_block_io_dec(struct mpi3mr_ioc *mrioc,
> -				struct mpi3mr_tgt_dev *tgtdev,
> -				struct mpi3mr_stgt_priv_data *stgt_priv)
> +				struct mpi3mr_tgt_dev *tgtdev, u64 seq)
>  {
> +	struct mpi3mr_stgt_priv_data *stgt_priv;
>  	unsigned long flags;
>  
> -	if (!stgt_priv)
> +	if (!seq)
>  		return;
>  
>  	spin_lock_irqsave(&mrioc->tgtdev_lock, flags);
> -	if (tgtdev->starget && tgtdev->starget->hostdata == stgt_priv)
> -		atomic_dec_if_positive(&stgt_priv->block_io);
> +	if (tgtdev->starget && tgtdev->starget->hostdata) {
> +		stgt_priv = tgtdev->starget->hostdata;
> +		if (stgt_priv->seq == seq)
> +			atomic_dec_if_positive(&stgt_priv->block_io);
> +	}
>  	spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
>  }

[Severity: Medium]
Can stgt_priv->block_io remain permanently elevated on a live scsi_target if
a BSG task management request completes while scsi_tgt_priv_data->num_luns
is temporarily zero?

In mpi3mr_sdev_destroy(), tgt_dev->starget is cleared whenever num_luns
reaches zero, even though the scsi_target and its hostdata (stgt_priv)
remain alive while starget->reap_ref is held:

drivers/scsi/mpi3mr/mpi3mr_os.c:mpi3mr_sdev_destroy() {
	...
	scsi_tgt_priv_data->num_luns--;
	...
	if (tgt_dev && (!scsi_tgt_priv_data->num_luns))
		tgt_dev->starget = NULL;
	...
}

And when another LUN is initialized on that same scsi_target,
mpi3mr_sdev_init() sets tgt_dev->starget back to starget:

drivers/scsi/mpi3mr/mpi3mr_os.c:mpi3mr_sdev_init() {
	...
	if (tgt_dev) {
		if (tgt_dev->starget == NULL)
			tgt_dev->starget = starget;
	...
}

This happens during __scsi_scan_target() (which holds starget->reap_ref)
when probing LUN 0 in scsi_probe_and_add_lun() returns
SCSI_SCAN_TARGET_PRESENT (such as when LUN 0 has peripheral qualifier 3) and
calls __scsi_remove_device() -> mpi3mr_sdev_destroy() before
scsi_report_lun_scan() or scsi_sequential_lun_scan() adds the remaining LUNs
on the same starget. It can also happen if the last LUN on a target is
removed via scsi_remove_device() while a concurrent __scsi_add_device() has
already acquired starget->reap_ref via scsi_alloc_target() and is waiting on
shost->scan_mutex.

If mpi3mr_bsg_process_mpt_cmds() calls mpi3mr_tgtdev_block_io_inc() while
LUN 0 exists, and then calls mpi3mr_tgtdev_block_io_dec() during the window
where tgt_dev->starget is NULL, mpi3mr_tgtdev_block_io_dec() sees
tgtdev->starget == NULL and skips the decrement. Once mpi3mr_sdev_init()
re-attaches tgt_dev->starget to that same starget, wouldn't
stgt_priv->block_io stay elevated and cause mpi3mr_qcmd() to return
SCSI_MLQUEUE_DEVICE_BUSY indefinitely for all commands to that target?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104120103.503871-1-ranjan.kumar@broadcom.com?part=15