On Tue, Jul 5, 2022 at 8:28 AM <yanjun.zhu@xxxxxxxxx> wrote:
>
> From: Zhu Yanjun <yanjun.zhu@xxxxxxxxx>
>
> The function rxe_create_qp calls rxe_qp_from_init. If some error
> occurs, the error handler of function rxe_qp_from_init will set
> both scq and rcq to NULL.
>
> Then rxe_create_qp calls rxe_put to handle qp. In the end,
> rxe_qp_do_cleanup is called by rxe_put. rxe_qp_do_cleanup directly
> accesses scq and rcq before checking them. This will cause
> null-ptr-deref error.
>
> The call graph is as below:
>
> rxe_create_qp {
> ...
> rxe_qp_from_init {
> ...
> err1:
> ...
> qp->rcq = NULL; <---rcq is set to NULL
> qp->scq = NULL; <---scq is set to NULL
> ...
> }
>
> qp_init:
> rxe_put{
> ...
> rxe_qp_do_cleanup {
> ...
> atomic_dec(&qp->scq->num_wq); <--- scq is accessed
> ...
> atomic_dec(&qp->rcq->num_wq); <--- rcq is accessed
> }
> }
>
> Fixes: 4703b4f0d94a ("RDMA/rxe: Enforce IBA C11-17")
> Signed-off-by: Zhu Yanjun <yanjun.zhu@xxxxxxxxx>
> ---
> V1->V2: Describe the error flows.
> ---
> drivers/infiniband/sw/rxe/rxe_qp.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
> index 22e9b85344c3..b79e1b43454e 100644
> --- a/drivers/infiniband/sw/rxe/rxe_qp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_qp.c
> @@ -804,13 +804,15 @@ static void rxe_qp_do_cleanup(struct work_struct *work)
> if (qp->rq.queue)
> rxe_queue_cleanup(qp->rq.queue);
>
> - atomic_dec(&qp->scq->num_wq);
> - if (qp->scq)
> + if (qp->scq) {
> + atomic_dec(&qp->scq->num_wq);
> rxe_put(qp->scq);
> + }
>
> - atomic_dec(&qp->rcq->num_wq);
> - if (qp->rcq)
> + if (qp->rcq) {
> + atomic_dec(&qp->rcq->num_wq);
> rxe_put(qp->rcq);
> + }
>
> if (qp->pd)
> rxe_put(qp->pd);
> --
> 2.34.1
Looks good.
Reviewed-by: Md Haris Iqbal <haris.iqbal@xxxxxxxxx>
Should the check for "rxe_cleanup_task(&qp->comp.task);" also happen
in this commit? or would it be covered in a different one?
>
