On Wed, Jul 6, 2022 at 3:10 PM Yanjun Zhu <yanjun.zhu@xxxxxxxxx> wrote:
>
> 在 2022/7/5 18:35, Haris Iqbal 写道:
> > 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?
>
> It is in a different commit. I will send it out very soon.
Okay. Thank you!
>
> Zhu Yanjun
>
> >
> >>
>
