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

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

[Rand Deeb]

The libata.force parameter can force a transfer mode (pio0-6, mwdma0-4,
udma0-7) for a port or device. ata_force_xfermask() applies it in
ata_do_set_mode() right after ata_dev_xfermask() has limited the device
masks to what the host supports. It replaces those masks with the forced
mode without checking the host masks, so a mode the controller does not
support is passed to the driver's ->set_piomode() or ->set_dmamode().

Several PATA drivers rely on the host masks to keep the mode number in
range when indexing their timing tables. ata_piix, pata_efar,
pata_it8213, pata_oldpiix, pata_radisys and pata_rdc declare ATA_PIO4
and ATA_MWDMA12_ONLY, index a five-entry PIO timing table with
pio_mode - XFER_PIO_0, and index a three-entry needed_pio[] table with
dma_mode - XFER_MW_DMA_0. Booting with libata.force=pio5, pio6, mwdma3
or mwdma4 makes them read past those tables, use the values read as
further indices into the timing table, and write the result to the
controller's timing registers.

Make ata_force_xfermask() ignore a forced transfer mode that is not in
the port's PIO, MWDMA or UDMA mask, and warn about it. Forcing a mode the
host supports still works as before, including a mode that
ata_dev_xfermask() removed for the device or cable.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 332673257056 ("libata: implement libata.force module parameter")
Signed-off-by: Rand Deeb <rand.sec96@gmail.com>
---
 drivers/ata/libata-core.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 92233fb25..324aaa81c 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -434,6 +434,7 @@ static void ata_force_link_limits(struct ata_link *link)
  */
 static void ata_force_xfermask(struct ata_device *dev)
 {
+	struct ata_port *ap = dev->link->ap;
 	int devno = dev->link->pmp + dev->devno;
 	int alt_devno = devno;
 	int i;
@@ -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;
+		}
+
 		if (udma_mask)
 			dev->udma_mask = udma_mask;
 		else if (mwdma_mask) {

base-commit: 4056b3c9d3c92ea45955641dbafeb9b960e146ef
-- 
2.39.5

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

[Damien Le Moal]

On 2026/10/14 14:59, Rand Deeb wrote:
> The libata.force parameter can force a transfer mode (pio0-6, mwdma0-4,
> udma0-7) for a port or device. ata_force_xfermask() applies it in
> ata_do_set_mode() right after ata_dev_xfermask() has limited the device
> masks to what the host supports. It replaces those masks with the forced
> mode without checking the host masks, so a mode the controller does not
> support is passed to the driver's ->set_piomode() or ->set_dmamode().
> 
> Several PATA drivers rely on the host masks to keep the mode number in
> range when indexing their timing tables. ata_piix, pata_efar,
> pata_it8213, pata_oldpiix, pata_radisys and pata_rdc declare ATA_PIO4
> and ATA_MWDMA12_ONLY, index a five-entry PIO timing table with
> pio_mode - XFER_PIO_0, and index a three-entry needed_pio[] table with
> dma_mode - XFER_MW_DMA_0. Booting with libata.force=pio5, pio6, mwdma3
> or mwdma4 makes them read past those tables, use the values read as
> further indices into the timing table, and write the result to the
> controller's timing registers.
> 
> Make ata_force_xfermask() ignore a forced transfer mode that is not in
> the port's PIO, MWDMA or UDMA mask, and warn about it. Forcing a mode the
> host supports still works as before, including a mode that
> ata_dev_xfermask() removed for the device or cable.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: 332673257056 ("libata: implement libata.force module parameter")
> Signed-off-by: Rand Deeb <rand.sec96@gmail.com>
> ---
>  drivers/ata/libata-core.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 92233fb25..324aaa81c 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -434,6 +434,7 @@ static void ata_force_link_limits(struct ata_link *link)
>   */
>  static void ata_force_xfermask(struct ata_device *dev)
>  {
> +	struct ata_port *ap = dev->link->ap;
>  	int devno = dev->link->pmp + dev->devno;
>  	int alt_devno = devno;
>  	int i;
> @@ -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;
> +		}

I would rewrite this as:

		/* Make sure that the port supports the specified xfer modes. */
		if (!(pio_mask & ap->pio_mask) &&
		    !(mwdma_mask & ap->mwdma_mask) &&
		    !(udma_mask & ap->udma_mask)) {
			ata_dev_warn(dev,
				"FORCE: ignoring unsupported xfer_mask %s\n",
				fe->param.name);
			continue;
		}

as that is easier to understand. The "continue" here should address Sashiko comment.

> +
>  		if (udma_mask)
>  			dev->udma_mask = udma_mask;
>  		else if (mwdma_mask) {
> 
> base-commit: 4056b3c9d3c92ea45955641dbafeb9b960e146ef


-- 
Damien Le Moal
Western Digital Research