Fixes for syzbot problem
Subsystems (2)
cover Fixes for syzbot problem
From: Zhu Yanjun <[email protected]>
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(-)
Signature · 1 line
2.31.1
PATCH 1/3 RDMA/rxe: Fix "kernel NULL pointer dereference" error
From: Zhu Yanjun <[email protected]>
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.
--- drivers/infiniband/sw/rxe/rxe_qp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/infiniband/sw/rxe/rxe_qp.c +3 -1
@@ -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);
Signature · 1 line
2.31.1
Re: [PATCH 1/3] RDMA/rxe: Fix "kernel NULL pointer dereference" error
Quoted 23 lines · On 8/21/22 20:16, [email protected] wrote:
From: Zhu Yanjun <[email protected]>
When
rxe_queue_initin the functionrxe_qp_init_reqfails, bothqp->req.task.funcandqp->req.task.argare not initialized.Because of creation of qp fails, the function
rxe_create_qpwill callrxe_qp_do_cleanupto handle allocated resource.Before calling
__rxe_do_task, bothqp->req.task.funcandqp->req.task.argshould be checked.Fixes 8700e3e7c485 ("Soft RoCE driver")Reported-by [email protected]Signed-off-by Zhu Yanjun <[email protected]>--- drivers/infiniband/sw/rxe/rxe_qp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)drivers/infiniband/sw/rxe/rxe_qp.c +0
-0
drivers/infiniband/sw/rxe/rxe_qp.c static void rxe_qp_do_cleanup(struct work_struct *work) · line 800
Quoted 5 lines
drivers/infiniband/sw/rxe/rxe_qp.c +1
-1@@ -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.
drivers/infiniband/sw/rxe/rxe_qp.c static void rxe_qp_do_cleanup(struct work_struct *work) · line 805
Quoted 5 lines
drivers/infiniband/sw/rxe/rxe_qp.c +2
-0+__rxe_do_task (&qp->req.task); +if (qp->sq.queue) {__rxe_do_task (&qp->comp.task);__rxe_do_task (&qp->req.task);
[email protected]
Reviewed-byRe: [PATCH 1/3] RDMA/rxe: Fix "kernel NULL pointer dereference" error
Quoted 26 lines · Bob Pearson
Quoted 25 lines · On 8/21/22 20:16, [email protected] wrote:
From: Zhu Yanjun <[email protected]>
When
rxe_queue_initin the functionrxe_qp_init_reqfails, bothqp->req.task.funcandqp->req.task.argare not initialized.Because of creation of qp fails, the function
rxe_create_qpwill callrxe_qp_do_cleanupto handle allocated resource.Before calling
__rxe_do_task, bothqp->req.task.funcandqp->req.task.argshould be checked.Fixes 8700e3e7c485 ("Soft RoCE driver")Reported-by [email protected]Signed-off-by Zhu Yanjun <[email protected]>--- drivers/infiniband/sw/rxe/rxe_qp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)drivers/infiniband/sw/rxe/rxe_qp.c +0
-0@@ -797,7 +797,9 @@ static void rxe_qp_do_cleanup(struct work_struct *work)rxe_cleanup_task (&qp->comp.task);
Quoted 4 lines
drivers/infiniband/sw/rxe/rxe_qp.c static void rxe_qp_do_cleanup(struct work_struct *work) · line 800
Quoted 3 lines
drivers/infiniband/sw/rxe/rxe_qp.c +1
-1/* 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
Quoted 7 lines
But, this is still fine since not performance critical.
drivers/infiniband/sw/rxe/rxe_qp.c static void rxe_qp_do_cleanup(struct work_struct *work) · line 805
Quoted 5 lines
drivers/infiniband/sw/rxe/rxe_qp.c +2
-0+__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 <[email protected]>
PATCH 2/3 RDMA/rxe: Fix the error caused by qp->sk
In reply to [email protected]From: Zhu Yanjun <[email protected]>
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.
--- drivers/infiniband/sw/rxe/rxe_qp.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/infiniband/sw/rxe/rxe_qp.c +4 -2
@@ -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 */
Signature · 1 line
2.31.1
Re: [PATCH 2/3] RDMA/rxe: Fix the error caused by qp->sk
Quoted 23 lines · On 8/21/22 20:16, [email protected] wrote:
From: Zhu Yanjun <[email protected]>
When
sock_create_kernin the functionrxe_qp_init_reqfails,qp->skis set to NULL.Then the function
rxe_create_qpwill callrxe_qp_do_cleanupto handle allocated resource.Before handling
qp->sk, this variable should be checked.Fixes 8700e3e7c485 ("Soft RoCE driver")Signed-off-by Zhu Yanjun <[email protected]>--- drivers/infiniband/sw/rxe/rxe_qp.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)drivers/infiniband/sw/rxe/rxe_qp.c +0
-0@@ -835,8 +835,10 @@ static void rxe_qp_do_cleanup(struct work_struct *work)free_rd_atomic_resources (qp);
drivers/infiniband/sw/rxe/rxe_qp.c static void rxe_qp_do_cleanup(struct work_struct *work) · line 844
Quoted 10 lines
drivers/infiniband/sw/rxe/rxe_qp.c +4
-2-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 */
[email protected]
Reviewed-byRe: [PATCH 2/3] RDMA/rxe: Fix the error caused by qp->sk
Quoted 6 lines · Bob Pearson
Quoted 5 lines · On 8/21/22 20:16, [email protected] wrote:
From: Zhu Yanjun <[email protected]>
When
sock_create_kernin the functionrxe_qp_init_reqfails,qp->skis set to NULL.
Quoted 7 lines
Quoted 7 lines
Then the function
rxe_create_qpwill callrxe_qp_do_cleanupto handle allocated resource.Before handling
qp->sk, this variable should be checked.Fixes 8700e3e7c485 ("Soft RoCE driver")Signed-off-by Zhu Yanjun <[email protected]>
Quoted 22 lines
Quoted 11 lines
--- drivers/infiniband/sw/rxe/rxe_qp.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)drivers/infiniband/sw/rxe/rxe_qp.c +0
-0@@ -835,8 +835,10 @@ static void rxe_qp_do_cleanup(struct work_struct *work)free_rd_atomic_resources (qp);drivers/infiniband/sw/rxe/rxe_qp.c static void rxe_qp_do_cleanup(struct work_struct *work) · line 844
Quoted 10 lines
drivers/infiniband/sw/rxe/rxe_qp.c +4
-2-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 <[email protected]>
PATCH 3/3 RDMA/rxe: Remove the unused variable obj
In reply to [email protected]From: Zhu Yanjun <[email protected]>
The member variable obj in struct rxe_task is not needed. So remove it to save memory.
--- 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(-)
drivers/infiniband/sw/rxe/rxe_qp.c +3 -3
@@ -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 ;
drivers/infiniband/sw/rxe/rxe_task.c +1 -2
@@ -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);
drivers/infiniband/sw/rxe/rxe_task.h +1 -2
@@ -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 */
Signature · 1 line
2.31.1
Re: [PATCH 3/3] RDMA/rxe: Remove the unused variable obj
Quoted 66 lines · On 8/21/22 20:16, [email protected] wrote:
From: Zhu Yanjun <[email protected]>
The member variable obj in struct
rxe_taskis not needed. So remove it to save memory.Signed-off-by Zhu Yanjun <[email protected]>--- 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(-)drivers/infiniband/sw/rxe/rxe_qp.c +3
-3@@ -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 ;drivers/infiniband/sw/rxe/rxe_task.c +1
-2@@ -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);drivers/infiniband/sw/rxe/rxe_task.h +0
-1@@ -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 */
drivers/infiniband/sw/rxe/rxe_task.h struct rxe_task { · line 40
Quoted 8 lines
drivers/infiniband/sw/rxe/rxe_task.h +1
-1@@ -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.
[email protected]
Reviewed-byRe: [PATCH 3/3] RDMA/rxe: Remove the unused variable obj
Quoted 7 lines · Bob Pearson
Quoted 6 lines · On 8/21/22 20:16, [email protected] wrote:
From: Zhu Yanjun <[email protected]>
The member variable obj in struct
rxe_taskis not needed. So remove it to save memory.Signed-off-by Zhu Yanjun <[email protected]>
Quoted 71 lines
Quoted 60 lines
--- 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(-)drivers/infiniband/sw/rxe/rxe_qp.c +3
-3@@ -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 ;drivers/infiniband/sw/rxe/rxe_task.c +1
-2@@ -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);drivers/infiniband/sw/rxe/rxe_task.h +0
-1@@ -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 */ drivers/infiniband/sw/rxe/rxe_task.h struct rxe_task { · line 40
Quoted 8 lines
drivers/infiniband/sw/rxe/rxe_task.h +1
-1@@ -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 <[email protected]>
Quoted 16 lines · 在 2022/8/22 9:16, [email protected] 写道:
From: Zhu Yanjun <[email protected]>
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->skfailed to initialized.
The commit ("RDMA/rxe: Fix the error caused byqp->sk") tries to solve this problem.
Quoted 8 lines
When I delved into the source code to solve the above problems, I found that the member variable obj in struct
rxe_taskis 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
Quoted 9 lines
RDMA/rxe: Fix "kernel NULL pointer dereference" error RDMA/rxe: Fix the error caused by
qp->skRDMA/rxe: Remove the unused variable objdrivers/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(-)
Leon Romanovsky
AppliedQuoted 21 lines · On Sun, Aug 21, 2022 at 09:16:12PM -0400, [email protected] wrote:
From: Zhu Yanjun <[email protected]>
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->skfailed to initialized.
The commit ("RDMA/rxe: Fix the error caused byqp->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_taskis not needed. So the commit ("RDMA/rxe: Remove the unused variable obj") removes this variable obj.
Quoted 6 lines
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 byqp->skRDMA/rxe: Remove the unused variable obj
Thanks, applied to -next with applied suggestion from Bob.