[PATCH] scsi: mpt3sas: fix OOB SGE write at the request frame boundary

[PATCH] scsi: mpt3sas: fix OOB SGE write at the request frame boundary

[Weibin Liu]

_ctl_do_mpt_command() bounds the user-supplied data_sge_offset with
strict inequalities:

	if (karg.data_sge_offset * 4 > ioc->request_sz ||
	    karg.data_sge_offset > (UINT_MAX / 4)) {
		ret = -EINVAL;
		goto out;
	}

When data_sge_offset * 4 is exactly ioc->request_sz, both comparisons
pass. The SGE array pointer is then derived as

	psge = (void *)request + (karg.data_sge_offset * 4);

which points just past the end of the request frame. The per-function
SGE builders then write entries at psge; _base_build_sg(), for one,
emits a zero-length SGE there when data_out_sz and data_in_sz are both
zero. The write lands outside the request frame and corrupts adjacent
memory in the request DMA pool.

Use >= for the frame size comparison so that an offset placing the
start of the SGE array at or beyond the end of the request frame is
rejected with -EINVAL.

Fixes: f92363d12359 ("[SCSI] mpt3sas: add new driver supporting 12GB SAS")
Cc: stable@vger.kernel.org
Signed-off-by: Weibin Liu <liuwb@xiaopeng.com>
---
 drivers/scsi/mpt3sas/mpt3sas_ctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
index 8bb947004885..d8d3ee11ed52 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
@@ -739,7 +739,7 @@ _ctl_do_mpt_command(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command karg,
 	}
 
 	/* Check for overflow and wraparound */
-	if (karg.data_sge_offset * 4 > ioc->request_sz ||
+	if (karg.data_sge_offset * 4 >= ioc->request_sz ||
 	    karg.data_sge_offset > (UINT_MAX / 4)) {
 		ret = -EINVAL;
 		goto out;

---
base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
change-id: 20261015-mpt3sas-sge-offset-38426dc931a6

Best regards,
-- 
Weibin Liu <liuwb@xiaopeng.com>

Re: [PATCH] scsi: mpt3sas: fix OOB SGE write at the request frame boundary

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: mpt3sas: incomplete bounds check leaves OOB write unpatched
--

commit c6cf70bbba6355fabaab51bc07707d150970dad8
Author: Weibin Liu <liuwb@xiaopeng.com>

scsi: mpt3sas: fix OOB SGE write at the request frame boundary

_ctl_do_mpt_command() bounds the user-supplied data_sge_offset with
strict inequalities. When data_sge_offset * 4 is exactly ioc->request_sz,
both comparisons pass, allowing an OOB write. This commit changes the
comparison to >= to reject offsets that point exactly at or past the
end of the request frame.

> diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
> index 8bb947004885b..d8d3ee11ed520 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
> @@ -739,7 +739,7 @@ _ctl_do_mpt_command(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command karg,
>  	}
>  
>  	/* Check for overflow and wraparound */
> -	if (karg.data_sge_offset * 4 > ioc->request_sz ||
> +	if (karg.data_sge_offset * 4 >= ioc->request_sz ||
>  	    karg.data_sge_offset > (UINT_MAX / 4)) {

[Severity: High]
Does this bounds check leave an out-of-bounds write unpatched?

If a user issues an MPT3COMMAND ioctl with karg.data_sge_offset set such
that karg.data_sge_offset * 4 is slightly less than ioc->request_sz
(e.g., ioc->request_sz - 4), the updated validation still passes because
ioc->request_sz - 4 is not >= ioc->request_sz.

The SGE array pointer psge would then be set to point 4 bytes before the
end of the request frame. The SGE builder functions (ioc->build_sg, such
as _base_build_sg() or _base_build_zero_len_sge()) write a minimum of 8
to 32 bytes of scatter-gather elements to psge without further bounds
checking:

drivers/scsi/mpt3sas/mpt3sas_base.c:_base_build_sg() {
...
	if (!data_out_sz && !data_in_sz) {
		_base_build_zero_len_sge(ioc, psge);
		return;
	}
...
}

Since only 4 bytes remain in the frame, the SGE write overflows the request
buffer by at least 4 bytes. Because the request buffers are allocated
contiguously in a DMA pool array, does this overflow corrupt the adjacent
request frame?

>  		ret = -EINVAL;
>  		goto out;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-mpt3sas-sge-offset-v1-1-9b91c265f2c1@xiaopeng.com?part=1