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

On 8/18/26 19:07, Niklas Cassel wrote:
> On Tue, Aug 18, 2026 at 06:52:45PM +0900, Damien Le Moal wrote:
>> On 8/18/26 18:42, 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: [email protected]
>>> Closes: https://lore.kernel.org/linux-ide/[email protected]/
>>> Signed-off-by: Niklas Cassel <[email protected]>
>>
>> Looks sensible to me, and very surprising that this problem was not cought before...
>>
>> Reviewed-by: Damien Le Moal <[email protected]>
> 
> I mean, syzbot using driver_override like this is kind of a stupid test.
> 
> If you try to use the generic ATA driver for a PCI device that is something
> completely different, e.g. an Ethernet controller... you kind of get what
> you asked for :)
> 
> But it is such a small patch, so I think it makes sense, even if all it
> does it to stop us from getting more silly reports like this.
> 
> 
> Tell me if you prefer the other patch, which checks BAR type instead.
> BAR type I/O is very rare, since it is old x86 legacy.
> (But such a patch would still allow someone to bind a PCI device with
> BARs marked as type I/O, even if that PCI device does not have storage
> class IDE.)

I definitely prefer the patch as is, checking that the storage class is IDE,
instead of doing guesswork with the BAR types.

> 
> 
> I think the main argument for the patch in $subject... is:
> if your PCI device is not class IDE... just add an entry to the match table.
> (I.e. we don't care that driver_override will not work with a device that
> does not have class IDE... If you really want the driver to work with such
> a device, you will have to add an explicit entry to the match table.)
> 
> 
> Kind regards,
> Niklas


-- 
Damien Le Moal
Western Digital Research