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

[Niklas Cassel <cassel@kernel.org>]

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