[PATCH 0/2] ata: bound the concurrent positioning ranges count from the device

[PATCH 0/2] ata: bound the concurrent positioning ranges count from the device

[Bryam Vargas via B4 Relay]

The concurrent positioning ranges log (0x47) reports its number of range
descriptors in the first byte, up to 255. Two paths walk that count at a
fixed 32-byte stride into a buffer whose size does not depend on it, with no
bound:

  1/2  ata_dev_config_cpr() reads nr_cpr=buf[0] descriptors out of a buffer
       sized from the (independent) GPL-directory log length. A small log
       with buf[0]=255 reads up to 7704 bytes past the slab; those bytes then
       reach the initiator via VPD B9h. Regression of c745dfc541e7.

  2/2  ata_scsiop_inq_b9() writes cpr_log->nr_cpr descriptors into the fixed
       2048-byte ata_scsi_rbuf. More than 62 ranges overflows it by up to
       6168 bytes.

Both counts come from the device, so the trigger is a malicious or faulty
drive (or an emulated one); 2/2 is then reachable by any INQUIRY VPD B9h,
including unprivileged SG_IO. drivers/scsi/sd.c:sd_read_cpr() derives the
count from the validated page length instead; these two paths don't.

They need separate bounds. With a large but self-consistent log
(buf_len=8704, buf[0]=255) the 1/2 clamp leaves nr_cpr=255: the read stays in
bounds but the 2/2 emit into the 2048-byte buffer still overflows. An
alternative for 2/2 is to grow ata_scsi_rbuf to the full page size
(64 + 256*32); I bounded the emitted count instead, since that buffer is
shared by every INQUIRY actor. Happy to switch if you'd rather size it.

A-B summary -- count vs buffer, both paths, unpatched vs patched, identical
on -m64 and -m32:

  path   nr_cpr  buf_len  unpatched        patched
  read   14      512      safe (boundary)  safe
  read   15      512      OOB read         safe
  read   255     512      OOB read         safe
  read   255     8704     safe             safe
  write  62      -        safe (boundary)  safe
  write  63      -        OOB write        safe
  write  255     8704     OOB write        safe   <- the cross-path case above

Early-return guards (major version < 11, absent log, zero count, NULL
cpr_log) and the unreachable buf_len < 64 underflow edge were checked too;
the 1/2 clamp is written to stay safe there. Reproducer modules (out-of-tree
KASAN sink-mirror + a userspace ASan twin, -m64/-m32) available on request.

---
Bryam Vargas (2):
      ata: libata-core: Clamp the concurrent positioning ranges count
      ata: libata-scsi: Bound the VPD B9h ranges to the response buffer

 drivers/ata/libata-core.c | 11 +++++++++++
 drivers/ata/libata-scsi.c | 12 +++++++++---
 2 files changed, 20 insertions(+), 3 deletions(-)
---
base-commit: 4549871118cf616eecdd2d939f78e3b9e1dddc48
change-id: 20260619-b4-disp-6200c44e-d6abc27892e4

Best regards,
-- 
bryamzxz <hexlabsecurity@proton.me>

[PATCH 1/2] ata: libata-core: Clamp the concurrent positioning ranges count

[Bryam Vargas via B4 Relay]

From: Bryam Vargas <hexlabsecurity@proton.me>

ata_dev_config_cpr() sizes the log buffer from the length reported in
the GPL directory but takes the number of range descriptors from buf[0],
which the device reports independently. A device advertising a small log
but a large count makes the descriptor loop read past the buffer: a
one-sector log with buf[0] = 255 reaches up to 7704 bytes beyond the
512-byte allocation, a slab out-of-bounds read whose contents are then
handed to the initiator through INQUIRY VPD page B9h.

Clamp the descriptor count to what the allocated buffer holds, as
sd_read_cpr() already does on the SCSI side.

