Skip to contentSkip to conversation

nvme: apple: fix device reference counting

PATCH In review Bin Lan 2 participants 2 messages Nov 26, 2024 Stable branch

lore
Share & tools
lore

Bin Lan

author
#0 lore raw

PATCH 1/1 nvme: apple: fix device reference counting

From: Keith Busch <[email protected]>

[ Upstream commit b9ecbfa45516182cd062fecd286db7907ba84210 ]

Drivers must call nvme_uninit_ctrl after a successful nvme_init_ctrl. Split the allocation side out to make the error handling boundary easier to navigate. The apple driver had been doing this wrong, leaking the controller device memory on a tagset failure.

Reviewed-by Christoph Hellwig <[email protected]>
Reviewed-by Chaitanya Kulkarni <[email protected]>
Signed-off-by Keith Busch <[email protected]>
[ Resolve minor conflicts ]
Signed-off-by Bin Lan <[email protected]>
---
 drivers/nvme/host/apple.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)
drivers/nvme/host/apple.c +22 -5
index 596bb11eeba5..396eb9437659 100644
@@ -1387,7 +1387,7 @@ static void devm_apple_nvme_mempool_destroy(void *data)
 	mempool_destroy(data);
 }
 
-static int apple_nvme_probe(struct platform_device *pdev)
+static struct apple_nvme *apple_nvme_alloc(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct apple_nvme *anv;

@@ -1395,7 +1395,7 @@ static int apple_nvme_probe(struct platform_device *pdev)
 
 	anv = devm_kzalloc(dev, sizeof(*anv), GFP_KERNEL);
 	if (!anv)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	anv->dev = get_device(dev);
 	anv->adminq.is_adminq = true;

@@ -1515,10 +1515,26 @@ static int apple_nvme_probe(struct platform_device *pdev)
 		goto put_dev;
 	}
 
+	return anv;
+put_dev:
+	put_device(anv->dev);
+	return ERR_PTR(ret);
+}
+
+static int apple_nvme_probe(struct platform_device *pdev)
+{
+	struct apple_nvme *anv;
+	int ret;
+
+	anv = apple_nvme_alloc(pdev);
+	if (IS_ERR(anv))
+		return PTR_ERR(anv);
+
 	anv->ctrl.admin_q = blk_mq_init_queue(&anv->admin_tagset);
 	if (IS_ERR(anv->ctrl.admin_q)) {
 		ret = -ENOMEM;
-		goto put_dev;
+		anv->ctrl.admin_q = NULL;
+		goto out_uninit_ctrl;
 	}
 
 	nvme_reset_ctrl(&anv->ctrl);

@@ -1526,8 +1542,9 @@ static int apple_nvme_probe(struct platform_device *pdev)
 
 	return 0;
 
-put_dev:
-	put_device(anv->dev);
+out_uninit_ctrl:
+	nvme_uninit_ctrl(&anv->ctrl);
+	nvme_put_ctrl(&anv->ctrl);
 	return ret;
 }
Signature · 1 line
2.34.1
maintainer
#1 lore raw

[ Sasha's backport helper bot ]

Hi,

The upstream commit SHA1 provided is correct: b9ecbfa45516182cd062fecd286db7907ba84210

WARNING: Author mismatch between patch and upstream commit:
Backport author: Bin Lan <[email protected]> Commit author: Keith Busch <[email protected]>

Status in newer kernel trees:
6.12.y | Present (exact SHA1)
6.11.y | Present (exact SHA1)
6.6.y | Not found

Note: The patch differs from the upstream commit:
---

tmp/tmp.6GPhqxSP1X renamed from - +13 -6
@@ -1,3 +1,5 @@
+[ Upstream commit b9ecbfa45516182cd062fecd286db7907ba84210 ]
+
 Drivers must call nvme_uninit_ctrl after a successful nvme_init_ctrl.
 Split the allocation side out to make the error handling boundary easier
 to navigate. The apple driver had been doing this wrong, leaking the

@@ -6,15 +8,17 @@
 Reviewed-by: Christoph Hellwig <[email protected]>
 Reviewed-by: Chaitanya Kulkarni <[email protected]>
 Signed-off-by: Keith Busch <[email protected]>
+[ Resolve minor conflicts ]
+Signed-off-by: Bin Lan <[email protected]>
 ---
  drivers/nvme/host/apple.c | 27 ++++++++++++++++++++++-----
  1 file changed, 22 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/nvme/host/apple.c b/drivers/nvme/host/apple.c
-index dd6ec0865141a..c43ada920c3b2 100644
+index 596bb11eeba5..396eb9437659 100644
 --- a/drivers/nvme/host/apple.c
 +++ b/drivers/nvme/host/apple.c
-@@ -1388,7 +1388,7 @@ static void devm_apple_nvme_mempool_destroy(void *data)
+@@ -1387,7 +1387,7 @@ static void devm_apple_nvme_mempool_destroy(void *data)
  	mempool_destroy(data);
  }
  

@@ -23,7 +27,7 @@
  {
  	struct device *dev = &pdev->dev;
  	struct apple_nvme *anv;
-@@ -1396,7 +1396,7 @@ static int apple_nvme_probe(struct platform_device *pdev)
+@@ -1395,7 +1395,7 @@ static int apple_nvme_probe(struct platform_device *pdev)
  
  	anv = devm_kzalloc(dev, sizeof(*anv), GFP_KERNEL);
  	if (!anv)

@@ -32,7 +36,7 @@
  
  	anv->dev = get_device(dev);
  	anv->adminq.is_adminq = true;
-@@ -1516,10 +1516,26 @@ static int apple_nvme_probe(struct platform_device *pdev)
+@@ -1515,10 +1515,26 @@ static int apple_nvme_probe(struct platform_device *pdev)
  		goto put_dev;
  	}
  

@@ -51,7 +55,7 @@
 +	if (IS_ERR(anv))
 +		return PTR_ERR(anv);
 +
- 	anv->ctrl.admin_q = blk_mq_alloc_queue(&anv->admin_tagset, NULL, NULL);
+ 	anv->ctrl.admin_q = blk_mq_init_queue(&anv->admin_tagset);
  	if (IS_ERR(anv->ctrl.admin_q)) {
  		ret = -ENOMEM;
 -		goto put_dev;

@@ -60,7 +64,7 @@
  	}
  
  	nvme_reset_ctrl(&anv->ctrl);
-@@ -1527,8 +1543,9 @@ static int apple_nvme_probe(struct platform_device *pdev)
+@@ -1526,8 +1542,9 @@ static int apple_nvme_probe(struct platform_device *pdev)
  
  	return 0;
  

@@ -72,3 +76,6 @@
  	return ret;
  }
  
+-- 
+2.34.1
+

---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-6.6.y        |  Success    |  Success   |