public inbox for linux-rdma@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Fixes for syzbot problem
@ 2022-08-22  1:16 yanjun.zhu
  2022-08-22  1:16 ` [PATCH 1/3] RDMA/rxe: Fix "kernel NULL pointer dereference" error yanjun.zhu
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: yanjun.zhu @ 2022-08-22  1:16 UTC (permalink / raw)
  To: jgg, leon, linux-rdma, yanjun.zhu, zyjzyj2000

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

In the link:

https://syzkaller.appspot.com/bug?id=3efca8275142fbfdde6cf77e1b18b1d5c6794d76

a rxe problem occurred. I tried to reproduce this problem in local hosts.
Finally I could reproduce this problem in local hosts.
The commit ("RDMA/rxe: Fix "kernel NULL pointer dereference" error") tries to
fix this problem.

After this syzbot problem disappeared, another problem appeared
when qp->sk failed to initialized.
The commit ("RDMA/rxe: Fix the error caused by qp->sk") tries to solve
this problem.

When I delved into the source code to solve the above problems, I found
that the member variable obj in struct rxe_task is not needed. So the
commit ("RDMA/rxe: Remove the unused variable obj") removes this
variable obj.

After the 3 commits, in locat hosts, the whole system can work well.

Zhu Yanjun (3):
  RDMA/rxe: Fix "kernel NULL pointer dereference" error
  RDMA/rxe: Fix the error caused by qp->sk
  RDMA/rxe: Remove the unused variable obj

 drivers/infiniband/sw/rxe/rxe_qp.c   | 16 ++++++++++------
 drivers/infiniband/sw/rxe/rxe_task.c |  3 +--
 drivers/infiniband/sw/rxe/rxe_task.h |  3 +--
 3 files changed, 12 insertions(+), 10 deletions(-)

-- 
2.31.1


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

* [PATCH 1/3] RDMA/rxe: Fix "kernel NULL pointer dereference" error
  2022-08-22  1:16 [PATCH 0/3] Fixes for syzbot problem yanjun.zhu
@ 2022-08-22  1:16 ` yanjun.zhu
  2022-08-22 19:00   ` Bob Pearson
  2022-08-22  1:16 ` [PATCH 2/3] RDMA/rxe: Fix the error caused by qp->sk yanjun.zhu
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: yanjun.zhu @ 2022-08-22  1:16 UTC (permalink / raw)
  To: jgg, leon, linux-rdma, yanjun.zhu, zyjzyj2000; +Cc: syzbot+ab99dc4c6e961eed8b8e

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

When rxe_queue_init in the function rxe_qp_init_req fails,
both qp->req.task.func and qp->req.task.arg are not initialized.

Because of creation of qp fails, the function rxe_create_qp will
call rxe_qp_do_cleanup to handle allocated resource.

Before calling __rxe_do_task, both qp->req.task.func and
qp->req.task.arg should be checked.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Reported-by: syzbot+ab99dc4c6e961eed8b8e@syzkaller.appspotmail.com
Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
---
 drivers/infiniband/sw/rxe/rxe_qp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
index 516bf9b95e48..f10b461b9963 100644
--- a/drivers/infiniband/sw/rxe/rxe_qp.c
+++ b/drivers/infiniband/sw/rxe/rxe_qp.c
@@ -797,7 +797,9 @@ static void rxe_qp_do_cleanup(struct work_struct *work)
 	rxe_cleanup_task(&qp->comp.task);
 
 	/* flush out any receive wr's or pending requests */
