Messages in this thread | | | From | "" <> | Subject | Re: [PATCH for-next v2] RDMA/rxe: Fix mr->map double free | Date | Wed, 16 Nov 2022 02:39:14 +0000 |
| |
On 16/11/2022 08:10, Yanjun Zhu wrote: >> >> index d4f10c2d1aa7..7c99d1591580 100644 >> --- a/drivers/infiniband/sw/rxe/rxe_mr.c >> +++ b/drivers/infiniband/sw/rxe/rxe_mr.c >> @@ -99,6 +99,7 @@ static int rxe_mr_alloc(struct rxe_mr *mr, int num_buf) >> kfree(mr->map[i]); >> kfree(mr->map); >> + mr->map = NULL; >> err1: >> return -ENOMEM; >> } >> @@ -122,7 +123,6 @@ int rxe_mr_init_user(struct rxe_dev *rxe, u64 >> start, u64 length, u64 iova, >> int num_buf; >> void *vaddr; >> int err; >> - int i; >> umem = ib_umem_get(&rxe->ib_dev, start, length, access); >> if (IS_ERR(umem)) { >> @@ -163,9 +163,8 @@ int rxe_mr_init_user(struct rxe_dev *rxe, u64 >> start, u64 length, u64 iova, >> pr_warn("%s: Unable to get virtual address\n", >> __func__); >> err = -ENOMEM; >> - goto err_cleanup_map; >> + goto err_release_umem; > > This call trace results from page_address's returning NULL, then goto > err_cleanup_map where mr->map[i] and mr->map are freed. > > And finally rxe_reg_user_mr gets an error from rxe_mr_init_user, the > function rxe_mr_cleanup is called to handle mr to free mr->map[i] and > mr->map again. > > So mr->map[i] and mr->map are double freed. > > As such, this commit is reasonable. > > But why page_address will return NULL?
ENOMEM? but I don't think we need taking too much care upon the reason.
this patch is most likely porting the reverted back, commit: 8ff5f5d9d8cf ("RDMA/rxe: Prevent double freeing rxe_map_set()")
Actually, the double free can be triggered by below error path too.
149 err = rxe_mr_alloc(mr, num_buf);
150 if (err) {
151 pr_warn("%s: Unable to allocate memory for map\n",
152 __func__);
153 goto err_release_umem;
154 }
where rxe_mr_alloc() freed the memory but don't set 'mr->map = NULL'
Thanks Zhijian
> > Zhu Yanjun >
| |