[PATCH] RDMA/hfi1: fix init_one() probe failure cleanup

[PATCH] RDMA/hfi1: fix init_one() probe failure cleanup

[Dawei Feng]

init_one() allocates hfi1_devdata before validating several module
parameters and initializing PCIe.  Failures in these paths currently jump
to bail and leak the devdata allocated by hfi1_alloc_devdata().

Probe failures after hfi1_init_dd() need a different cleanup path.  On
failure, hfi1_init_dd() frees devdata itself, but after it succeeds the
driver also owns RX state and MSI-X interrupt resources that must be
released before postinit_cleanup().

Fix the early paths to free devdata directly, keep the hfi1_init_dd()
failure path to PCIe cleanup only, and release MSI-X and RX resources on
post-hfi1_init_dd() failures.

The bug was first flagged by an experimental analysis tool we are
developing for kernel memory-management bugs while analyzing
v6.13-rc1. The tool is still under development and is not yet publicly
available. Manual inspection confirms that the bug is still
present in v7.1.1.

An x86_64 allyesconfig build showed no new warnings. As we do not have an
HFI1 adapter to test with, no runtime testing was able to be performed.

Fixes: 7724105686e7 ("IB/hfi1: add driver files")
Fixes: 57f97e96625f ("IB/hfi1: Get the hfi1_devdata structure as early as possible")
Fixes: 4730f4a6c6b2 ("IB/hfi1: Activate the dummy netdev")
Cc: stable@vger.kernel.org
Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
---
 drivers/infiniband/hw/hfi1/init.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/init.c b/drivers/infiniband/hw/hfi1/init.c
index b7fd8b1fbbbd..09fc71891e4c 100644
--- a/drivers/infiniband/hw/hfi1/init.c
+++ b/drivers/infiniband/hw/hfi1/init.c
@@ -1590,14 +1590,14 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	/* Validate some global module parameters */
 	ret = hfi1_validate_rcvhdrcnt(dd, rcvhdrcnt);
 	if (ret)
-		goto bail;
+		goto free_dd;
 
 	/* use the encoding function as a sanitization check */
 	if (!encode_rcv_header_entry_size(hfi1_hdrq_entsize)) {
 		dd_dev_err(dd, "Invalid HdrQ Entry size %u\n",
 			   hfi1_hdrq_entsize);
 		ret = -EINVAL;
-		goto bail;
+		goto free_dd;
 	}
 
 	/* The receive eager buffer size must be set before the receive
@@ -1622,7 +1622,7 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	} else {
 		dd_dev_err(dd, "Invalid Eager buffer size of 0\n");
 		ret = -EINVAL;
-		goto bail;
+		goto free_dd;
 	}
 
 	/* restrict value of hfi1_rcvarr_split */
@@ -1630,7 +1630,7 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	ret = hfi1_pcie_init(dd);
 	if (ret)
-		goto bail;
+		goto free_dd;
 
 	/*
 	 * Do device-specific initialization, function table setup, dd
@@ -1642,7 +1642,7 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	ret = create_workqueues(dd);
 	if (ret)
-		goto clean_bail;
+		goto postinit_bail;
 
 	/* do the generic initialization */
 	initfail = hfi1_init(dd, 0);
@@ -1685,6 +1685,7 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 			hfi1_device_remove(dd);
 		if (!ret)
 			hfi1_unregister_ib_device(dd);
+		hfi1_free_rx(dd);
 		postinit_cleanup(dd);
 		if (initfail)
 			ret = initfail;
@@ -1695,8 +1696,18 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	return 0;
 
+postinit_bail:
+	msix_clean_up_interrupts(dd);
+	hfi1_free_rx(dd);
+	postinit_cleanup(dd);
+	goto bail;
+
 clean_bail:
 	hfi1_pcie_cleanup(pdev);
+	goto bail;
+
+free_dd:
+	hfi1_free_devdata(dd);
 bail:
 	return ret;
 }
-- 
2.34.1

Re: [PATCH] RDMA/hfi1: fix init_one() probe failure cleanup

[Leon Romanovsky]

On Sat, Jun 27, 2026 at 02:01:59PM +0800, Dawei Feng wrote:
> init_one() allocates hfi1_devdata before validating several module
> parameters and initializing PCIe.  Failures in these paths currently jump
> to bail and leak the devdata allocated by hfi1_alloc_devdata().
> 
> Probe failures after hfi1_init_dd() need a different cleanup path.  On
> failure, hfi1_init_dd() frees devdata itself, but after it succeeds the
> driver also owns RX state and MSI-X interrupt resources that must be
> released before postinit_cleanup().
> 
> Fix the early paths to free devdata directly, keep the hfi1_init_dd()
> failure path to PCIe cleanup only, and release MSI-X and RX resources on
> post-hfi1_init_dd() failures.
> 
> The bug was first flagged by an experimental analysis tool we are
> developing for kernel memory-management bugs while analyzing
> v6.13-rc1. The tool is still under development and is not yet publicly
> available. Manual inspection confirms that the bug is still
> present in v7.1.1.
> 
> An x86_64 allyesconfig build showed no new warnings. As we do not have an
> HFI1 adapter to test with, no runtime testing was able to be performed.
> 
> Fixes: 7724105686e7 ("IB/hfi1: add driver files")
> Fixes: 57f97e96625f ("IB/hfi1: Get the hfi1_devdata structure as early as possible")
> Fixes: 4730f4a6c6b2 ("IB/hfi1: Activate the dummy netdev")
> Cc: stable@vger.kernel.org
> Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
> ---
>  drivers/infiniband/hw/hfi1/init.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)

Just move hfi1_validate_rcvhdrcnt() to be before hfi1_alloc_devdata()
and remove error prints.

Thanks

Re: [PATCH] RDMA/hfi1: fix init_one() probe failure cleanup

[Dawei Feng]

On Sun, 5 Jul 2026 15:23:28 +0300, Leon Romanovsky wrote:
>Just move hfi1_validate_rcvhdrcnt() to be before hfi1_alloc_devdata()
>and remove error prints.

Hi, Leon,

I agree that this approach is suitable for the hfi1_validate_rcvhdrcnt()
failure path. However, it is not generally applicable to the other failure
paths. The later failures still occur after hfi1_alloc_devdata(), so they
still need proper cleanup handling to avoid leaking the allocated devdata.
Therefore, I think the current approach may be more appropriate for this
fix.

By the way, I also noticed that the error handling can be optimized by
returning ret directly from cleanup labels, avoiding chained gotos. Would
you like me to make this change in the next version? The change would look
like this:

+postinit_bail:
+	hfi1_free_rx(dd);
+	postinit_cleanup(dd);
+	return ret;
+
 clean_bail:
 	hfi1_pcie_cleanup(pdev);
+	return ret;
+
+free_dd:
+	hfi1_free_devdata(dd);
+	return ret;
 bail:
 	return ret;
}

Best regards,
Dawei