[PATCH 0/6] comedi: validate the IRQ numbers supplied by userspace

[Yogesh Gaur]

comedi boards are configured through the COMEDI_DEVCONFIG ioctl, which
lets userspace pick the interrupt line for ISA-style boards. Fourteen
drivers hand that value to request_irq(); eight bound it to the
interrupts the board can actually assert first, six do not.

On x86 an unbounded value can name an interrupt that belongs to the
IO-APIC GSI domain of a PCI device. request_irq() succeeds and
register_handler_proc() creates /proc/irq/<n>/<board>. When that PCI
device is later unbound, mp_unmap_irq() drops mp_chip_data->count to
zero and frees the descriptor from under the still-installed comedi
handler:

	remove_proc_entry: removing non-empty directory 'irq/20', leaking at least 'comedi_parport'
	WARNING: fs/proc/generic.c:747 at remove_proc_entry+0x4e7/0x610 fs/proc/generic.c:747
	Call Trace:
	 <TASK>
	 unregister_irq_proc+0x206/0x2a0 kernel/irq/proc.c:406
	 free_desc+0x89/0x330 kernel/irq/irqdesc.c:482
	 irq_free_descs+0x84/0xc0 kernel/irq/irqdesc.c:865
	 irq_domain_free_irqs+0x46a/0x5c0 kernel/irq/irqdomain.c:1917
	 mp_unmap_irq+0xf8/0x130 arch/x86/kernel/apic/io_apic.c:1061
	 acpi_unregister_gsi_ioapic+0x40/0x60 arch/x86/kernel/acpi/boot.c:722
	 acpi_pci_irq_disable+0x275/0x360 drivers/acpi/pci_irq.c:517
	 pci_disable_device+0x130/0x270 drivers/pci/pci.c:2206
	 pci_device_remove+0xb2/0x1d0 drivers/pci/pci-driver.c:512
	 device_release_driver_internal+0x44e/0x620 drivers/base/dd.c:1372
	 unbind_store+0xf8/0x110 drivers/base/bus.c:244
	 </TASK>

The leaked /proc entry the warning names is the mild part.
mp_chip_data->count tracks GSI mappings rather than request_irq() users,
and __setup_irq() takes no reference on the descriptor, so the irq_desc
is freed with the comedi irqaction still attached to it.

Restricting the value to the ISA range is enough to close this:
mp_chip_data->isa_irq is set only by alloc_isa_irq_from_domain(), and
mp_unmap_irq() returns early when it is set, so a legacy interrupt can
never be freed out from under a requester. It also costs nothing real,
since every affected driver is for an ISA or PC/104 board.

Each patch bounds one driver, following the check das16m1.c already has.
Where the driver documents which interrupts its board can assert, that
set is used; otherwise the bound is the plain 1-15 ISA range. As in
das16m1.c an out-of-range value is ignored rather than rejected, so the
board still attaches without interrupt support -- the same thing that
happens today when request_irq() fails.

syzbot found this through comedi_parport, fixed by patch 1. The other
five are the same bug reachable the same way; I have no reproducer for
those, they came out of auditing the callers.

I have none of this hardware, so the change is by inspection only.

Reported-by: syzbot+690d666eb12fca6e1e61@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=690d666eb12fca6e1e61

Yogesh Gaur (6):
  comedi: comedi_parport: validate the IRQ supplied by userspace
  comedi: ni_atmio16d: validate the IRQ supplied by userspace
  comedi: dt2814: validate the IRQ supplied by userspace
  comedi: dmm32at: validate the IRQ supplied by userspace
  comedi: pcmmio: validate the IRQ supplied by userspace
  comedi: pcmuio: validate the IRQs supplied by userspace

 drivers/comedi/drivers/comedi_parport.c | 3 ++-
 drivers/comedi/drivers/dmm32at.c        | 3 ++-
 drivers/comedi/drivers/dt2814.c         | 3 ++-
 drivers/comedi/drivers/ni_atmio16d.c    | 4 +++-
 drivers/comedi/drivers/pcmmio.c         | 3 ++-
 drivers/comedi/drivers/pcmuio.c         | 5 +++--
 6 files changed, 14 insertions(+), 7 deletions(-)

-- 
2.55.0.windows.5