-	__rxe_do_task(&qp->req.task);
+	if (qp->req.task.func && qp->req.task.arg)
+		__rxe_do_task(&qp->req.task);
+
 	if (qp->sq.queue) {
 		__rxe_do_task(&qp->comp.task);
 		__rxe_do_task(&qp->req.task);
-- 
2.31.1


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

* [PATCH 2/3] RDMA/rxe: Fix the error caused by qp->sk
  2022-08-22  1:16 [PATCH 0/3] Fixes for syzbot problem yanjun.zhu
  2022-08-22  1:16 ` [PATCH 1/3] RDMA/rxe: Fix "kernel NULL pointer dereference" error yanjun.zhu
@ 2022-08-22  1:16 ` yanjun.zhu
  2022-08-22 19:01   ` Bob Pearson
  2022-08-22  1:16 ` [PATCH 3/3] RDMA/rxe: Remove the unused variable obj yanjun.zhu
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: yanjun.zhu @ 2022-08-22  1:16 UTC (permalink / raw)
  To: jgg, leon, linux-rdma, yanjun.zhu, zyjzyj2000

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

When sock_create_kern in the function rxe_qp_init_req fails,
qp->sk is set to NULL.

Then the function rxe_create_qp will call rxe_qp_do_cleanup
to handle allocated resource.

Before handling qp->sk, this variable should be checked.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
---
 drivers/infiniband/sw/rxe/rxe_qp.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
index f10b461b9963..b229052ae91a 100644
--- a/drivers/infiniband/sw/rxe/rxe_qp.c
+++ b/drivers/infiniband/sw/rxe/rxe_qp.c
@@ -835,8 +835,10 @@ static void rxe_qp_do_cleanup(struct work_struct *work)
 
 	free_rd_atomic_resources(qp);
 
-	kernel_sock_shutdown(qp->sk, SHUT_RDWR);
-	sock_release(qp->sk);
+	if (qp->sk) {
+		kernel_sock_shutdown(qp->sk, SHUT_RDWR);
+		sock_release(qp->sk);
+	}
 }
 
 /* called when the last reference to the qp is dropped */
-- 
2.31.1


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

* [PATCH 3/3] RDMA/rxe: Remove the unused variable obj
  2022-08-22  1:16 [PATCH 0/3] Fixes for syzbot problem yanjun.zhu
  2022-08-22  1:16 ` [PATCH 1/3] RDMA/rxe: Fix "kernel NULL pointer dereference" error yanjun.zhu
  2022-08-22  1:16 ` [PATCH 2/3] RDMA/rxe: Fix the error caused by qp->sk yanjun.zhu
@ 2022-08-22  1:16 ` yanjun.zhu
  2022-08-22 19:06   ` Bob Pearson
  2022-08-29 14:59 ` [PATCH 0/3] Fixes for syzbot problem Yanjun Zhu
  2022-08-31  6:54 ` Leon Romanovsky
  4 siblings, 1 reply; 12+ messages in thread
From: yanjun.zhu @ 2022-08-22  1:16 UTC (permalink / raw)
  To: jgg, leon, linux-rdma, yanjun.zhu, zyjzyj2000

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

