On Tue, Jun 02, 2026 at 10:28:11AM +0000, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) > to consider: > > Pre-existing issues: > - [High] Missing error check for xa_store() leads to resource leak and > potential kernel oops on allocation failure. > -- > > commit 853688f50dc4ca56dd74698a3ecfdfeaf857e716 > Author: Shradha Gupta <[email protected]> > > net: mana: Optimize irq affinity for low vcpu configs > > This patch optimizes irq affinity in the mana driver for low vCPU > environments. When the number of allocated irqs is greater than the vCPU > count, it ensures that queue irqs are spread across all available vCPUs > to prevent softirq handling imbalances. > > > diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c > > b/drivers/net/ethernet/microsoft/mana/gdma_main.c > > --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c > > +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c > [ ... ] > > @@ -1767,13 +1784,42 @@ static int mana_gd_setup_dyn_irqs(struct pci_dev > > *pdev, int nvec) > > * first CPU sibling group since they are already affinitized to HWC IRQ > > */ > > cpus_read_lock(); > > - if (gc->num_msix_usable <= num_online_cpus()) > > - skip_first_cpu = true; > > + if (gc->num_msix_usable <= num_online_cpus()) { > > + err = irq_setup(irqs, nvec, gc->numa_node, true); > > + if (err) { > > + cpus_read_unlock(); > > + goto free_irq; > > [Severity: High] > This isn't a bug introduced by this patch, but looking at the error handling > path for goto free_irq, is there a potential resource leak if xa_store() > fails earlier in the function? > > When setting up the irqs in mana_gd_setup_dyn_irqs(), we call xa_store() > with GFP_KERNEL: > > xa_store(&gc->irq_contexts, i, gic, GFP_KERNEL); > > However, the return value is not checked. If this allocation fails, the gic > pointer is not saved. If a subsequent operation like irq_setup() fails and > we jump to the free_irq label, the cleanup loop attempts to retrieve it: > > gic = xa_load(&gc->irq_contexts, i); > if (WARN_ON(!gic)) > continue; > > Because the store failed, xa_load() will return NULL, causing the loop > to continue without calling free_irq() or kfree(gic). Could this result > in an un-freed irq handler that might trigger a panic later if the > module is unloaded and the hardware raises an interrupt? > > > + } > > + } else { > [ ... ] > > -- > Sashiko AI review ยท > https://sashiko.dev/#/patchset/[email protected]?part=1
Since this isn't a bug introduced by this patch, I will fix it in a seperate patch. Will submit the fix for this bug by next week. Thanks, Shradha.
