[PATCH v2] RDMA/mlx5: Fix devx subscribe-event unwind NULL dereference

[PATCH v2] RDMA/mlx5: Fix devx subscribe-event unwind NULL dereference

[Prathamesh Deshpande]

MLX5_IB_METHOD_DEVX_SUBSCRIBE_EVENT() links event_sub into sub_list
before initializing the fields used by the shared error path.

If eventfd_ctx_fdget() then fails, the unwind path dereferences
event_sub->ev_file in uverbs_uobject_put() and calls
subscribe_event_xa_dealloc() with an unset xa_key_level1.

subscribe_event_xa_alloc() creates the XA entry exactly once for a given
key_level1, on the first occurrence of that key. The unwind path must
therefore call subscribe_event_xa_dealloc() exactly once for it as well.

Enforce that by adding devx_key_in_sub_list() and calling
subscribe_event_xa_dealloc() only when the last matching pending entry is
being cleaned up.

Fixes: 759738537142 ("IB/mlx5: Enable subscription for device events over DEVX")
Signed-off-by: Prathamesh Deshpande <prathameshdeshpande7@gmail.com>
---
v2:
   - fix duplicate-key unwind by deallocating each XA entry only once
   - add devx_key_in_sub_list() to detect the last matching pending entry
   - keep event_sub->ev_file and xa_key_level1 initialization before sub_list insertion
   - update commit message to explain the duplicate-key unwind rule

 drivers/infiniband/hw/mlx5/devx.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/devx.c b/drivers/infiniband/hw/mlx5/devx.c
index 645ebcc0832d..c2ae5a140471 100644
--- a/drivers/infiniband/hw/mlx5/devx.c
+++ b/drivers/infiniband/hw/mlx5/devx.c
@@ -1913,6 +1913,17 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_ASYNC_QUERY)(
 	return err;
 }
 
+static bool devx_key_in_sub_list(struct list_head *list, u32 key_level1)
+{
+	struct devx_event_subscription *s;
+
+	list_for_each_entry(s, list, event_list)
+		if (s->xa_key_level1 == key_level1)
+			return true;
+
+	return false;
+}
+
 static void
 subscribe_event_xa_dealloc(struct mlx5_devx_event_table *devx_event_table,
 			   u32 key_level1,
@@ -2160,10 +2171,17 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_SUBSCRIBE_EVENT)(
 
 		event_sub = kzalloc_obj(*event_sub);
 		if (!event_sub) {
+			if (!devx_key_in_sub_list(&sub_list, key_level1))
+				subscribe_event_xa_dealloc(devx_event_table,
+							   key_level1,
+							   obj,
+							   obj_id);
 			err = -ENOMEM;
 			goto err;
 		}
 
+		event_sub->ev_file = ev_file;
+		event_sub->xa_key_level1 = key_level1;
 		list_add_tail(&event_sub->event_list, &sub_list);
 		uverbs_uobject_get(&ev_file->uobj);
 		if (use_eventfd) {
@@ -2178,9 +2196,6 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_SUBSCRIBE_EVENT)(
 		}
 
 		event_sub->cookie = cookie;
-		event_sub->ev_file = ev_file;
-		/* May be needed upon cleanup the devx object/subscription */
-		event_sub->xa_key_level1 = key_level1;
 		event_sub->xa_key_level2 = obj_id;
 		INIT_LIST_HEAD(&event_sub->obj_list);
 	}
