[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.