[PATCH v2] RDMA/mlx5: Wait for in-flight page faults on implicit MR null_mkey dereg

[PATCH v2] RDMA/mlx5: Wait for in-flight page faults on implicit MR null_mkey dereg

[lirongqing]

From: Li RongQing <lirongqing@baidu.com>

An implicit ODP MR (imr) publishes two mkeys into dev->odp_mkeys: the
main imr->mmkey and imr->null_mmkey (MLX5_MKEY_NULL). Both are stored via
mlx5r_store_odp_mkey(), which initialises their usecount to 1, and
find_odp_mkey() takes a reference on whichever mkey it finds for the
duration of a page fault.

__mlx5_ib_dereg_mr() erases and waits on the *main* mmkey usecount only.
For the null_mmkey, mlx5_ib_free_odp_mr() merely xa_erase()s it from
odp_mkeys and calls mlx5_core_destroy_mkey() -- it never waits for an
in-flight memory-scheme page fault (MLX5_MKEY_NULL) that holds a
reference on null_mmkey. After xa_erase(), find_odp_mkey() stops
returning the null_mmkey, but a fault already past the lookup still
holds a reference and dereferences the imr (via container_of and
pagefault_mr) after __mlx5_ib_dereg_mr() proceeds to kfree(mr).

Mirror the main-mmkey handling: after xa_erase() of null_mmkey, call
mlx5r_deref_wait_odp_mkey() to drop the reference taken at store time
and wait for any in-flight fault to finish before destroying the mkey
and freeing the imr.

mlx5r_deref_wait_odp_mkey() wakes and waits on null_mmkey.wait, so that
waitqueue must be initialised. The main mmkey's waitqueue is initialised
in mlx5_mr_cache_alloc(), but the null_mmkey is created through the raw
mlx5_core_create_mkey() (not the mlx5_ib_create_mkey() wrapper that
initialises the waitqueue) and nothing else sets it up, leaving it
zero-initialised. Initialising it in alloc_implicit_mr_null_mkey() right
after creation avoids a NULL pointer dereference in __wake_up_common()
when the waitqueue head's ->head.next is NULL.

Fixes: 6f2487bfafce ("RDMA/mlx5: Add implicit MR handling to ODP memory scheme")
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
Diff with v1: add init_waitqueue_head(&imr->null_mmkey.wait);

 drivers/infiniband/hw/mlx5/odp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/infiniband/hw/mlx5/odp.c b/drivers/infiniband/hw/mlx5/odp.c
index b861861..766f545 100644
--- a/drivers/infiniband/hw/mlx5/odp.c
+++ b/drivers/infiniband/hw/mlx5/odp.c
@@ -595,6 +595,7 @@ static int alloc_implicit_mr_null_mkey(struct mlx5_ib_dev *dev,
 		goto free_in;
 
 	imr->null_mmkey.type = MLX5_MKEY_NULL;
+	init_waitqueue_head(&imr->null_mmkey.wait);
 
 free_in:
 	kfree(in);
@@ -683,6 +684,8 @@ void mlx5_ib_free_odp_mr(struct mlx5_ib_mr *mr)
 		xa_erase(&mr_to_mdev(mr)->odp_mkeys,
 			 mlx5_base_mkey(mr->null_mmkey.key));
 
+		mlx5r_deref_wait_odp_mkey(&mr->null_mmkey);
+
 		mlx5_core_destroy_mkey(mr_to_mdev(mr)->mdev,
 				       mr->null_mmkey.key);
 	}
-- 
2.9.4

Re: [PATCH v2] RDMA/mlx5: Wait for in-flight page faults on implicit MR null_mkey dereg

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] RDMA/mlx5: child MR leak in mlx5_ib_free_odp_mr due to late null_mmkey wait
--

commit f5fc9e80765babb54d4d8c2db70efadd75e2ccc8
Author: Li RongQing <lirongqing@baidu.com>

    RDMA/mlx5: Wait for in-flight page faults on implicit MR null_mkey dereg

This commit mirrors the main-mmkey handling for null_mmkey by dropping the
reference and waiting for any in-flight faults to finish before destroying
the mkey and freeing the imr. It also initializes the waitqueue for the
null_mmkey right after creation to avoid a NULL pointer dereference.

> diff --git a/drivers/infiniband/hw/mlx5/odp.c b/drivers/infiniband/hw/mlx5/odp.c
> index 8dcbb0e5ccd7c..c7367718e24f2 100644
> --- a/drivers/infiniband/hw/mlx5/odp.c
> +++ b/drivers/infiniband/hw/mlx5/odp.c
[ ... ]
> @@ -683,6 +684,8 @@ void mlx5_ib_free_odp_mr(struct mlx5_ib_mr *mr)
>  		xa_erase(&mr_to_mdev(mr)->odp_mkeys,
>  			 mlx5_base_mkey(mr->null_mmkey.key));
>  
> +		mlx5r_deref_wait_odp_mkey(&mr->null_mmkey);
> +

[Severity: High]
Does this code leak child MRs when an in-flight page fault races with MR
deregistration?

If a page fault on an implicit ODP MR uses the null_mmkey and is queued
concurrently with MR deregistration, __mlx5_ib_dereg_mr() will call
mlx5_ib_free_odp_mr() which iterates over and deletes existing child MRs.

Because the null_mmkey is erased and waited upon here, after the
mr->implicit_children xarray is already cleaned up, the in-flight page
fault handler can still look up the null_mmkey, take a reference, and
create a new child MR.

The new child is inserted into mr->implicit_children (which succeeds
because the slot was just emptied by the cleanup loop). Finally,
mlx5_ib_free_odp_mr() waits for the handlers to finish. The parent mr is
freed, but the newly created child MR is permanently leaked and retains a
dangling parent pointer.

>  		mlx5_core_destroy_mkey(mr_to_mdev(mr)->mdev,
>  				       mr->null_mmkey.key);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924065017.2424-1-lirongqing@baidu.com?part=1