diff mbox series

[PATCHv2,1/1] RDMA/core: avoid kernel NULL pointer error

Message ID 1574655875-3475-1-git-send-email-yanjun.zhu@oracle.com (mailing list archive)
State Changes Requested
Delegated to: Jason Gunthorpe
Headers show
Series [PATCHv2,1/1] RDMA/core: avoid kernel NULL pointer error | expand

Commit Message

Zhu Yanjun Nov. 25, 2019, 4:24 a.m. UTC
When the interface related with IB device is set to down/up over and
over again, the following call trace will pop out.
"
 Call Trace:
  [<ffffffffa039ff8d>] ib_mad_completion_handler+0x7d/0xa0 [ib_mad]
  [<ffffffff810a1a41>] process_one_work+0x151/0x4b0
  [<ffffffff810a1ec0>] worker_thread+0x120/0x480
  [<ffffffff810a709e>] kthread+0xce/0xf0
  [<ffffffff816e9962>] ret_from_fork+0x42/0x70

 RIP  [<ffffffffa039f926>] ib_mad_recv_done_handler+0x26/0x610 [ib_mad]
"
From vmcore, we can find the following:
"
crash7lates> struct ib_mad_list_head ffff881fb3713400
struct ib_mad_list_head {
  list = {
    next = 0xffff881fb3713800,
    prev = 0xffff881fe01395c0
  },
  mad_queue = 0x0
}
"

Before the call trace, a lot of ib_cancel_mad is sent to the sender.
So it is necessary to check mad_queue in struct ib_mad_list_head to avoid
"kernel NULL pointer" error.

From the new customer report, when there is something wrong with IB HW/FW,
the above call trace will appear. It seems that bad IB HW/FW will cause
this problem.

Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
---
V1->V2: Add new bug symptoms.
---
 drivers/infiniband/core/mad.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Zhu Yanjun Nov. 25, 2019, 4:29 a.m. UTC | #1
Probably this problem is caused by IB HW/FW. When IB device is set to down/up

for several times or IB HW/FW is bad, this similar prolem will appear.

In the future, when the developer confronts this similar problem, he can use

this patch to have a try.

Zhu Yanjun

