Re: [PATCH v2] ata: libata-core: Reject an invalid concurrent positioning ranges count

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch strictly rejects the entire Concurrent Positioning Ranges (CPR) log if the range count exceeds 62 or the returned log size, permanently disabling the feature for otherwise compliant devices.
--

commit fb7353b474c07b1173b746d017d47feac25f57b1
Author: Bryam Vargas <hexlabsecurity@proton.me>

ata: libata-core: Reject an invalid concurrent positioning ranges count

This commit adds bounds checking for the concurrent positioning ranges (CPR)
log count. It rejects the log if the count exceeds 62 (the maximum capacity
of the VPD B9h buffer) or if the count exceeds the returned log size.

> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 3d0027ec33c2a..017a16be158b6 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -2832,6 +2832,18 @@ static void ata_dev_config_cpr(struct ata_device *dev)
>  	if (!nr_cpr)
>  		goto out;
>  
> +	/*
> +	 * The count is reported independently of the log size and is also
> +	 * emitted into the fixed 2048-byte VPD B9h buffer, which holds at most
> +	 * (2048 - 64) / 32 = 62 descriptors. Reject a count that exceeds that
> +	 * or does not fit the log the device returned.
> +	 */
> +	if (nr_cpr > 62 || buf_len < 64 + (size_t)nr_cpr * 32) {
> +		ata_dev_warn(dev,
> +			     "Invalid number of concurrent positioning ranges\n");
> +		goto out;
> +	}

[Severity: Medium]
Does this code completely disable the CPR feature for spec-compliant devices
that report more than 62 ranges?

The ATA specification allows up to 255 concurrent positioning ranges, and the
62-descriptor limit seems to be an internal Linux SCSI translation limit based
on the fixed 2048-byte ata_scsi_rbuf size. By jumping to out, we discard the
log entirely rather than capping it to fit internal limits.

Also, the commit message states "Capping the stored count keeps the emitter in
bounds", but the code appears to reject the log outright instead of capping
the count. Could we instead dynamically cap nr_cpr to 62 here, so that devices
with more ranges still get partial functionality rather than having the feature
disabled entirely?

> +
>  	cpr_log = kzalloc_flex(*cpr_log, cpr, nr_cpr);
>  	if (!cpr_log)
>  		goto out;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622-b4-disp-5734c6f2-v2-1-53083c2df3c6@proton.me?part=1