Fixes: c745dfc541e7 ("libata: fix reading concurrent positioning ranges log")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
 drivers/ata/libata-core.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 3d0027ec33c2..e8d708f0810e 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2832,6 +2832,17 @@ static void ata_dev_config_cpr(struct ata_device *dev)
 	if (!nr_cpr)
 		goto out;
 
+	/*
+	 * The log size is reported in the GPL directory independently of the
+	 * number of range descriptors in buf[0]. Clamp the count to what the
+	 * allocated buffer holds so the loop below cannot read past it.
+	 */
+	if (buf_len < 64 + (size_t)nr_cpr * 32) {
+		nr_cpr = buf_len > 64 ? (buf_len - 64) / 32 : 0;
+		if (!nr_cpr)
+			goto out;
+	}
+
 	cpr_log = kzalloc_flex(*cpr_log, cpr, nr_cpr);
 	if (!cpr_log)
 		goto out;

-- 
2.43.0

Re: [PATCH 1/2] ata: libata-core: Clamp the concurrent positioning ranges count

[Damien Le Moal]

On 6/20/26 11:36, Bryam Vargas via B4 Relay wrote:
> From: Bryam Vargas <hexlabsecurity@proton.me>
> 
> ata_dev_config_cpr() sizes the log buffer from the length reported in
> the GPL directory but takes the number of range descriptors from buf[0],
> which the device reports independently. A device advertising a small log
> but a large count makes the descriptor loop read past the buffer: a
> one-sector log with buf[0] = 255 reaches up to 7704 bytes beyond the
> 512-byte allocation, a slab out-of-bounds read whose contents are then
> handed to the initiator through INQUIRY VPD page B9h.
> 
> Clamp the descriptor count to what the allocated buffer holds, as
> sd_read_cpr() already does on the SCSI side.
> 
> Fixes: c745dfc541e7 ("libata: fix reading concurrent positioning ranges log")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> ---
>  drivers/ata/libata-core.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 3d0027ec33c2..e8d708f0810e 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -2832,6 +2832,17 @@ static void ata_dev_config_cpr(struct ata_device *dev)
>  	if (!nr_cpr)
>  		goto out;
>  
> +	/*
> +	 * The log size is reported in the GPL directory independently of the
> +	 * number of range descriptors in buf[0]. Clamp the count to what the
> +	 * allocated buffer holds so the loop below cannot read past it.
> +	 */
> +	if (buf_len < 64 + (size_t)nr_cpr * 32) {
> +		nr_cpr = buf_len > 64 ? (buf_len - 64) / 32 : 0;
> +		if (!nr_cpr)
> +			goto out;
> +	}

If the device gives an invalid number of ranges, we should not try to fix the
broken value and warn and ignore it entirely. So let's simplify this:

	if (buf_len < 64 + (size_t)nr_cpr * 32) {
		ata_dev_warn(dev,
			"Invalid number of concurrent positioning ranges\n");
		goto out;
	}


-- 
Damien Le Moal
Western Digital Research

[PATCH 2/2] ata: libata-scsi: Bound the VPD B9h ranges to the response buffer

[Bryam Vargas via B4 Relay]

From: Bryam Vargas <hexlabsecurity@proton.me>

ata_scsiop_inq_b9() writes one 32-byte range descriptor per
cpr_log->nr_cpr into the fixed 2048-byte ata_scsi_rbuf with no bound.
The count originates from the device (concurrent positioning ranges log
0x47, up to 255), so an INQUIRY VPD page B9h to a drive whose log
advertises more than 62 ranges overflows the static buffer by up to 6168
bytes: a global out-of-bounds write into adjacent kernel data.

Emit only as many descriptors as the response buffer can hold and size
the page length to match. The companion core fix bounds the stored count
against the device log, but a large self-consistent log still yields
nr_cpr up to 255, so the emitter needs its own bound.

