[PATCH v2] nvme: Add error path for xa_store in nvme_init_effects

[PATCH v2] nvme: Add error path for xa_store in nvme_init_effects

[Keisuke Nishimura]

The xa_store() may fail due to memory allocation failure because there
is no guarantee that the index NVME_CSI_NVM is already used. This fix
introduces a new function to handle the error path.

Fixes: cc115cbe12d9 ("nvme: always initialize known command effects")
Signed-off-by: Keisuke Nishimura <keisuke.nishimura@inria.fr>
---
 drivers/nvme/host/core.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 758de89b47b4..ce8b70dc45b2 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3173,6 +3173,25 @@ static int nvme_init_non_mdts_limits(struct nvme_ctrl *ctrl)
 	return ret;
 }
 
+static int nvme_init_effects_log(struct nvme_ctrl *ctrl,
+		u8 csi, struct nvme_effects_log **log)
+{
+	struct nvme_effects_log *effects, *old;
+
+	effects = kzalloc(sizeof(*effects), GFP_KERNEL);
+	if (effects)
+		return -ENOMEM;
+
+	old = xa_store(&ctrl->cels, csi, effects, GFP_KERNEL);
+	if (xa_is_err(old)) {
+		kfree(effects);
+		return xa_err(old);
+	}
+
+	*log = effects;
+	return 0;
+}
+
 static void nvme_init_known_nvm_effects(struct nvme_ctrl *ctrl)
 {
 	struct nvme_effects_log	*log = ctrl->effects;
@@ -3219,10 +3238,9 @@ static int nvme_init_effects(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
 	}
 
 	if (!ctrl->effects) {
-		ctrl->effects = kzalloc(sizeof(*ctrl->effects), GFP_KERNEL);
-		if (!ctrl->effects)
-			return -ENOMEM;
-		xa_store(&ctrl->cels, NVME_CSI_NVM, ctrl->effects, GFP_KERNEL);
+		ret = nvme_init_effects_log(ctrl, NVME_CSI_NVM, &ctrl->effects);
+		if (ret < 0)
+			return ret;
 	}
 
 	nvme_init_known_nvm_effects(ctrl);
-- 
2.34.1

Re: [PATCH v2] nvme: Add error path for xa_store in nvme_init_effects

[Christoph Hellwig]

On Mon, Dec 16, 2024 at 04:27:20PM +0100, Keisuke Nishimura wrote:
> The xa_store() may fail due to memory allocation failure because there
> is no guarantee that the index NVME_CSI_NVM is already used. This fix
> introduces a new function to handle the error path.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v2] nvme: Add error path for xa_store in nvme_init_effects

[Keisuke Nishimura]

On 16/12/2024 16:49, Christoph Hellwig wrote:
> On Mon, Dec 16, 2024 at 04:27:20PM +0100, Keisuke Nishimura wrote:
>> The xa_store() may fail due to memory allocation failure because there
>> is no guarantee that the index NVME_CSI_NVM is already used. This fix
>> introduces a new function to handle the error path.
> 
> Looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 


This is a gentle ping. Please let me know if there is anything I can do:)

best,
Keisuke

Re: [PATCH v2] nvme: Add error path for xa_store in nvme_init_effects

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH v2] nvme: Add error path for xa_store in nvme_init_effects

[Keith Busch]

On Mon, Dec 16, 2024 at 04:27:20PM +0100, Keisuke Nishimura wrote:
> The xa_store() may fail due to memory allocation failure because there
> is no guarantee that the index NVME_CSI_NVM is already used. This fix
> introduces a new function to handle the error path.
> 
> Fixes: cc115cbe12d9 ("nvme: always initialize known command effects")
> Signed-off-by: Keisuke Nishimura <keisuke.nishimura@inria.fr>

Queued up in nvme-6.14, thanks.