From: Keith Busch kbusch@kernel.org
[ 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 hch@lst.de Reviewed-by: Chaitanya Kulkarni kch@nvidia.com Signed-off-by: Keith Busch kbusch@kernel.org [Minor context change fixed.] Signed-off-by: Bin Lan bin.lan.cn@windriver.com Signed-off-by: He Zhe zhe.he@windriver.com --- Build test passed. --- drivers/nvme/host/apple.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/drivers/nvme/host/apple.c b/drivers/nvme/host/apple.c index 262d2b60ac6d..057917e23b6f 100644 --- a/drivers/nvme/host/apple.c +++ b/drivers/nvme/host/apple.c @@ -1373,7 +1373,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; @@ -1381,7 +1381,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; @@ -1501,10 +1501,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; }
if (!blk_get_queue(anv->ctrl.admin_q)) { @@ -1512,7 +1528,7 @@ static int apple_nvme_probe(struct platform_device *pdev) blk_mq_destroy_queue(anv->ctrl.admin_q); anv->ctrl.admin_q = NULL; ret = -ENODEV; - goto put_dev; + goto out_uninit_ctrl; }
nvme_reset_ctrl(&anv->ctrl); @@ -1520,8 +1536,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; }
[ Sasha's backport helper bot ]
Hi,
✅ All tests passed successfully. No issues detected. No action required from the submitter.
The upstream commit SHA1 provided is correct: b9ecbfa45516182cd062fecd286db7907ba84210
WARNING: Author mismatch between patch and upstream commit: Backport author: bin.lan.cn@windriver.com Commit author: Keith Buschkbusch@kernel.org
Status in newer kernel trees: 6.14.y | Present (exact SHA1) 6.12.y | Present (exact SHA1) 6.6.y | Present (different SHA1: f7d9a18572fc)
Note: The patch differs from the upstream commit: --- 1: b9ecbfa455161 ! 1: 66a0d9b8ca96a nvme: apple: fix device reference counting @@ Metadata ## Commit message ## nvme: apple: fix device reference counting
+ [ 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 @@ Commit message Reviewed-by: Christoph Hellwig hch@lst.de Reviewed-by: Chaitanya Kulkarni kch@nvidia.com Signed-off-by: Keith Busch kbusch@kernel.org + [Minor context change fixed.] + Signed-off-by: Bin Lan bin.lan.cn@windriver.com + Signed-off-by: He Zhe zhe.he@windriver.com
## drivers/nvme/host/apple.c ## @@ drivers/nvme/host/apple.c: static void devm_apple_nvme_mempool_destroy(void *data) @@ drivers/nvme/host/apple.c: static int apple_nvme_probe(struct platform_device *p + 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; @@ drivers/nvme/host/apple.c: static int apple_nvme_probe(struct platform_device *p + goto out_uninit_ctrl; }
+ if (!blk_get_queue(anv->ctrl.admin_q)) { +@@ drivers/nvme/host/apple.c: static int apple_nvme_probe(struct platform_device *pdev) + blk_mq_destroy_queue(anv->ctrl.admin_q); + anv->ctrl.admin_q = NULL; + ret = -ENODEV; +- goto put_dev; ++ goto out_uninit_ctrl; + } + nvme_reset_ctrl(&anv->ctrl); @@ drivers/nvme/host/apple.c: static int apple_nvme_probe(struct platform_device *pdev)
---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-6.1.y | Success | Success |
linux-stable-mirror@lists.linaro.org