On Thu, Sep 01, 2022 at 03:04:27PM -0500, Bob Pearson wrote:
> Move referencing pd in mr objects ahead of any possible errors
> so that it will always be set in rxe_mr_complete() to avoid
> seg faults when rxe_put(mr_pd(mr)) is called. Adjust the reference
> counts so that each call to rxe_mr_init_xxx() takes one reference.
> This reference count is dropped in rxe_mr_cleanup() in error paths
> in the reg mr verbs and the dereg mr verb. Minor white space cleanups.
>
> These errors have been seen in rxe_mr_init_user() when there isn't
> enough memory to create the mr maps. Previously the error return
> path didn't reach setting ibpd in mr->ibmr which caused a seg fault.
>
> Fixes: 364e282c4fe7e ("RDMA/rxe: Split MEM into MR and MW")
> Signed-off-by: Bob Pearson <rpearsonhpe@xxxxxxxxx>
> ---
> v6:
> Separated from other patch in original series and resubmitted
> rebased to current for-rc.
> Renamed from "RDMA/rxe: Set pd early in mr alloc routines" to
> "RDMA/rxe: Fix pd ref counting in rxe mr verbs"
> Added more text to describe the change.
> Added fixes line.
> Simplified the patch by leaving pd code in rxe_mr.c instead of
> moving it up to rxe_verbs.c
> v5:
> Dropped cleanup code from patch per Li Zhijian.
> Split up into two small patches.
> v4:
> Added set mr->ibmr.pd back to avoid an -ENOMEM error causing
> rxe_put(mr_pd(mr)); to seg fault in rxe_mr_cleanup() since pd
> is not getting set in the error path.
> v3:
> Rebased to apply to current for-next after
> Revert "RDMA/rxe: Create duplicate mapping tables for FMRs"
> v2:
> Moved setting mr->umem until after checks to avoid sending
> an ERR_PTR to ib_umem_release().
> Cleaned up umem and map sets if errors occur in alloc mr calls.
> Rebased to current for-next.
> ---
> drivers/infiniband/sw/rxe/rxe_mr.c | 24 ++++++++++++++----------
> drivers/infiniband/sw/rxe/rxe_verbs.c | 27 +++++++--------------------
> 2 files changed, 21 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
> index 850b80f5ad8b..5f4daffccb40 100644
> --- a/drivers/infiniband/sw/rxe/rxe_mr.c
> +++ b/drivers/infiniband/sw/rxe/rxe_mr.c
> @@ -107,7 +107,9 @@ void rxe_mr_init_dma(struct rxe_pd *pd, int access, struct rxe_mr *mr)
> {
> rxe_mr_init(access, mr);
>
> + rxe_get(pd);
rxe_get() can fail, why don't you check for failure here and in all
places?
Thanks
