All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCHv2 1/1] RDMA/rxe: Fix BUG: KASAN: null-ptr-deref in rxe_qp_do_cleanup
  2022-07-05 22:54 [PATCHv2 1/1] RDMA/rxe: Fix BUG: KASAN: null-ptr-deref in rxe_qp_do_cleanup yanjun.zhu
@ 2022-07-05 10:35 ` Haris Iqbal
  2022-07-06 13:10   ` Yanjun Zhu
  2022-07-18 11:33 ` Leon Romanovsky
  1 sibling, 1 reply; 6+ messages in thread
From: Haris Iqbal @ 2022-07-05 10:35 UTC (permalink / raw)
  To: yanjun.zhu; +Cc: jgg, leon, linux-rdma

On Tue, Jul 5, 2022 at 8:28 AM <yanjun.zhu@linux.dev> wrote:
>
> From: Zhu Yanjun <yanjun.zhu@linux.dev>
>
> 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@linux.dev>
> ---
> 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@ionos.com>

Should the check for "rxe_cleanup_task(&qp->comp.task);" also happen
in this commit? or would it be covered in a different one?

>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCHv2 1/1] RDMA/rxe: Fix BUG: KASAN: null-ptr-deref in rxe_qp_do_cleanup
@ 2022-07-05 22:54 yanjun.zhu
  2022-07-05 10:35 ` Haris Iqbal
  2022-07-18 11:33 ` Leon Romanovsky
  0 siblings, 2 replies; 6+ messages in thread
From: yanjun.zhu @ 2022-07-05 22:54 UTC (permalink / raw)
  To: jgg, leon, linux-rdma, yanjun.zhu

From: Zhu Yanjun <yanjun.zhu@linux.dev>

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@linux.dev>
---
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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 1/1] RDMA/rxe: Fix BUG: KASAN: null-ptr-deref in rxe_qp_do_cleanup
  2022-07-05 10:35 ` Haris Iqbal
@ 2022-07-06 13:10   ` Yanjun Zhu
  2022-07-06 13:11     ` Haris Iqbal
  0 siblings, 1 reply; 6+ messages in thread
From: Yanjun Zhu @ 2022-07-06 13:10 UTC (permalink / raw)
  To: Haris Iqbal, yanjun.zhu; +Cc: jgg, leon, linux-rdma

在 2022/7/5 18:35, Haris Iqbal 写道:
> On Tue, Jul 5, 2022 at 8:28 AM <yanjun.zhu@linux.dev> wrote:
>>
>> From: Zhu Yanjun <yanjun.zhu@linux.dev>
>>
>> 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@linux.dev>
>> ---
>> 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@ionos.com>
> 
> 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.

Zhu Yanjun

> 
>>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 1/1] RDMA/rxe: Fix BUG: KASAN: null-ptr-deref in rxe_qp_do_cleanup
  2022-07-06 13:10   ` Yanjun Zhu
@ 2022-07-06 13:11     ` Haris Iqbal
  2022-07-14 16:57       ` Bob Pearson
  0 siblings, 1 reply; 6+ messages in thread
From: Haris Iqbal @ 2022-07-06 13:11 UTC (permalink / raw)
  To: Yanjun Zhu; +Cc: jgg, leon, linux-rdma

On Wed, Jul 6, 2022 at 3:10 PM Yanjun Zhu <yanjun.zhu@linux.dev> wrote:
>
> 在 2022/7/5 18:35, Haris Iqbal 写道:
> > On Tue, Jul 5, 2022 at 8:28 AM <yanjun.zhu@linux.dev> wrote:
> >>
> >> From: Zhu Yanjun <yanjun.zhu@linux.dev>
> >>
> >> 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@linux.dev>
> >> ---
> >> 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@ionos.com>
> >
> > 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
>
> >
> >>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 1/1] RDMA/rxe: Fix BUG: KASAN: null-ptr-deref in rxe_qp_do_cleanup
  2022-07-06 13:11     ` Haris Iqbal
@ 2022-07-14 16:57       ` Bob Pearson
  0 siblings, 0 replies; 6+ messages in thread
From: Bob Pearson @ 2022-07-14 16:57 UTC (permalink / raw)
  To: Haris Iqbal, Yanjun Zhu; +Cc: jgg, leon, linux-rdma

On 7/6/22 08:11, Haris Iqbal wrote:
> On Wed, Jul 6, 2022 at 3:10 PM Yanjun Zhu <yanjun.zhu@linux.dev> wrote:
>>
>> 在 2022/7/5 18:35, Haris Iqbal 写道:
>>> On Tue, Jul 5, 2022 at 8:28 AM <yanjun.zhu@linux.dev> wrote:
>>>>
>>>> From: Zhu Yanjun <yanjun.zhu@linux.dev>
>>>>
>>>> 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@linux.dev>
>>>> ---
>>>> 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@ionos.com>
>>>
>>> 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
>>
>>>
>>>>
>>

Agreed.

Reviewed-by: Bob Pearson <rpearsonhpe@gmail.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 1/1] RDMA/rxe: Fix BUG: KASAN: null-ptr-deref in rxe_qp_do_cleanup
  2022-07-05 22:54 [PATCHv2 1/1] RDMA/rxe: Fix BUG: KASAN: null-ptr-deref in rxe_qp_do_cleanup yanjun.zhu
  2022-07-05 10:35 ` Haris Iqbal
@ 2022-07-18 11:33 ` Leon Romanovsky
  1 sibling, 0 replies; 6+ messages in thread
From: Leon Romanovsky @ 2022-07-18 11:33 UTC (permalink / raw)
  To: yanjun.zhu; +Cc: jgg, linux-rdma

On Tue, Jul 05, 2022 at 06:54:14PM -0400, yanjun.zhu@linux.dev wrote:
> From: Zhu Yanjun <yanjun.zhu@linux.dev>
> 
> 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@linux.dev>
> ---
> V1->V2: Describe the error flows.
> ---
>  drivers/infiniband/sw/rxe/rxe_qp.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 

Thanks, applied.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-07-18 11:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-05 22:54 [PATCHv2 1/1] RDMA/rxe: Fix BUG: KASAN: null-ptr-deref in rxe_qp_do_cleanup yanjun.zhu
2022-07-05 10:35 ` Haris Iqbal
2022-07-06 13:10   ` Yanjun Zhu
2022-07-06 13:11     ` Haris Iqbal
2022-07-14 16:57       ` Bob Pearson
2022-07-18 11:33 ` Leon Romanovsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.