Re: [PATCH 6/7] s390/pci: add error cleanup in zpci_directed_irq_init
"Tobias Schumacher" <[email protected]>
| Newsgroups | dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-s390 |
|---|---|
| Message-ID | <[email protected]> |
On Wed Aug 19, 2026 at 11:02 AM CEST, sashiko-bot wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [High] The newly added error cleanup path in `zpci_directed_irq_init` introduces a DMA use-after-free vulnerability. > -- > > --- Patch [6]: [PATCH 6/7] s390/pci: add error cleanup in zpci_directed_irq_init --- > commit 5a0360870b410d13391fe09d0a8840ffaa9cb9be > Author: Tobias Schumacher <[email protected]> > > s390/pci: add error cleanup in zpci_directed_irq_init > > This commit adds an error cleanup path to zpci_directed_irq_init to release > airq_iv resources if per-CPU allocation fails, preventing a memory leak. > >> diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c >> index 1515d8d7460ea..1ddf6b3625a26 100644 >> --- a/arch/s390/pci/pci_irq.c >> +++ b/arch/s390/pci/pci_irq.c >> @@ -586,7 +586,7 @@ static int __init zpci_directed_irq_init(void) >> >> zpci_ibv = kzalloc_objs(*zpci_ibv, num_possible_cpus()); >> if (!zpci_ibv) >> - return -ENOMEM; >> + goto out_free_sbv; >> >> for_each_possible_cpu(cpu) { > > [ ... ] > >> @@ -599,13 +599,25 @@ static int __init zpci_directed_irq_init(void) >> AIRQ_IV_CACHELINE | >> (!cpu ? AIRQ_IV_ALLOC : 0), NULL); >> if (!zpci_ibv[cpu]) >> - return -ENOMEM; >> + goto out_free_ibv; >> } >> on_each_cpu(cpu_enable_directed_irq, NULL, 1); >> >> zpci_irq_chip.irq_set_affinity = zpci_set_irq_affinity; >> >> return 0; >> + >> +out_free_ibv: >> + for_each_possible_cpu(cpu) { >> + if (zpci_ibv[cpu]) >> + airq_iv_release(zpci_ibv[cpu]); >> + } >> + kfree(zpci_ibv); >> + zpci_ibv = NULL; >> +out_free_sbv: >> + airq_iv_release(zpci_sbv); > > [Severity: High] > Does this new error path introduce a potential DMA use-after-free? > > Just before the allocation of zpci_ibv, the hardware is configured with the > physical address of zpci_sbv->vector: > > arch/s390/pci/pci_irq.c:zpci_directed_irq_init() { > ... > iib.diib.disb_addr = virt_to_phys(zpci_sbv->vector); > zpci_set_irq_ctrl(SIC_IRQ_MODE_DIRECT, 0, &iib); > > zpci_ibv = kzalloc_objs(*zpci_ibv, num_possible_cpus()); > if (!zpci_ibv) > goto out_free_sbv; > ... > } > > If we jump to out_free_sbv and free zpci_sbv via airq_iv_release(zpci_sbv) > without resetting the hardware configuration first, could the adapter > corrupt memory by writing to the freed physical page during a spurious > interrupt? > > Would it make sense to either defer the zpci_set_irq_ctrl() hardware > configuration until all software allocations succeed, or unregister the > hardware in the out_free_sbv error path? zpci_directed_irq_init() is reached from zpci_irq_init(), which pci_base_init() calls before any device exists. A function only starts generating directed interrupts once it has been registered with zpci_set_directed_irq(), called from zpci_msi_prepare()->zpci_set_irq(). Tobias