Fixes: fe22e1c2f705 ("libata: support concurrent positioning ranges log")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
 drivers/ata/libata-scsi.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index d43207c6e467..d43306c5d375 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2355,18 +2355,24 @@ 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 i, nr_cpr;
 
 	if (!cpr_log) {
 		ata_scsi_set_invalid_field(dev, cmd, 2, 0xff);
 		return 0;
 	}
 
+	/*
+	 * The page is built in the fixed-size ata_scsi_rbuf, so the number of
+	 * range descriptors returned is bounded by that buffer's size.
+	 */
+	nr_cpr = min_t(int, cpr_log->nr_cpr, (ATA_SCSI_RBUF_SIZE - 64) / 32);
+
 	/* 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(64 + nr_cpr * 32 - 4, &rbuf[2]);
 
-	for (i = 0; i < cpr_log->nr_cpr; i++, desc += 32) {
+	for (i = 0; i < nr_cpr; i++, desc += 32) {
 		desc[0] = cpr_log->cpr[i].num;
 		desc[1] = cpr_log->cpr[i].num_storage_elements;
 		put_unaligned_be64(cpr_log->cpr[i].start_lba, &desc[8]);

-- 
2.43.0

Re: [PATCH 2/2] ata: libata-scsi: Bound the VPD B9h ranges to the response buffer

[Damien Le Moal]

On 6/20/26 11:36, Bryam Vargas via B4 Relay wrote:
> From: Bryam Vargas <hexlabsecurity@proton.me>
> 
> ata_scsiop_inq_b9() writes one 32-byte range descriptor per
> cpr_log->nr_cpr into the fixed 2048-byte ata_scsi_rbuf with no bound.
> The count originates from the device (concurrent positioning ranges log
> 0x47, up to 255), so an INQUIRY VPD page B9h to a drive whose log
> advertises more than 62 ranges overflows the static buffer by up to 6168
> bytes: a global out-of-bounds write into adjacent kernel data.
> 
> Emit only as many descriptors as the response buffer can hold and size
> the page length to match. The companion core fix bounds the stored count
> against the device log, but a large self-consistent log still yields
> nr_cpr up to 255, so the emitter needs its own bound.
> 
> Fixes: fe22e1c2f705 ("libata: support concurrent positioning ranges log")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> ---
>  drivers/ata/libata-scsi.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index d43207c6e467..d43306c5d375 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -2355,18 +2355,24 @@ 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 i, nr_cpr;
>  
>  	if (!cpr_log) {
>  		ata_scsi_set_invalid_field(dev, cmd, 2, 0xff);
>  		return 0;
>  	}
>  
> +	/*
> +	 * The page is built in the fixed-size ata_scsi_rbuf, so the number of
> +	 * range descriptors returned is bounded by that buffer's size.
> +	 */
> +	nr_cpr = min_t(int, cpr_log->nr_cpr, (ATA_SCSI_RBUF_SIZE - 64) / 32);
> +

I do not think this is the right place to fix this. Rather, we should define the
maximum number of ranges we can support given a 2K rbuf, that is, (2048 - 64) /
32 = 62, and check on probe that we are not actually exceeding that value when
scanning the disk. If we do, warn and ignore CPR.

62 ranges would mean 62 actuators... We are light years away from seeing drives
with that many actuators (we are lucky if we get 2, so...).

So I think we can drop this patch and simply reinforce the checks on the number
of cpr in patch 1. Or make this additional patch another fix if you prefer.
Either way is OK with me.

>  	/* 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(64 + nr_cpr * 32 - 4, &rbuf[2]);
>  
> -	for (i = 0; i < cpr_log->nr_cpr; i++, desc += 32) {
> +	for (i = 0; i < nr_cpr; i++, desc += 32) {
>  		desc[0] = cpr_log->cpr[i].num;
>  		desc[1] = cpr_log->cpr[i].num_storage_elements;
>  		put_unaligned_be64(cpr_log->cpr[i].start_lba, &desc[8]);
> 


-- 
Damien Le Moal
Western Digital Research