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

[PATCH] 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
adds an error handling path to check the return value.

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

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 758de89b47b4..38eb42f1110e 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3219,10 +3219,18 @@ 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)
+		struct nvme_effects_log *effects, *old;
+
+		effects = kzalloc(sizeof(*effects), GFP_KERNEL);
+		if (effects)
 			return -ENOMEM;
-		xa_store(&ctrl->cels, NVME_CSI_NVM, ctrl->effects, GFP_KERNEL);
+
+		old = xa_store(&ctrl->cels, NVME_CSI_NVM, effects, GFP_KERNEL);
+		if (xa_is_err(old)) {
+			kfree(effects);
+			return xa_err(old);
+		}
+		ctrl->effects = effects;
 	}
 
 	nvme_init_known_nvm_effects(ctrl);
-- 
2.34.1

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

[Christoph Hellwig]

On Mon, Dec 02, 2024 at 04:39:22PM +0100, Keisuke Nishimura wrote:
>  	if (!ctrl->effects) {
> +		struct nvme_effects_log *effects, *old;
> +
> +		effects = kzalloc(sizeof(*effects), GFP_KERNEL);
> +		if (effects)
>  			return -ENOMEM;
> +
> +		old = xa_store(&ctrl->cels, NVME_CSI_NVM, effects, GFP_KERNEL);
> +		if (xa_is_err(old)) {
> +			kfree(effects);
> +			return xa_err(old);
> +		}
> +		ctrl->effects = effects;

Please split this into a helper now that is becoming rather complex.

Otherwise this looks fine to me.

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

[Keisuke Nishimura]

On 12/12/2024 07:28, Christoph Hellwig wrote:
> On Mon, Dec 02, 2024 at 04:39:22PM +0100, Keisuke Nishimura wrote:
>>   	if (!ctrl->effects) {
>> +		struct nvme_effects_log *effects, *old;
>> +
>> +		effects = kzalloc(sizeof(*effects), GFP_KERNEL);
>> +		if (effects)
>>   			return -ENOMEM;
>> +
>> +		old = xa_store(&ctrl->cels, NVME_CSI_NVM, effects, GFP_KERNEL);
>> +		if (xa_is_err(old)) {
>> +			kfree(effects);
>> +			return xa_err(old);
>> +		}
>> +		ctrl->effects = effects;
> 
> Please split this into a helper now that is becoming rather complex.
> 
> Otherwise this looks fine to me.

Thank you for the review. I will send the new version soon.

Keisuke