[PATCH] scsi: mpi3mr: Fix integer overflow in reply frame size calculation

[PATCH] scsi: mpi3mr: Fix integer overflow in reply frame size calculation

[Milan P. Gandhi]

The driver reads reply_frame_size from firmware as a u16 value in dwords
and multiplies by 4 to convert to bytes. If the firmware (malicious or
buggy) provides a value greater than U16_MAX / 4, the multiplication
overflows the u16 result before assignment to mrioc->facts.reply_sz.

For example:
  reply_frame_size = 20000 dwords
  20000 * 4 = 80000 bytes (as int)
  Assigned to u16: 80000 & 0xFFFF = 14464 bytes (truncated)

This truncated value is then used to allocate DMA reply buffers via
kzalloc(). The firmware believes it can send 80000-byte replies, but
the driver only allocated 14464 bytes, causing a heap buffer overflow
when the firmware DMAs the full-sized reply.

Additionally, if firmware reports reply_frame_size == 0, facts.reply_sz
becomes 0, which can cause division-by-zero or zero-sized DMA pool
allocations downstream.

Fix by validating reply_frame_size in mpi3mr_issue_iocfacts() before
processing the facts data. Reject values that are zero or would overflow
u16 when converted to bytes.

An attacker with firmware control could exploit this to corrupt kernel
heap memory via DMA, potentially leading to privilege escalation.

Fixes: 824a156633df ("scsi: mpi3mr: Base driver code")
Cc: stable@vger.kernel.org
Signed-off-by: Milan P. Gandhi <mgandhi@redhat.com>
---
 drivers/scsi/mpi3mr/mpi3mr_fw.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
index f0d3cd398dd0..2b72218e79fb 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
@@ -3261,6 +3261,7 @@ static int mpi3mr_issue_iocfacts(struct mpi3mr_ioc *mrioc,
 	struct mpi3_ioc_facts_data *facts_data)
 {
 	struct mpi3_ioc_facts_request iocfacts_req;
+	u16 reply_frame_size;
 	void *data = NULL;
 	dma_addr_t data_dma;
 	u32 data_len = sizeof(*facts_data);
@@ -3318,6 +3319,19 @@ static int mpi3mr_issue_iocfacts(struct mpi3mr_ioc *mrioc,
 		goto out_unlock;
 	}
 	memcpy(facts_data, (u8 *)data, data_len);
+
+	reply_frame_size = le16_to_cpu(facts_data->reply_frame_size);
+	/*
+	 * Validate reply_frame_size to prevent integer overflow when
+	 * converting dwords to bytes (reply_frame_size * 4).
+	 */
+	if (!reply_frame_size || reply_frame_size > (U16_MAX / 4)) {
+		ioc_err(mrioc, "Invalid reply_frame_size %u from firmware\n",
+			reply_frame_size);
+		retval = -EINVAL;
+		goto out_unlock;
+	}
+
 	mpi3mr_process_factsdata(mrioc, facts_data);
 out_unlock:
 	mrioc->init_cmds.state = MPI3MR_CMD_NOTUSED;
-- 
2.56.0

Re: [PATCH] scsi: mpi3mr: Fix integer overflow in reply frame size calculation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] scsi: mpi3mr: missing minimum reply_frame_size check in mpi3mr_issue_iocfacts()
--

commit b19e0f1b924cd80d0da53959c0e35b88d0537a9a
Author: Milan P. Gandhi <mgandhi@redhat.com>

scsi: mpi3mr: Fix integer overflow in reply frame size calculation

This commit validates reply_frame_size in mpi3mr_issue_iocfacts() to prevent
a u16 integer overflow when multiplying by 4 to compute mrioc->facts.reply_sz,
and rejects a zero reply_frame_size.

> diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
> index f0d3cd398dd00..2b72218e79fb3 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
> +++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
> @@ -3261,6 +3261,7 @@ static int mpi3mr_issue_iocfacts(struct mpi3mr_ioc *mrioc,
>  	struct mpi3_ioc_facts_data *facts_data)
>  {
>  	struct mpi3_ioc_facts_request iocfacts_req;
> +	u16 reply_frame_size;
>  	void *data = NULL;
>  	dma_addr_t data_dma;
>  	u32 data_len = sizeof(*facts_data);
> @@ -3318,6 +3319,19 @@ static int mpi3mr_issue_iocfacts(struct mpi3mr_ioc *mrioc,
>  		goto out_unlock;
>  	}
>  	memcpy(facts_data, (u8 *)data, data_len);
> +
> +	reply_frame_size = le16_to_cpu(facts_data->reply_frame_size);
> +	/*
> +	 * Validate reply_frame_size to prevent integer overflow when
> +	 * converting dwords to bytes (reply_frame_size * 4).
> +	 */
> +	if (!reply_frame_size || reply_frame_size > (U16_MAX / 4)) {
> +		ioc_err(mrioc, "Invalid reply_frame_size %u from firmware\n",
> +			reply_frame_size);
> +		retval = -EINVAL;
> +		goto out_unlock;
> +	}

[Severity: Critical]
Is rejecting only reply_frame_size == 0 at the lower bound sufficient when
validating firmware-provided facts data?

If buggy or malicious firmware reports reply_frame_size between 1 and 15
dwords (setting mrioc->facts.reply_sz and mrioc->reply_sz to 4..60 bytes),
several downstream consumers fail:

1. In drivers/scsi/mpi3mr/mpi3mr_app.c:mpi3mr_enable_logdata(),
   mrioc->logdata_entry_sz (u16) is computed as:

	mrioc->logdata_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);

   Since sizeof(struct mpi3_event_notification_reply) is 28 and
   MPI3MR_BSG_LOGDATA_ENTRY_HEADER_SZ is 4, this evaluates to
   mrioc->reply_sz - 20. When reply_frame_size == 5 (mrioc->reply_sz == 20),
   mrioc->logdata_entry_sz becomes 0 and kcalloc() returns ZERO_SIZE_PTR,
   which then causes a division by zero in mpi3mr_get_logdata():

	entry_sz = mrioc->logdata_entry_sz;
	num_entries = job->request_payload.payload_len / entry_sz;

   and a ZERO_SIZE_PTR dereference in mpi3mr_app_save_logdata_th(). When
   reply_frame_size is 1..4 (mrioc->reply_sz is 4..16),
   mrioc->logdata_entry_sz underflows u16 to 65520..65532.

2. In drivers/scsi/mpi3mr/mpi3mr_app.c:mpi3mr_bsg_process_mpt_cmds(),
   bsg_reply_buf is allocated with sizeof(*bsg_reply_buf) + mrioc->reply_sz
   bytes. When MPI3MR_CMD_REPLY_VALID is not set, bsg_reply_buf->reply_buf
   is cast to struct mpi3_status_reply_descriptor * (8 bytes):

	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;

   When reply_frame_size == 1 (mrioc->reply_sz == 4), writing
   status_desc->ioc_log_info writes 4 bytes past the end of bsg_reply_buf.

3. In drivers/scsi/mpi3mr/mpi3mr_fw.c:mpi3mr_alloc_reply_sense_bufs(),
   internal command reply buffers (such as mrioc->transport_cmds.reply,
   mrioc->pel_cmds.reply, and mrioc->host_tm_cmds.reply) are allocated with
   kzalloc(mrioc->reply_sz, GFP_KERNEL), and mrioc->reply_buf slots are
   sized to mrioc->reply_sz bytes. However, downstream completion paths read
   fixed-size reply structures up to 64 bytes without checking
   mrioc->reply_sz:

   - drivers/scsi/mpi3mr/mpi3mr_transport.c:mpi3mr_post_transport_req()
     copies reply_sz (up to sizeof(struct mpi3_iounit_control_reply), 64
     bytes) from mrioc->transport_cmds.reply:

	if ((reply) && (mrioc->transport_cmds.state & MPI3MR_CMD_REPLY_VALID))
		memcpy((u8 *)reply, mrioc->transport_cmds.reply, reply_sz);

   - drivers/scsi/mpi3mr/mpi3mr_os.c:mpi3mr_process_op_reply_desc() reads
     struct mpi3_scsi_io_reply (56 bytes) from mrioc->reply_buf:

	sense_buf = mpi3mr_get_sensebuf_virt_addr(mrioc,
	    le64_to_cpu(scsi_reply->sense_data_buffer_address));

   - drivers/scsi/mpi3mr/mpi3mr_fw.c:mpi3mr_process_admin_reply_desc(),
     PEL completion handlers, and task management handlers dereference
     struct mpi3_default_reply (16 bytes), struct mpi3_pel_reply (28 bytes),
     and struct mpi3_scsi_task_mgmt_reply (28 bytes).

Should mpi3mr_issue_iocfacts() enforce a minimum reply_frame_size of at least
16 dwords (64 bytes, covering struct mpi3_iounit_control_reply and all other
fixed reply structures) instead of only checking for zero?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103123316.223917-1-mgandhi@redhat.com?part=1

