[PATCH] ata: libata-scsi: add out-of-bounds checks in ata_scsiop_inq_b9

[PATCH] ata: libata-scsi: add out-of-bounds checks in ata_scsiop_inq_b9

[Karuna Ramkumar]

The function ata_scsiop_inq_b9() currently attempts to write 64 +
nr_cpr * 32 number of bytes. If nr_cpr crosses 62, this can cause a
buffer overflow since rbuf is 2048 bytes (ATA_SCSI_RBUF_SIZE) in size.

Fix this by adding bounds check before writing to rbuf. If the total
bytes are greater than ATA_SCSI_RBUF_SIZE, then fail the command with
ABORTED_COMMAND error and exit the function.

Tested this flow by modifying nr_cpr and confirming that the function
exits early, avoiding performing an out-of-bounds write.

Fixes: fe22e1c2f705 ("libata: support concurrent positioning ranges log")
Link: https://lore.kernel.org/all/0E78BE5F-B4EE-460A-A6D5-F0CE47BFC7DF@kernel.org/
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Karuna Ramkumar <rkaruna@google.com>
---
 drivers/ata/libata-scsi.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 5868526301a2..6f54e23a586c 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2361,15 +2361,22 @@ static unsigned int ata_scsiop_inq_b9(struct ata_device *dev,
 	struct ata_cpr_log *cpr_log = dev->cpr_log;
 	u8 *desc = &rbuf[64];
 	int i;
+	int page_len;
 
 	if (!cpr_log) {
 		ata_scsi_set_invalid_field(dev, cmd, 2, 0xff);
 		return 0;
 	}
 
+	page_len = 64 + (int)cpr_log->nr_cpr * 32;
+	if (page_len > ATA_SCSI_RBUF_SIZE) {
+		ata_scsi_set_sense(dev, cmd, ABORTED_COMMAND, 0, 0);
+		return 0;
+	}
+
 	/* SCSI Concurrent Positioning Ranges VPD page: SBC-5 rev 1 or later */
 	rbuf[1] = 0xb9;
-	put_unaligned_be16(64 + (int)cpr_log->nr_cpr * 32 - 4, &rbuf[2]);
+	put_unaligned_be16(page_len - 4, &rbuf[2]);
 
 	for (i = 0; i < cpr_log->nr_cpr; i++, desc += 32) {
 		desc[0] = cpr_log->cpr[i].num;
-- 
2.55.0.795.g602f6c329a-goog

Re: [PATCH] ata: libata-scsi: add out-of-bounds checks in ata_scsiop_inq_b9

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The newly added bounds check to prevent `page_len > ATA_SCSI_RBUF_SIZE` is mathematically dead code.
--

--- Patch [1]: [PATCH] ata: libata-scsi: add out-of-bounds checks in ata_scsiop_inq_b9 ---
commit bacad4b494e8c54b7f847f50ed2b764d2bdf3fde
Author: Karuna Ramkumar <rkaruna@google.com>
ata: libata-scsi: add out-of-bounds checks in ata_scsiop_inq_b9

This commit adds a bounds check to ata_scsiop_inq_b9() to ensure the
calculated page length does not exceed the SCSI request buffer size. If the
calculated size is larger, the command is aborted to prevent a buffer
overflow.

Link: https://lore.kernel.org/all/0E78BE5F-B4EE-460A-A6D5-F0CE47BFC7DF@kernel.org/

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
[ ... ]
> @@ -2361,15 +2361,22 @@ static unsigned int ata_scsiop_inq_b9(struct ata_device *dev,
[ ... ]
>  	if (!cpr_log) {
>  		ata_scsi_set_invalid_field(dev, cmd, 2, 0xff);
>  		return 0;
>  	}
>  
> +	page_len = 64 + (int)cpr_log->nr_cpr * 32;
> +	if (page_len > ATA_SCSI_RBUF_SIZE) {
> +		ata_scsi_set_sense(dev, cmd, ABORTED_COMMAND, 0, 0);
> +		return 0;
> +	}

[Severity: Low]
Is this bounds check reachable during normal operation?

Looking at how cpr_log is populated in ata_read_cpr_log(), nr_cpr is
already explicitly validated against ATA_DEV_MAX_CPR. Since ATA_DEV_MAX_CPR
is mathematically derived from ATA_SCSI_RBUF_SIZE, a device reporting a
value exceeding this limit has its log discarded, leaving dev->cpr_log as
NULL.

Because ata_scsiop_inq_b9() immediately returns if cpr_log is NULL right
above this snippet, it seems page_len can never evaluate to a value greater
than ATA_SCSI_RBUF_SIZE. Does this make the new check mathematically dead
code under real inputs?

> +
>  	/* SCSI Concurrent Positioning Ranges VPD page: SBC-5 rev 1 or later */
>  	rbuf[1] = 0xb9;
> -	put_unaligned_be16(64 + (int)cpr_log->nr_cpr * 32 - 4, &rbuf[2]);
> +	put_unaligned_be16(page_len - 4, &rbuf[2]);
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710193235.334782-1-rkaruna@google.com?part=1