Re: [PATCH] ata: libata-core: Ignore forced transfer modes the host does not support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ata: libata-core: early return bypasses valid fallback force configs
--

commit b2b8699db9f1c2c52ee37c5d556e07a3dde1ab2e
Author: Rand Deeb <rand.sec96@gmail.com>

ata: libata-core: Ignore forced transfer modes the host does not support

This commit adds a check to ignore forced transfer modes that the host
controller does not support. This prevents out-of-bounds reads in driver
timing tables when an unsupported mode is forced.

> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 92233fb25051a..324aaa81c8a30 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -458,6 +459,17 @@ static void ata_force_xfermask(struct ata_device *dev)
>  
>  		ata_unpack_xfermask(fe->param.xfer_mask,
>  				    &pio_mask, &mwdma_mask, &udma_mask);
> +
> +		/* Never force a transfer mode the host does not support */
> +		if ((pio_mask & ~ap->pio_mask) ||
> +		    (mwdma_mask & ~ap->mwdma_mask) ||
> +		    (udma_mask & ~ap->udma_mask)) {
> +			ata_dev_warn(dev,
> +				     "FORCE: %s not supported by the host, ignored\n",
> +				     fe->param.name);
> +			return;

[Severity: Medium]
When a specific override is rejected by this new host capability check in
ata_force_xfermask(), does using return prematurely abort the loop?

The libata.force parameter allows stacking multiple hierarchical overrides
(e.g., libata.force=pio4,1.0:pio5), which are evaluated backwards so the
most specific match wins.

By returning instead of using continue, the code appears to silently bypass
earlier valid fallback overrides like the port-wide pio4 in the example above.

This would cause the device to fall back to its auto-negotiated mask instead
of the valid user-specified fallback, which might cause issues if the user
was relying on the fallback constraint to disable broken DMA.

> +		}
> +
>  		if (udma_mask)
>  			dev->udma_mask = udma_mask;
>  		else if (mwdma_mask) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014125916.1975-1-rand.sec96@gmail.com?part=1