Re: [PATCH] scsi: mpi3mr: Fix integer overflow in reply frame size calculation

[Milan P. Gandhi]

On Tue, Nov 03, 2026 at 12:49:25PM +0000, sashiko-bot@kernel.org wrote:
> Should mpi3mr_issue_iocfacts() enforce a minimum reply_frame_size of at least
> 16 dwords (64 bytes, covering struct mpi3_iounit_control_reply and all other
> fixed reply structures) instead of only checking for zero?

Thanks for the detailed analysis. You are completely right.
A reply_frame_size below 16 dwords (64 bytes) breaks several downstream
consumers that assume a minimum reply frame size—most notably
mrioc->logdata_entry_sz underflowing or causing division-by-zero,
out-of-bounds writes in bsg_reply_buf, and truncating fixed completion
replies like struct mpi3_iounit_control_reply (64 bytes).

I will prepare and submit a v2 enforcing a minimum bound of 16 dwords
(64 bytes) along with the upper bound overflow check.

[PATCH v2] scsi: mpi3mr: Validate reply frame size bounds

[Milan P. Gandhi]

Firmware provides reply_frame_size in dwords, which the driver multiplies
by 4 to compute facts.reply_sz (u16). If the reported size exceeds
U16_MAX / 4, the multiplication overflows u16 before assignment, causing
under-allocated DMA reply buffers.

Furthermore, several paths assume a minimal reply frame size:
- logdata_entry_sz calculations underflow or hit zero when reply_sz <= 20
  bytes, leading to zero-size allocations and divide-by-zero crashes.
- Unsolicited status descriptors and fixed replies (such as struct
  mpi3_iounit_control_reply, 64 bytes) require at least 16 dwords.

Validate reply_frame_size in mpi3mr_issue_iocfacts() to ensure it is
within [16, U16_MAX / 4] dwords before processing facts data.

Fixes: 824a156633df ("scsi: mpi3mr: Base driver code")
Cc: stable@vger.kernel.org
Signed-off-by: Milan P. Gandhi <mgandhi@redhat.com>
---
v2:
 - Check lower bound (16 dwords) to avoid downstream underflows.
 - Add MPI3MR_MIN_REPLY_FRAME_SZ_DWORDS define.
 - Clarify commit message and error message bounds.

 drivers/scsi/mpi3mr/mpi3mr.h    |  1 +
 drivers/scsi/mpi3mr/mpi3mr_fw.c | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
index d6e16707fd97..e78383df619c 100644
--- a/drivers/scsi/mpi3mr/mpi3mr.h
+++ b/drivers/scsi/mpi3mr/mpi3mr.h
@@ -83,6 +83,7 @@ extern atomic64_t event_counter;
 #define MPI3MR_ADMIN_REPLY_Q_SIZE	(8 * MPI3MR_PAGE_SIZE_4K)
 #define MPI3MR_ADMIN_REQ_FRAME_SZ	128
 #define MPI3MR_ADMIN_REPLY_FRAME_SZ	16
+#define MPI3MR_MIN_REPLY_FRAME_SZ_DWORDS	16	/* 64 bytes */
 
 /* Operational queue management definitions */
 #define MPI3MR_OP_REQ_Q_QD		512
diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
index f0d3cd398dd0..60a92366e590 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
@@ -3261,6 +3261,7 @@ static int mpi3mr_issue_iocfacts(struct mpi3mr_ioc *mrioc,
 	struct mpi3_ioc_facts_data *facts_data)
 {
 	struct mpi3_ioc_facts_request iocfacts_req;
+	u16 reply_frame_size;
 	void *data = NULL;
 	dma_addr_t data_dma;
 	u32 data_len = sizeof(*facts_data);
@@ -3318,6 +3319,23 @@ static int mpi3mr_issue_iocfacts(struct mpi3mr_ioc *mrioc,
 		goto out_unlock;
 	}
 	memcpy(facts_data, (u8 *)data, data_len);
+
+	reply_frame_size = le16_to_cpu(facts_data->reply_frame_size);
+	/*
+	 * Validate reply_frame_size:
+	 * - Must be at least 16 dwords (64 bytes) to accommodate fixed reply
+	 *   structures and avoid underflow/divide-by-zero in logdata sizing.
+	 * - Must not exceed U16_MAX / 4 to prevent integer overflow.
+	 */
+	if (reply_frame_size < MPI3MR_MIN_REPLY_FRAME_SZ_DWORDS ||
+	    reply_frame_size > (U16_MAX / 4)) {
+		ioc_err(mrioc, "Invalid reply_frame_size %u (must be %u..%u dwords)\n",
+			reply_frame_size, MPI3MR_MIN_REPLY_FRAME_SZ_DWORDS,
+			(unsigned int)(U16_MAX / 4));
+		retval = -EINVAL;
+		goto out_unlock;
+	}
+
 	mpi3mr_process_factsdata(mrioc, facts_data);
 out_unlock:
 	mrioc->init_cmds.state = MPI3MR_CMD_NOTUSED;
-- 
2.56.0

Re: [PATCH v2] scsi: mpi3mr: Validate reply frame size bounds

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: mpi3mr: MPI3MR_MIN_REPLY_FRAME_SZ_DWORDS too small for device events
--

commit d0d39880e8d702685fdb1265ca4630cc2c3c1e34
Author: Milan P. Gandhi <mgandhi@redhat.com>

scsi: mpi3mr: Validate reply frame size bounds

This commit validates that the firmware-reported reply_frame_size in
mpi3mr_issue_iocfacts() falls within [16, U16_MAX / 4] dwords to prevent
integer overflow when computing facts.reply_sz and underflow when sizing
logdata entries.

> diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
> index d6e16707fd97c..e78383df619c3 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr.h
> +++ b/drivers/scsi/mpi3mr/mpi3mr.h
> @@ -83,6 +83,7 @@ extern atomic64_t event_counter;
>  #define MPI3MR_ADMIN_REPLY_Q_SIZE	(8 * MPI3MR_PAGE_SIZE_4K)
>  #define MPI3MR_ADMIN_REQ_FRAME_SZ	128
>  #define MPI3MR_ADMIN_REPLY_FRAME_SZ	16
> +#define MPI3MR_MIN_REPLY_FRAME_SZ_DWORDS	16	/* 64 bytes */
>  
>  /* Operational queue management definitions */
>  #define MPI3MR_OP_REQ_Q_QD		512
> diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
> index f0d3cd398dd00..60a92366e5904 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
> +++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
> @@ -3261,6 +3261,7 @@ static int mpi3mr_issue_iocfacts(struct mpi3mr_ioc *mrioc,
>  	struct mpi3_ioc_facts_data *facts_data)
>  {
>  	struct mpi3_ioc_facts_request iocfacts_req;
> +	u16 reply_frame_size;
>  	void *data = NULL;
>  	dma_addr_t data_dma;
>  	u32 data_len = sizeof(*facts_data);
> @@ -3318,6 +3319,23 @@ static int mpi3mr_issue_iocfacts(struct mpi3mr_ioc *mrioc,
>  		goto out_unlock;
>  	}
>  	memcpy(facts_data, (u8 *)data, data_len);
> +
> +	reply_frame_size = le16_to_cpu(facts_data->reply_frame_size);
> +	/*
> +	 * Validate reply_frame_size:
> +	 * - Must be at least 16 dwords (64 bytes) to accommodate fixed reply
> +	 *   structures and avoid underflow/divide-by-zero in logdata sizing.
> +	 * - Must not exceed U16_MAX / 4 to prevent integer overflow.
> +	 */
> +	if (reply_frame_size < MPI3MR_MIN_REPLY_FRAME_SZ_DWORDS ||
> +	    reply_frame_size > (U16_MAX / 4)) {
> +		ioc_err(mrioc, "Invalid reply_frame_size %u (must be %u..%u dwords)\n",
> +			reply_frame_size, MPI3MR_MIN_REPLY_FRAME_SZ_DWORDS,
> +			(unsigned int)(U16_MAX / 4));
> +		retval = -EINVAL;
> +		goto out_unlock;
> +	}

[Severity: High]
Is a minimum of 16 dwords (64 bytes) for MPI3MR_MIN_REPLY_FRAME_SZ_DWORDS
sufficient for event notification replies that carry struct
mpi3_device_page0?

Event notification replies (struct mpi3_event_notification_reply) are
delivered in the mrioc->reply_buf frames of size mrioc->reply_sz
(reply_frame_size * 4), where the event_data payload starts at byte offset
24 (6 dwords). For MPI3_EVENT_DEVICE_ADDED, the event_data payload is
struct mpi3_device_page0 (64 bytes / 16 dwords), which requires a reply
frame of at least 24 + 64 = 88 bytes (22 dwords).

When reply_frame_size is between 16 and 21 dwords (64 to 84 bytes),
mpi3mr_issue_iocfacts() accepts it, and an incoming MPI3_EVENT_DEVICE_ADDED
event in drivers/scsi/mpi3mr/mpi3mr_os.c:mpi3mr_os_handle_events() casts
event_reply->event_data to struct mpi3_device_page0:

	case MPI3_EVENT_DEVICE_ADDED:
	{
		struct mpi3_device_page0 *dev_pg0 =
		    (struct mpi3_device_page0 *)event_reply->event_data;
		if (mpi3mr_create_tgtdev(mrioc, dev_pg0))
			...

mpi3mr_create_tgtdev() -> mpi3mr_update_tgtdev() then reads dev_pg0->
device_specific fields up to offset 59 of dev_pg0 (offset 83 of
event_reply), reading past a 64..84 byte reply frame and past the end of
mrioc->reply_buf when using the last frame in the pool.

In addition, when mpi3mr_os_handle_events() allocates fwevt for the bottom
half, it clamps the event_data allocation size sz to avail_reply_room
(mrioc->reply_sz - 24, which is 40..48 bytes for 16..18 dwords):

	if (mrioc->reply_sz > offsetof(struct mpi3_event_notification_reply, event_data))
		avail_reply_room = mrioc->reply_sz -
		    offsetof(struct mpi3_event_notification_reply, event_data);
	else
		avail_reply_room = 0;
	sz = min_t(u16, event_reply->event_data_length * 4, avail_reply_room);
	fwevt = mpi3mr_alloc_fwevt(sz);

When drivers/scsi/mpi3mr/mpi3mr_os.c:mpi3mr_fwevt_bh() later processes
MPI3_EVENT_DEVICE_ADDED with persistent_id ==
MPI3_DEVICE0_PERSISTENTID_INVALID and sas_transport_enabled set for a
SAS/SATA device:

	dev_pg0 = (struct mpi3_device_page0 *)fwevt->event_data;
	perst_id = le16_to_cpu(dev_pg0->persistent_id);
	handle = le16_to_cpu(dev_pg0->dev_handle);
	if (perst_id != MPI3_DEVICE0_PERSISTENTID_INVALID)
		mpi3mr_report_tgtdev_to_host(mrioc, perst_id);
	else if (mrioc->sas_transport_enabled &&
	    (dev_pg0->device_form == MPI3_DEVICE_DEVFORM_SAS_SATA)) {
		sasinf = &dev_pg0->device_specific.sas_sata_format;
		dev_info = le16_to_cpu(sasinf->device_info);

can reading sasinf->device_info at offset 50..51 of fwevt->event_data read
out of bounds of the slab object when only 40..48 bytes were allocated?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105122401.266559-1-mgandhi@redhat.com?part=1

Re: [PATCH v2] scsi: mpi3mr: Validate reply frame size bounds

[Milan P. Gandhi]

On Thu, Nov 05, 2026 at 12:45:39PM +0000, sashiko-bot@kernel.org wrote:
> [Severity: High]
> Is a minimum of 16 dwords (64 bytes) for MPI3MR_MIN_REPLY_FRAME_SZ_DWORDS
> sufficient for event notification replies that carry struct
> mpi3_device_page0?

The intent of this patch is specifically to establish baseline firmware 
interface sanity in mpi3mr_issue_iocfacts()—preventing the integer overflow 
on reply_sz, accommodating standard fixed reply frames, and avoiding 
underflows in logdata sizing calculations.

Event notification payload validation touches a broader set of event types 
and handling paths, which would expand the scope of this targeted fix. 
That can be addressed in a separate follow-up patch.

Thanks,
Milan

Re: [PATCH v2] scsi: mpi3mr: Validate reply frame size bounds

[Martin K. Petersen (Oracle)]

On Thu, 05 Nov 2026 17:54:01 +0530, Milan P. Gandhi wrote:

> Firmware provides reply_frame_size in dwords, which the driver multiplies
> by 4 to compute facts.reply_sz (u16). If the reported size exceeds
> U16_MAX / 4, the multiplication overflows u16 before assignment, causing
> under-allocated DMA reply buffers.
> 
> Furthermore, several paths assume a minimal reply frame size:
> - logdata_entry_sz calculations underflow or hit zero when reply_sz <= 20
>   bytes, leading to zero-size allocations and divide-by-zero crashes.
> - Unsolicited status descriptors and fixed replies (such as struct
>   mpi3_iounit_control_reply, 64 bytes) require at least 16 dwords.
> 
> [...]

Applied to 7.4/scsi-fixes, thanks!

[1/1] scsi: mpi3mr: Validate reply frame size bounds
      https://git.kernel.org/mkp/scsi/c/e550f2b2b4aa

-- 
Martin K. Petersen