The member variable obj in struct rxe_task is not needed.
So remove it to save memory.

Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
---
 drivers/infiniband/sw/rxe/rxe_qp.c   | 6 +++---
 drivers/infiniband/sw/rxe/rxe_task.c | 3 +--
 drivers/infiniband/sw/rxe/rxe_task.h | 3 +--
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
index b229052ae91a..ee4f7a4a7e01 100644
--- a/drivers/infiniband/sw/rxe/rxe_qp.c
+++ b/drivers/infiniband/sw/rxe/rxe_qp.c
@@ -242,9 +242,9 @@ static int rxe_qp_init_req(struct rxe_dev *rxe, struct rxe_qp *qp,
 
 	skb_queue_head_init(&qp->req_pkts);
 
-	rxe_init_task(rxe, &qp->req.task, qp,
+	rxe_init_task(&qp->req.task, qp,
 		      rxe_requester, "req");
-	rxe_init_task(rxe, &qp->comp.task, qp,
+	rxe_init_task(&qp->comp.task, qp,
 		      rxe_completer, "comp");
 
 	qp->qp_timeout_jiffies = 0; /* Can't be set for UD/UC in modify_qp */
@@ -292,7 +292,7 @@ static int rxe_qp_init_resp(struct rxe_dev *rxe, struct rxe_qp *qp,
 
 	skb_queue_head_init(&qp->resp_pkts);
 
-	rxe_init_task(rxe, &qp->resp.task, qp,
+	rxe_init_task(&qp->resp.task, qp,
 		      rxe_responder, "resp");
 
 	qp->resp.opcode		= OPCODE_NONE;
diff --git a/drivers/infiniband/sw/rxe/rxe_task.c b/drivers/infiniband/sw/rxe/rxe_task.c
index 2248cf33d776..ec2b7de1c497 100644
--- a/drivers/infiniband/sw/rxe/rxe_task.c
+++ b/drivers/infiniband/sw/rxe/rxe_task.c
@@ -94,10 +94,9 @@ void rxe_do_task(struct tasklet_struct *t)
 	task->ret = ret;
 }
 
-int rxe_init_task(void *obj, struct rxe_task *task,
+int rxe_init_task(struct rxe_task *task,
 		  void *arg, int (*func)(void *), char *name)
 {
-	task->obj	= obj;
 	task->arg	= arg;
 	task->func	= func;
 	snprintf(task->name, sizeof(task->name), "%s", name);
diff --git a/drivers/infiniband/sw/rxe/rxe_task.h b/drivers/infiniband/sw/rxe/rxe_task.h
index 11d183fd3338..7f612a1c68a7 100644
--- a/drivers/infiniband/sw/rxe/rxe_task.h
+++ b/drivers/infiniband/sw/rxe/rxe_task.h
@@ -19,7 +19,6 @@ enum {
  * called again.
  */
 struct rxe_task {
-	void			*obj;
 	struct tasklet_struct	tasklet;
 	int			state;
 	spinlock_t		state_lock; /* spinlock for task state */
@@ -35,7 +34,7 @@ struct rxe_task {
  *	arg  => parameter to pass to fcn
  *	func => function to call until it returns != 0
  */
-int rxe_init_task(void *obj, struct rxe_task *task,
+int rxe_init_task(struct rxe_task *task,
 		  void *arg, int (*func)(void *), char *name);
 
 /* cleanup task */
-- 
2.31.1


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

* Re: [PATCH 1/3] RDMA/rxe: Fix "kernel NULL pointer dereference" error
  2022-08-22  1:16 ` [PATCH 1/3] RDMA/rxe: Fix "kernel NULL pointer dereference" error yanjun.zhu
@ 2022-08-22 19:00   ` Bob Pearson
  2022-08-23  5:43     ` lizhijian
  0 siblings, 1 reply; 12+ messages in thread
From: Bob Pearson @ 2022-08-22 19:00 UTC (permalink / raw)
  To: yanjun.zhu, jgg, leon, linux-rdma, zyjzyj2000; +Cc: syzbot+ab99dc4c6e961eed8b8e

On 8/21/22 20:16, yanjun.zhu@linux.dev wrote:
> From: Zhu Yanjun <yanjun.zhu@linux.dev>
> 
> When rxe_queue_init in the function rxe_qp_init_req fails,
> both qp->req.task.func and qp->req.task.arg are not initialized.
> 
> Because of creation of qp fails, the function rxe_create_qp will
> call rxe_qp_do_cleanup to handle allocated resource.
> 
> Before calling __rxe_do_task, both qp->req.task.func and
> qp->req.task.arg should be checked.
> 
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Reported-by: syzbot+ab99dc4c6e961eed8b8e@syzkaller.appspotmail.com
> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
> ---
>  drivers/infiniband/sw/rxe/rxe_qp.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
> index 516bf9b95e48..f10b461b9963 100644
> --- a/drivers/infiniband/sw/rxe/rxe_qp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_qp.c
> @@ -797,7 +797,9 @@ static void rxe_qp_do_cleanup(struct work_struct *work)
>  	rxe_cleanup_task(&qp->comp.task);
>  
>  	/* flush out any receive wr's or pending requests */
> -	__rxe_do_task(&qp->req.task);
> +	if (qp->req.task.func && qp->req.task.arg)
func would be enough since they get set together. But, this is still fine since not performance critical.
> +		__rxe_do_task(&qp->req.task);
> +
>  	if (qp->sq.queue) {
>  		__rxe_do_task(&qp->comp.task);
>  		__rxe_do_task(&qp->req.task);

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

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

* Re: [PATCH 2/3] RDMA/rxe: Fix the error caused by qp->sk
  2022-08-22  1:16 ` [PATCH 2/3] RDMA/rxe: Fix the error caused by qp->sk yanjun.zhu
@ 2022-08-22 19:01   ` Bob Pearson
  2022-08-23  5:53     ` lizhijian
  0 siblings, 1 reply; 12+ messages in thread
From: Bob Pearson @ 2022-08-22 19:01 UTC (permalink / raw)
  To: yanjun.zhu, jgg, leon, linux-rdma, zyjzyj2000

On 8/21/22 20:16, yanjun.zhu@linux.dev wrote:
> From: Zhu Yanjun <yanjun.zhu@linux.dev>
> 
> When sock_create_kern in the function rxe_qp_init_req fails,
> qp->sk is set to NULL.
> 
> Then the function rxe_create_qp will call rxe_qp_do_cleanup
> to handle allocated resource.
> 
> Before handling qp->sk, this variable should be checked.
> 
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
> ---
>  drivers/infiniband/sw/rxe/rxe_qp.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
> index f10b461b9963..b229052ae91a 100644
> --- a/drivers/infiniband/sw/rxe/rxe_qp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_qp.c
> @@ -835,8 +835,10 @@ static void rxe_qp_do_cleanup(struct work_struct *work)
>  
>  	free_rd_atomic_resources(qp);
>  
> -	kernel_sock_shutdown(qp->sk, SHUT_RDWR);
> -	sock_release(qp->sk);
> +	if (qp->sk) {
> +		kernel_sock_shutdown(qp->sk, SHUT_RDWR);
> +		sock_release(qp->sk);
> +	}
>  }
>  
>  /* called when the last reference to the qp is dropped */

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

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

* Re: [PATCH 3/3] RDMA/rxe: Remove the unused variable obj
  2022-08-22  1:16 ` [PATCH 3/3] RDMA/rxe: Remove the unused variable obj yanjun.zhu
@ 2022-08-22 19:06   ` Bob Pearson
  2022-08-23  5:54     ` lizhijian
  0 siblings, 1 reply; 12+ messages in thread
From: Bob Pearson @ 2022-08-22 19:06 UTC (permalink / raw)
  To: yanjun.zhu, jgg, leon, linux-rdma, zyjzyj2000

On 8/21/22 20:16, yanjun.zhu@linux.dev wrote:
> From: Zhu Yanjun <yanjun.zhu@linux.dev>
> 
> The member variable obj in struct rxe_task is not needed.
> So remove it to save memory.
> 
> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
> ---
>  drivers/infiniband/sw/rxe/rxe_qp.c   | 6 +++---
>  drivers/infiniband/sw/rxe/rxe_task.c | 3 +--
>  drivers/infiniband/sw/rxe/rxe_task.h | 3 +--
>  3 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
> index b229052ae91a..ee4f7a4a7e01 100644
> --- a/drivers/infiniband/sw/rxe/rxe_qp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_qp.c
> @@ -242,9 +242,9 @@ static int rxe_qp_init_req(struct rxe_dev *rxe, struct rxe_qp *qp,
>  
>  	skb_queue_head_init(&qp->req_pkts);
>  
> -	rxe_init_task(rxe, &qp->req.task, qp,
> +	rxe_init_task(&qp->req.task, qp,
>  		      rxe_requester, "req");
> -	rxe_init_task(rxe, &qp->comp.task, qp,
> +	rxe_init_task(&qp->comp.task, qp,
>  		      rxe_completer, "comp");
>  
>  	qp->qp_timeout_jiffies = 0; /* Can't be set for UD/UC in modify_qp */
> @@ -292,7 +292,7 @@ static int rxe_qp_init_resp(struct rxe_dev *rxe, struct rxe_qp *qp,
>  
>  	skb_queue_head_init(&qp->resp_pkts);
>  
> -	rxe_init_task(rxe, &qp->resp.task, qp,
> +	rxe_init_task(&qp->resp.task, qp,
>  		      rxe_responder, "resp");
>  
>  	qp->resp.opcode		= OPCODE_NONE;
> diff --git a/drivers/infiniband/sw/rxe/rxe_task.c b/drivers/infiniband/sw/rxe/rxe_task.c
> index 2248cf33d776..ec2b7de1c497 100644
> --- a/drivers/infiniband/sw/rxe/rxe_task.c
> +++ b/drivers/infiniband/sw/rxe/rxe_task.c
> @@ -94,10 +94,9 @@ void rxe_do_task(struct tasklet_struct *t)
>  	task->ret = ret;
>  }
>  
> -int rxe_init_task(void *obj, struct rxe_task *task,
> +int rxe_init_task(struct rxe_task *task,
>  		  void *arg, int (*func)(void *), char *name)
>  {
> -	task->obj	= obj;
>  	task->arg	= arg;
>  	task->func	= func;
>  	snprintf(task->name, sizeof(task->name), "%s", name);
> diff --git a/drivers/infiniband/sw/rxe/rxe_task.h b/drivers/infiniband/sw/rxe/rxe_task.h
> index 11d183fd3338..7f612a1c68a7 100644
> --- a/drivers/infiniband/sw/rxe/rxe_task.h
> +++ b/drivers/infiniband/sw/rxe/rxe_task.h
> @@ -19,7 +19,6 @@ enum {
>   * called again.
>   */
>  struct rxe_task {
> -	void			*obj;
>  	struct tasklet_struct	tasklet;
>  	int			state;
>  	spinlock_t		state_lock; /* spinlock for task state */
> @@ -35,7 +34,7 @@ struct rxe_task {
>   *	arg  => parameter to pass to fcn
>   *	func => function to call until it returns != 0
>   */
> -int rxe_init_task(void *obj, struct rxe_task *task,
> +int rxe_init_task(struct rxe_task *task,
>  		  void *arg, int (*func)(void *), char *name);
>  
>  /* cleanup task */

Looks OK to me. Not sure why that was ever there.

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

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

* Re: [PATCH 1/3] RDMA/rxe: Fix "kernel NULL pointer dereference" error
  2022-08-22 19:00   ` Bob Pearson
@ 2022-08-23  5:43     ` lizhijian
  0 siblings, 0 replies; 12+ messages in thread
From: lizhijian @ 2022-08-23  5:43 UTC (permalink / raw)
  To: Bob Pearson, yanjun.zhu@linux.dev, jgg@ziepe.ca, leon@kernel.org,
	linux-rdma@vger.kernel.org, zyjzyj2000@gmail.com
  Cc: syzbot+ab99dc4c6e961eed8b8e@syzkaller.appspotmail.com



On 23/08/2022 03:00, Bob Pearson wrote:
> On 8/21/22 20:16, yanjun.zhu@linux.dev wrote:
>> From: Zhu Yanjun <yanjun.zhu@linux.dev>
>>
>> When rxe_queue_init in the function rxe_qp_init_req fails,
>> both qp->req.task.func and qp->req.task.arg are not initialized.
>>
>> Because of creation of qp fails, the function rxe_create_qp will
>> call rxe_qp_do_cleanup to handle allocated resource.
>>
>> Before calling __rxe_do_task, both qp->req.task.func and
>> qp->req.task.arg should be checked.
>>
>> Fixes: 8700e3e7c485 ("Soft RoCE driver")
>> Reported-by: syzbot+ab99dc4c6e961eed8b8e@syzkaller.appspotmail.com
>> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
>> ---
>>   drivers/infiniband/sw/rxe/rxe_qp.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
>> index 516bf9b95e48..f10b461b9963 100644
>> --- a/drivers/infiniband/sw/rxe/rxe_qp.c
>> +++ b/drivers/infiniband/sw/rxe/rxe_qp.c
>> @@ -797,7 +797,9 @@ static void rxe_qp_do_cleanup(struct work_struct *work)
>>   	rxe_cleanup_task(&qp->comp.task);
>>   
>>   	/* flush out any receive wr's or pending requests */
>> -	__rxe_do_task(&qp->req.task);
>> +	if (qp->req.task.func && qp->req.task.arg)
> func would be enough since they get set together.
Agreed

otherwise, looks good

Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>


> But, this is still fine since not performance critical.
>> +		__rxe_do_task(&qp->req.task);
>> +
>>   	if (qp->sq.queue) {
>>   		__rxe_do_task(&qp->comp.task);
>>   		__rxe_do_task(&qp->req.task);
> Reviewed-by: Bob Pearson <rpearsonhpe@gmail.com>

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

* Re: [PATCH 2/3] RDMA/rxe: Fix the error caused by qp->sk
  2022-08-22 19:01   ` Bob Pearson
@ 2022-08-23  5:53     ` lizhijian
  0 siblings, 0 replies; 12+ messages in thread
From: lizhijian @ 2022-08-23  5:53 UTC (permalink / raw)
  To: Bob Pearson, yanjun.zhu@linux.dev, jgg@ziepe.ca, leon@kernel.org,
	linux-rdma@vger.kernel.org, zyjzyj2000@gmail.com



On 23/08/2022 03:01, Bob Pearson wrote:
> On 8/21/22 20:16, yanjun.zhu@linux.dev wrote:
>> From: Zhu Yanjun <yanjun.zhu@linux.dev>
>>
>> When sock_create_kern in the function rxe_qp_init_req fails,
>> qp->sk is set to NULL.
>>
>> Then the function rxe_create_qp will call rxe_qp_do_cleanup
>> to handle allocated resource.
>>
>> Before handling qp->sk, this variable should be checked.
>>
>> Fixes: 8700e3e7c485 ("Soft RoCE driver")
>> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>


>> ---
>>   drivers/infiniband/sw/rxe/rxe_qp.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
>> index f10b461b9963..b229052ae91a 100644
>> --- a/drivers/infiniband/sw/rxe/rxe_qp.c
>> +++ b/drivers/infiniband/sw/rxe/rxe_qp.c
>> @@ -835,8 +835,10 @@ static void rxe_qp_do_cleanup(struct work_struct *work)
>>   
>>   	free_rd_atomic_resources(qp);
>>   
>> -	kernel_sock_shutdown(qp->sk, SHUT_RDWR);
>> -	sock_release(qp->sk);
>> +	if (qp->sk) {
>> +		kernel_sock_shutdown(qp->sk, SHUT_RDWR);
>> +		sock_release(qp->sk);
>> +	}
>>   }
>>   
>>   /* called when the last reference to the qp is dropped */
> Reviewed-by: Bob Pearson <rpearsonhpe@gmail.com>

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

* Re: [PATCH 3/3] RDMA/rxe: Remove the unused variable obj
  2022-08-22 19:06   ` Bob Pearson
@ 2022-08-23  5:54     ` lizhijian
  0 siblings, 0 replies; 12+ messages in thread
From: lizhijian @ 2022-08-23  5:54 UTC (permalink / raw)
  To: Bob Pearson, yanjun.zhu@linux.dev, jgg@ziepe.ca, leon@kernel.org,
	linux-rdma@vger.kernel.org, zyjzyj2000@gmail.com



On 23/08/2022 03:06, Bob Pearson wrote:
> On 8/21/22 20:16, yanjun.zhu@linux.dev wrote:
>> From: Zhu Yanjun <yanjun.zhu@linux.dev>
>>
>> The member variable obj in struct rxe_task is not needed.
>> So remove it to save memory.
>>
>> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>

Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>


>> ---
>>   drivers/infiniband/sw/rxe/rxe_qp.c   | 6 +++---
>>   drivers/infiniband/sw/rxe/rxe_task.c | 3 +--
>>   drivers/infiniband/sw/rxe/rxe_task.h | 3 +--
>>   3 files changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
>> index b229052ae91a..ee4f7a4a7e01 100644
>> --- a/drivers/infiniband/sw/rxe/rxe_qp.c
>> +++ b/drivers/infiniband/sw/rxe/rxe_qp.c
>> @@ -242,9 +242,9 @@ static int rxe_qp_init_req(struct rxe_dev *rxe, struct rxe_qp *qp,
>>   
>>   	skb_queue_head_init(&qp->req_pkts);
>>   
>> -	rxe_init_task(rxe, &qp->req.task, qp,
>> +	rxe_init_task(&qp->req.task, qp,
>>   		      rxe_requester, "req");
>> -	rxe_init_task(rxe, &qp->comp.task, qp,
>> +	rxe_init_task(&qp->comp.task, qp,
>>   		      rxe_completer, "comp");
>>   
>>   	qp->qp_timeout_jiffies = 0; /* Can't be set for UD/UC in modify_qp */
>> @@ -292,7 +292,7 @@ static int rxe_qp_init_resp(struct rxe_dev *rxe, struct rxe_qp *qp,
>>   
>>   	skb_queue_head_init(&qp->resp_pkts);
>>   
>> -	rxe_init_task(rxe, &qp->resp.task, qp,
>> +	rxe_init_task(&qp->resp.task, qp,
>>   		      rxe_responder, "resp");
>>   
>>   	qp->resp.opcode		= OPCODE_NONE;
>> diff --git a/drivers/infiniband/sw/rxe/rxe_task.c b/drivers/infiniband/sw/rxe/rxe_task.c
>> index 2248cf33d776..ec2b7de1c497 100644
>> --- a/drivers/infiniband/sw/rxe/rxe_task.c
>> +++ b/drivers/infiniband/sw/rxe/rxe_task.c
>> @@ -94,10 +94,9 @@ void rxe_do_task(struct tasklet_struct *t)
>>   	task->ret = ret;
>>   }
>>   
>> -int rxe_init_task(void *obj, struct rxe_task *task,
>> +int rxe_init_task(struct rxe_task *task,
>>   		  void *arg, int (*func)(void *), char *name)
>>   {
>> -	task->obj	= obj;
>>   	task->arg	= arg;
>>   	task->func	= func;
>>   	snprintf(task->name, sizeof(task->name), "%s", name);
>> diff --git a/drivers/infiniband/sw/rxe/rxe_task.h b/drivers/infiniband/sw/rxe/rxe_task.h
>> index 11d183fd3338..7f612a1c68a7 100644
>> --- a/drivers/infiniband/sw/rxe/rxe_task.h
>> +++ b/drivers/infiniband/sw/rxe/rxe_task.h
>> @@ -19,7 +19,6 @@ enum {
>>    * called again.
>>    */
>>   struct rxe_task {
>> -	void			*obj;
>>   	struct tasklet_struct	tasklet;
>>   	int			state;
>>   	spinlock_t		state_lock; /* spinlock for task state */
>> @@ -35,7 +34,7 @@ struct rxe_task {
>>    *	arg  => parameter to pass to fcn
>>    *	func => function to call until it returns != 0
>>    */
>> -int rxe_init_task(void *obj, struct rxe_task *task,
>> +int rxe_init_task(struct rxe_task *task,
>>   		  void *arg, int (*func)(void *), char *name);
>>   
>>   /* cleanup task */
> Looks OK to me. Not sure why that was ever there.
>
> Reviewed-by: Bob Pearson <rpearsonhpe@gmail.com>

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

* Re: [PATCH 0/3] Fixes for syzbot problem
  2022-08-22  1:16 [PATCH 0/3] Fixes for syzbot problem yanjun.zhu
                   ` (2 preceding siblings ...)
  2022-08-22  1:16 ` [PATCH 3/3] RDMA/rxe: Remove the unused variable obj yanjun.zhu
@ 2022-08-29 14:59 ` Yanjun Zhu
  2022-08-31  6:54 ` Leon Romanovsky
  4 siblings, 0 replies; 12+ messages in thread
From: Yanjun Zhu @ 2022-08-29 14:59 UTC (permalink / raw)
  To: yanjun.zhu, jgg, leon, linux-rdma, zyjzyj2000

在 2022/8/22 9:16, yanjun.zhu@linux.dev 写道:
> From: Zhu Yanjun <yanjun.zhu@linux.dev>
> 
> In the link:
> 
> https://syzkaller.appspot.com/bug?id=3efca8275142fbfdde6cf77e1b18b1d5c6794d76
> 
> a rxe problem occurred. I tried to reproduce this problem in local hosts.
> Finally I could reproduce this problem in local hosts.
> The commit ("RDMA/rxe: Fix "kernel NULL pointer dereference" error") tries to
> fix this problem.
> 
> After this syzbot problem disappeared, another problem appeared
> when qp->sk failed to initialized.
> The commit ("RDMA/rxe: Fix the error caused by qp->sk") tries to solve
> this problem.
> 
> When I delved into the source code to solve the above problems, I found
> that the member variable obj in struct rxe_task is not needed. So the
> commit ("RDMA/rxe: Remove the unused variable obj") removes this
> variable obj.
> 
> After the 3 commits, in locat hosts, the whole system can work well.
> 
> Zhu Yanjun (3):

Gently ping

Zhu Yanjun

>    RDMA/rxe: Fix "kernel NULL pointer dereference" error
>    RDMA/rxe: Fix the error caused by qp->sk
>    RDMA/rxe: Remove the unused variable obj
> 
>   drivers/infiniband/sw/rxe/rxe_qp.c   | 16 ++++++++++------
>   drivers/infiniband/sw/rxe/rxe_task.c |  3 +--
>   drivers/infiniband/sw/rxe/rxe_task.h |  3 +--
>   3 files changed, 12 insertions(+), 10 deletions(-)
> 


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

* Re: [PATCH 0/3] Fixes for syzbot problem
  2022-08-22  1:16 [PATCH 0/3] Fixes for syzbot problem yanjun.zhu
                   ` (3 preceding siblings ...)
  2022-08-29 14:59 ` [PATCH 0/3] Fixes for syzbot problem Yanjun Zhu
@ 2022-08-31  6:54 ` Leon Romanovsky
  4 siblings, 0 replies; 12+ messages in thread
From: Leon Romanovsky @ 2022-08-31  6:54 UTC (permalink / raw)
  To: yanjun.zhu; +Cc: jgg, linux-rdma, zyjzyj2000

On Sun, Aug 21, 2022 at 09:16:12PM -0400, yanjun.zhu@linux.dev wrote:
> From: Zhu Yanjun <yanjun.zhu@linux.dev>
> 
> In the link:
> 
> https://syzkaller.appspot.com/bug?id=3efca8275142fbfdde6cf77e1b18b1d5c6794d76
> 
> a rxe problem occurred. I tried to reproduce this problem in local hosts.
> Finally I could reproduce this problem in local hosts.
> The commit ("RDMA/rxe: Fix "kernel NULL pointer dereference" error") tries to
> fix this problem.
> 
> After this syzbot problem disappeared, another problem appeared
> when qp->sk failed to initialized.
> The commit ("RDMA/rxe: Fix the error caused by qp->sk") tries to solve
> this problem.
> 
> When I delved into the source code to solve the above problems, I found
> that the member variable obj in struct rxe_task is not needed. So the
> commit ("RDMA/rxe: Remove the unused variable obj") removes this
> variable obj.
> 
> After the 3 commits, in locat hosts, the whole system can work well.
> 
> Zhu Yanjun (3):
>   RDMA/rxe: Fix "kernel NULL pointer dereference" error
>   RDMA/rxe: Fix the error caused by qp->sk
>   RDMA/rxe: Remove the unused variable obj

Thanks, applied to -next with applied suggestion from Bob.

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

end of thread, other threads:[~2022-08-31  6:54 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-22  1:16 [PATCH 0/3] Fixes for syzbot problem yanjun.zhu
2022-08-22  1:16 ` [PATCH 1/3] RDMA/rxe: Fix "kernel NULL pointer dereference" error yanjun.zhu
2022-08-22 19:00   ` Bob Pearson
2022-08-23  5:43     ` lizhijian
2022-08-22  1:16 ` [PATCH 2/3] RDMA/rxe: Fix the error caused by qp->sk yanjun.zhu
2022-08-22 19:01   ` Bob Pearson
2022-08-23  5:53     ` lizhijian
2022-08-22  1:16 ` [PATCH 3/3] RDMA/rxe: Remove the unused variable obj yanjun.zhu
2022-08-22 19:06   ` Bob Pearson
2022-08-23  5:54     ` lizhijian
2022-08-29 14:59 ` [PATCH 0/3] Fixes for syzbot problem Yanjun Zhu
2022-08-31  6:54 ` Leon Romanovsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox