Re: [PATCH] ata: ata_generic: Do not bind to devices that are not IDE controllers

[Niklas Cassel <cassel@kernel.org>]

On Tue, Aug 18, 2026 at 11:42:05AM +0200, Niklas Cassel wrote:
> syzbot force-bound ata_generic to 0000:00:03.0 on a QEMU arm64 virt
> machine. That device is a virtio-blk-pci device holding the root file
> system, and it reports PCI class 0x010000, i.e. PCI_CLASS_STORAGE_SCSI.
>
> QEMU gives the virtio-blk-pci device a legacy virtio I/O BAR0 and a 4 KiB
> MSI-X BAR1, so both resources are non-empty, the port is not discarded,
> and the device control register ends up in the middle of the MSI-X table.
>
> The emulated device rejects the byte write, and arm64 reports the
> resulting bus error as a fatal synchronous external abort:
>
>   Internal error: synchronous external abort: 0000000096000050 [#1]  SMP
>   pc : ata_sff_freeze+0x7c/0x90 drivers/ata/libata-sff.c:1606
>   Call trace:
>    ata_sff_freeze+0x7c/0x90
>    ata_eh_freeze_port+0x34/0x5c
>    ata_host_start+0x13c/0x228
>    ata_pci_sff_activate_host+0x50/0x340
>    ata_pci_init_one+0x19c/0x1d8
>    ata_pci_bmdma_init_one+0x14/0x20
>    ata_generic_init_one+0xc4/0x1ac
>    local_pci_probe+0x40/0xa8
>    pci_device_probe+0xd8/0x288
>    really_probe+0xbc/0x2bc
>    device_driver_attach+0x48/0xb4
>    bind_store+0x7c/0xd8
>
> Refuse devices which neither report the IDE class nor appear in our ID
> table. Table entries keep binding as before, because some of the listed
> controllers cannot be assumed to report the IDE class. A controller which
> needs ata_generic but does not report the IDE class should get an ID table
> entry, which is what the table is for.
>
> Binding a driver to unrelated hardware requires root and is what
> driver_override is meant to do, so this does not fix a privilege boundary.
>
> This change only stops ata_generic from binding to a PCI device which it
> has no reason to believe to be an IDE controller.
>
> Reported-by: syzbot+891c7b195b408052e519@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/linux-ide/6a82bc54.10853dc7.22f513.001b.GAE@google.com/
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  drivers/ata/ata_generic.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/ata/ata_generic.c b/drivers/ata/ata_generic.c
> index e70b6c089cf1..f57db4480ad3 100644
> --- a/drivers/ata/ata_generic.c
> +++ b/drivers/ata/ata_generic.c
> @@ -150,6 +150,9 @@ static int is_intel_ider(struct pci_dev *dev)
>	return 1;
>  }
>
> +/* Forward declaration for the pci_match_id() call in ata_generic_init_one() */
> +static const struct pci_device_id ata_generic[];
> +
>  /**
>   *	ata_generic_init_one		-	attach generic IDE
>   *	@dev: PCI device found
> @@ -172,6 +175,19 @@ static int ata_generic_init_one(struct pci_dev *dev, const struct pci_device_id
>	};
>	const struct ata_port_info *ppi[] = { &info, NULL };
>
> +	/*
> +	 * A device matched through driver_override or through an ID added
> +	 * with new_id does not come from our ID table, so pci_match_device()
> +	 * hands us a synthetic ID with no driver_data and none of the checks
> +	 * below apply. Probing maps BAR0 and BAR1 as the ATA command and
> +	 * control blocks and writes to the device control register, so only
> +	 * continue for devices which report the IDE class or which we list
> +	 * ourselves.
> +	 */
> +	if (!pci_match_id(ata_generic, dev) &&
> +	    (dev->class >> 8) != PCI_CLASS_STORAGE_IDE)
> +		return -ENODEV;
> +
>	/* Don't use the generic entry unless instructed to do so */
>	if ((id->driver_data & ATA_GEN_CLASS_MATCH) && all_generic_ide == 0)
>		return -ENODEV;
> --
> 2.55.0
>

Another suggested patch is to something like:


--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -2115,6 +2115,14 @@ static bool ata_resources_present(struct pci_dev *pdev, int port)
	/* Check the PCI resources for this channel are enabled */
	port *= 2;
	for (i = 0; i < 2; i++) {
+               /*
+                * The command and control blocks of a native mode PCI IDE
+                * controller are I/O resources, as are the legacy mode
+                * resources which the PCI core fabricates for compatibility
+                * mode controllers.  A memory resource is not a taskfile.
+                */
+               if (!(pci_resource_flags(pdev, port + i) & IORESOURCE_IO))
+                       return false;

Since IDE controllers always have BAR0 and BAR1 as IORESOURCE_IO and not
IORESOURCE_MEM.

Verified that:
ata_generic, ata_piix, pata_rdc and the ~40 other pata_* PCI drivers,
plus sata_nv, sata_sis, sata_uli, sata_via (vt6420/vt8251) all use I/O BARs
for the register blocks.


That would still allow someone to use driver_override to bind a PCI device
that has class code != PCI_CLASS_STORAGE_IDE, but would still fail e.g.
a virtio-pci device, which has the BARs of type MEM and not type I/O.


Kind regards,
Niklas