@@ -2225,10 +2240,11 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_SUBSCRIBE_EVENT)(
 	list_for_each_entry_safe(event_sub, tmp_sub, &sub_list, event_list) {
 		list_del(&event_sub->event_list);
 
-		subscribe_event_xa_dealloc(devx_event_table,
-					   event_sub->xa_key_level1,
-					   obj,
-					   obj_id);
+		if (!devx_key_in_sub_list(&sub_list, event_sub->xa_key_level1))
+			subscribe_event_xa_dealloc(devx_event_table,
+						   event_sub->xa_key_level1,
+						   obj,
+						   obj_id);
 
 		if (event_sub->eventfd)
 			eventfd_ctx_put(event_sub->eventfd);
-- 
2.43.0

Re: [PATCH v2] RDMA/mlx5: Fix devx subscribe-event unwind NULL dereference

[Yishai Hadas]

On 29/04/2026 1:42, Prathamesh Deshpande wrote:
> MLX5_IB_METHOD_DEVX_SUBSCRIBE_EVENT() links event_sub into sub_list
> before initializing the fields used by the shared error path.
> 
> If eventfd_ctx_fdget() then fails, the unwind path dereferences
> event_sub->ev_file in uverbs_uobject_put() and calls
> subscribe_event_xa_dealloc() with an unset xa_key_level1.
> 
> subscribe_event_xa_alloc() creates the XA entry exactly once for a given
> key_level1, on the first occurrence of that key. The unwind path must
> therefore call subscribe_event_xa_dealloc() exactly once for it as well.
> 
> Enforce that by adding devx_key_in_sub_list() and calling
> subscribe_event_xa_dealloc() only when the last matching pending entry is
> being cleaned up.
> 
> Fixes: 759738537142 ("IB/mlx5: Enable subscription for device events over DEVX")
> Signed-off-by: Prathamesh Deshpande <prathameshdeshpande7@gmail.com>
> ---
> v2:
>     - fix duplicate-key unwind by deallocating each XA entry only once
>     - add devx_key_in_sub_list() to detect the last matching pending entry
>     - keep event_sub->ev_file and xa_key_level1 initialization before sub_list insertion
>     - update commit message to explain the duplicate-key unwind rule
> 
>   drivers/infiniband/hw/mlx5/devx.c | 30 +++++++++++++++++++++++-------
>   1 file changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/mlx5/devx.c b/drivers/infiniband/hw/mlx5/devx.c
> index 645ebcc0832d..c2ae5a140471 100644
> --- a/drivers/infiniband/hw/mlx5/devx.c
> +++ b/drivers/infiniband/hw/mlx5/devx.c
> @@ -1913,6 +1913,17 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_ASYNC_QUERY)(
>   	return err;
>   }
>   
> +static bool devx_key_in_sub_list(struct list_head *list, u32 key_level1)
> +{
> +	struct devx_event_subscription *s;
> +
> +	list_for_each_entry(s, list, event_list)
> +		if (s->xa_key_level1 == key_level1)
> +			return true;
> +
> +	return false;
> +}
> +
>   static void
>   subscribe_event_xa_dealloc(struct mlx5_devx_event_table *devx_event_table,
>   			   u32 key_level1,
> @@ -2160,10 +2171,17 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_SUBSCRIBE_EVENT)(
>   
>   		event_sub = kzalloc_obj(*event_sub);
>   		if (!event_sub) {
> +			if (!devx_key_in_sub_list(&sub_list, key_level1))
> +				subscribe_event_xa_dealloc(devx_event_table,
> +							   key_level1,
> +							   obj,
> +							   obj_id);
>   			err = -ENOMEM;
>   			goto err;
>   		}
>   
> +		event_sub->ev_file = ev_file;
> +		event_sub->xa_key_level1 = key_level1;
>   		list_add_tail(&event_sub->event_list, &sub_list);
>   		uverbs_uobject_get(&ev_file->uobj);
>   		if (use_eventfd) {
> @@ -2178,9 +2196,6 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_SUBSCRIBE_EVENT)(
>   		}
>   
>   		event_sub->cookie = cookie;
> -		event_sub->ev_file = ev_file;
> -		/* May be needed upon cleanup the devx object/subscription */
> -		event_sub->xa_key_level1 = key_level1;
>   		event_sub->xa_key_level2 = obj_id;
>   		INIT_LIST_HEAD(&event_sub->obj_list);
>   	}
> @@ -2225,10 +2240,11 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_SUBSCRIBE_EVENT)(
>   	list_for_each_entry_safe(event_sub, tmp_sub, &sub_list, event_list) {
>   		list_del(&event_sub->event_list);
>   
> -		subscribe_event_xa_dealloc(devx_event_table,
> -					   event_sub->xa_key_level1,
> -					   obj,
> -					   obj_id);
> +		if (!devx_key_in_sub_list(&sub_list, event_sub->xa_key_level1))
> +			subscribe_event_xa_dealloc(devx_event_table,
> +						   event_sub->xa_key_level1,
> +						   obj,
> +						   obj_id);
>   
>   		if (event_sub->eventfd)
>   			eventfd_ctx_put(event_sub->eventfd);

Reviewed-by: Yishai Hadas <yishaih@nvidia.com>

Yishai