Subject: [EXTERNAL] Re: [net-next,v3,4/6] net: mana: Use GIC functions to
allocate global EQs

This is an AI-generated review of your patch. The human sending this email has
considered the AI review valid, or at least plausible.

For local reproduction steps see
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fnetdev-
ai.bots.linux.dev%2Fai-
local.html&data=05%7C02%7Clongli%40microsoft.com%7Cb43e1b10bd564c760
a4d08de7eb17c82%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6390
87497958656662%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWU
sIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%
7C0%7C%7C%7C&sdata=r63AcJ7Cd3APZSSpl4SPBcGxSdosinxkIurfTp9yZmc%3D&
reserved=0
---
net: mana: Use GIC functions to allocate global EQs

Replace the GDMA global interrupt setup code with the new GIC allocation and
release functions for managing interrupt contexts.

quoted

diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c

b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index c43fd8089e77..bdc9dc437fb7 100644

--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c

[ ... ]

quoted

@@ -2052,26 +1998,17 @@ static int mana_gd_setup_remaining_irqs(struct

pci_dev *pdev)  static void mana_gd_remove_irqs(struct pci_dev *pdev)
{
 	struct gdma_context *gc = pci_get_drvdata(pdev);
-	struct gdma_irq_context *gic;
 	int irq, i;

 	if (gc->max_num_msix < 1)
 		return;

-	for (i = 0; i < gc->max_num_msix; i++) {
-		irq = pci_irq_vector(pdev, i);
-		if (irq < 0)
-			continue;
-
-		gic = xa_load(&gc->irq_contexts, i);
-		if (WARN_ON(!gic))
-			continue;
-
+	for (i = 0; i < (gc->msi_sharing ? gc->max_num_msix : 1); i++) {

                                                               ^^^ Does this loop condition leak GIC
references when msi_sharing is false?

When msi_sharing is false, the loop only releases the GIC at index 0.
However, mana_gd_setup_dyn_irqs allocates GICs at indices 1 through nvec:

drivers/net/ethernet/microsoft/mana/gdma_main.c:mana_gd_setup_dyn_irqs()
{
    for (i = 1; i <= nvec; i++) {
        gic = mana_gd_get_gic(gc, false, &i);
        ...
    }
}

Each mana_gd_get_gic call initializes the GIC with refcount=1. These GICs at
indices 1..nvec are never released because mana_gd_remove_irqs only processes
index 0 when msi_sharing is false.

The old code iterated over all gc->max_num_msix entries unconditionally:

    for (i = 0; i < gc->max_num_msix; i++) {
        ...
        gic = xa_load(&gc->irq_contexts, i);
        ...
        free_irq(irq, gic);
        xa_erase(&gc->irq_contexts, i);
        kfree(gic);
    }

This ensured every allocated GIC was freed. The new code with the conditional
loop breaks this, resulting in:

1. Leaked IRQ handlers - request_irq without matching free_irq for indices
   1..nvec
2. Leaked gdma_irq_context memory - the GIC structures are never freed 3.
Leaked xarray entries - the entries remain in gc->irq_contexts

Additionally, pci_free_irq_vectors is called immediately after the loop, which
attempts to tear down MSI-X vectors while IRQ handlers at indices 1..nvec are
still registered. Can this trigger warnings or use-after-free if an interrupt fires
during teardown?

The next commit in the series (4475f13983ab "net: mana: Allocate interrupt
context for each EQ when creating vPort") adds per-EQ get/put of GICs, but the
EQ teardown only drops the EQ's reference (refcount goes from 2 to 1). The base
reference from mana_gd_setup_dyn_irqs remains unreleased.

Thank you, I'm sending v4 to fix this.

Long