On Mon, Nov 25, 2019 at 12:14 PM Zhu Yanjun <yanjun.zhu@oracle.com> wrote:
>
> When the interface related with IB device is set to down/up over and
> over again, the following call trace will pop out.
> "
>  Call Trace:
>   [<ffffffffa039ff8d>] ib_mad_completion_handler+0x7d/0xa0 [ib_mad]
>   [<ffffffff810a1a41>] process_one_work+0x151/0x4b0
>   [<ffffffff810a1ec0>] worker_thread+0x120/0x480
>   [<ffffffff810a709e>] kthread+0xce/0xf0
>   [<ffffffff816e9962>] ret_from_fork+0x42/0x70
>
>  RIP  [<ffffffffa039f926>] ib_mad_recv_done_handler+0x26/0x610 [ib_mad]
> "
> From vmcore, we can find the following:
> "
> crash7lates> struct ib_mad_list_head ffff881fb3713400
> struct ib_mad_list_head {
>   list = {
>     next = 0xffff881fb3713800,
>     prev = 0xffff881fe01395c0
>   },
>   mad_queue = 0x0
> }
> "
>
> Before the call trace, a lot of ib_cancel_mad is sent to the sender.
> So it is necessary to check mad_queue in struct ib_mad_list_head to avoid
> "kernel NULL pointer" error.
>
> From the new customer report, when there is something wrong with IB HW/FW,
> the above call trace will appear. It seems that bad IB HW/FW will cause
> this problem.
>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
> ---
> V1->V2: Add new bug symptoms.
> ---
>  drivers/infiniband/core/mad.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
> index 9947d16..43f596c 100644
> --- a/drivers/infiniband/core/mad.c
> +++ b/drivers/infiniband/core/mad.c
> @@ -2279,6 +2279,17 @@ static void ib_mad_recv_done(struct ib_cq *cq, struct ib_wc *wc)
>                 return;
>         }
>
> +       if (unlikely(!mad_list->mad_queue)) {
> +               /*
> +                * When the interface related with IB device is set to down/up,
> +                * a lot of ib_cancel_mad packets are sent to the sender. In
> +                * sender, the mad packets are cancelled.  The receiver will
> +                * find mad_queue NULL. If the receiver does not test mad_queue,
> +                * the receiver will crash with "kernel NULL pointer" error.
> +                */
> +               return;
> +       }
> +
>         qp_info = mad_list->mad_queue->qp_info;
>         dequeue_mad(mad_list);
>
> --
> 2.7.4
>
Jason Gunthorpe Nov. 27, 2019, 8:07 p.m. UTC | #2
On Sun, Nov 24, 2019 at 11:24:35PM -0500, Zhu Yanjun wrote:
> When the interface related with IB device is set to down/up over and
> over again, the following call trace will pop out.
> "
>  Call Trace:
>   [<ffffffffa039ff8d>] ib_mad_completion_handler+0x7d/0xa0 [ib_mad]
>   [<ffffffff810a1a41>] process_one_work+0x151/0x4b0
>   [<ffffffff810a1ec0>] worker_thread+0x120/0x480
>   [<ffffffff810a709e>] kthread+0xce/0xf0
>   [<ffffffff816e9962>] ret_from_fork+0x42/0x70
> 
>  RIP  [<ffffffffa039f926>] ib_mad_recv_done_handler+0x26/0x610 [ib_mad]
> "
> From vmcore, we can find the following:
> "
> crash7lates> struct ib_mad_list_head ffff881fb3713400
> struct ib_mad_list_head {
>   list = {
>     next = 0xffff881fb3713800,
>     prev = 0xffff881fe01395c0
>   },
>   mad_queue = 0x0
> }
> "
> 
> Before the call trace, a lot of ib_cancel_mad is sent to the sender.
> So it is necessary to check mad_queue in struct ib_mad_list_head to avoid
> "kernel NULL pointer" error.
> 
> From the new customer report, when there is something wrong with IB HW/FW,
> the above call trace will appear. It seems that bad IB HW/FW will cause
> this problem.
> 
> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
> V1->V2: Add new bug symptoms.
>  drivers/infiniband/core/mad.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
> index 9947d16..43f596c 100644
> +++ b/drivers/infiniband/core/mad.c
> @@ -2279,6 +2279,17 @@ static void ib_mad_recv_done(struct ib_cq *cq, struct ib_wc *wc)
>  		return;
>  	}
>  
> +	if (unlikely(!mad_list->mad_queue)) {
> +		/*
> +		 * When the interface related with IB device is set to down/up,
> +		 * a lot of ib_cancel_mad packets are sent to the sender. In
> +		 * sender, the mad packets are cancelled.  The receiver will
> +		 * find mad_queue NULL. If the receiver does not test mad_queue,
> +		 * the receiver will crash with "kernel NULL pointer" error.
> +		 */
> +		return;
> +	}

I feel like this patch was sent already? 

It is not possible for mad_queue to be NULL here without another bug,
so this can't be the right fix.

This is because:

		mad_priv->header.mad_list.mad_queue = recv_queue;
		mad_priv->header.mad_list.cqe.done = ib_mad_recv_done;
		recv_wr.wr_cqe = &mad_priv->header.mad_list.cqe;

And then we do

	struct ib_mad_list_head *mad_list =
		container_of(wc->wr_cqe, struct ib_mad_list_head, cqe);

So there is no point where the mad_list could be legimiately NULL'd
before getting here, something else must be happening, you must figure
out and describe how the NULL is happening.

Jason
Zhu Yanjun Nov. 28, 2019, 5:06 a.m. UTC | #3
On Thu, Nov 28, 2019 at 4:07 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Sun, Nov 24, 2019 at 11:24:35PM -0500, Zhu Yanjun wrote:
> > When the interface related with IB device is set to down/up over and
> > over again, the following call trace will pop out.
> > "
> >  Call Trace:
> >   [<ffffffffa039ff8d>] ib_mad_completion_handler+0x7d/0xa0 [ib_mad]
> >   [<ffffffff810a1a41>] process_one_work+0x151/0x4b0
> >   [<ffffffff810a1ec0>] worker_thread+0x120/0x480
> >   [<ffffffff810a709e>] kthread+0xce/0xf0
> >   [<ffffffff816e9962>] ret_from_fork+0x42/0x70
> >
> >  RIP  [<ffffffffa039f926>] ib_mad_recv_done_handler+0x26/0x610 [ib_mad]
> > "
> > From vmcore, we can find the following:
> > "
> > crash7lates> struct ib_mad_list_head ffff881fb3713400
> > struct ib_mad_list_head {
> >   list = {
> >     next = 0xffff881fb3713800,
> >     prev = 0xffff881fe01395c0
> >   },
> >   mad_queue = 0x0
> > }
> > "
> >
> > Before the call trace, a lot of ib_cancel_mad is sent to the sender.
> > So it is necessary to check mad_queue in struct ib_mad_list_head to avoid
> > "kernel NULL pointer" error.
> >
> > From the new customer report, when there is something wrong with IB HW/FW,
> > the above call trace will appear. It seems that bad IB HW/FW will cause
> > this problem.
> >
> > Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
> > V1->V2: Add new bug symptoms.
> >  drivers/infiniband/core/mad.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
> > index 9947d16..43f596c 100644
> > +++ b/drivers/infiniband/core/mad.c
> > @@ -2279,6 +2279,17 @@ static void ib_mad_recv_done(struct ib_cq *cq, struct ib_wc *wc)
> >               return;
> >       }
> >
> > +     if (unlikely(!mad_list->mad_queue)) {
> > +             /*
> > +              * When the interface related with IB device is set to down/up,
> > +              * a lot of ib_cancel_mad packets are sent to the sender. In
> > +              * sender, the mad packets are cancelled.  The receiver will
> > +              * find mad_queue NULL. If the receiver does not test mad_queue,
> > +              * the receiver will crash with "kernel NULL pointer" error.
> > +              */
> > +             return;
> > +     }
>
> I feel like this patch was sent already?
>
> It is not possible for mad_queue to be NULL here without another bug,
> so this can't be the right fix.
>
> This is because:
>
>                 mad_priv->header.mad_list.mad_queue = recv_queue;
>                 mad_priv->header.mad_list.cqe.done = ib_mad_recv_done;
>                 recv_wr.wr_cqe = &mad_priv->header.mad_list.cqe;
>
> And then we do
>
>         struct ib_mad_list_head *mad_list =
>                 container_of(wc->wr_cqe, struct ib_mad_list_head, cqe);
>
> So there is no point where the mad_list could be legimiately NULL'd
> before getting here, something else must be happening, you must figure
> out and describe how the NULL is happening.

Yes. From the kernel source code, this bug does not occur. But from
the bug symptoms, it is possible that this bug is caused by the HW/FW.
From the commit logs, in 2 scenarios,  this bug will occur. One is to
set IB interface down/up over and over again, the other is bad IB
device.
After the bad IB device is replaced, this bug did not appear again.

The reason that I sent this patch again is to let the developer
suspect the HW/FW when this bug occurs again. I can not check the
HW/FW since I can not access these IB devices and FW source code.

Just as a reminder, not to expect to merge this patch into mainline.

Zhu Yanjun

>
> Jason
diff mbox series

Patch

diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
index 9947d16..43f596c 100644
--- a/drivers/infiniband/core/mad.c
+++ b/drivers/infiniband/core/mad.c
@@ -2279,6 +2279,17 @@  static void ib_mad_recv_done(struct ib_cq *cq, struct ib_wc *wc)
 		return;
 	}
 
+	if (unlikely(!mad_list->mad_queue)) {
+		/*
+		 * When the interface related with IB device is set to down/up,
+		 * a lot of ib_cancel_mad packets are sent to the sender. In
+		 * sender, the mad packets are cancelled.  The receiver will
+		 * find mad_queue NULL. If the receiver does not test mad_queue,
+		 * the receiver will crash with "kernel NULL pointer" error.
+		 */
+		return;
+	}
+
 	qp_info = mad_list->mad_queue->qp_info;
 	dequeue_mad(mad_list);