2023-11-30 08:32:45

by Shifeng Li

[permalink] [raw]
Subject: [PATCH v2] RDMA/irdma: Avoid free the non-cqp_request scratch

When creating ceq_0 during probing irdma, cqp.sc_cqp will be sent as
a cqp_request to cqp->sc_cqp.sq_ring. If the request is pending when
removing the irdma driver or unplugging its aux device, cqp.sc_cqp
will be dereferenced as wrong struct in irdma_free_pending_cqp_request().

crash> bt 3669
PID: 3669 TASK: ffff88aef892c000 CPU: 28 COMMAND: "kworker/28:0"
#0 [fffffe0000549e38] crash_nmi_callback at ffffffff810e3a34
#1 [fffffe0000549e40] nmi_handle at ffffffff810788b2
#2 [fffffe0000549ea0] default_do_nmi at ffffffff8107938f
#3 [fffffe0000549eb8] do_nmi at ffffffff81079582
#4 [fffffe0000549ef0] end_repeat_nmi at ffffffff82e016b4
[exception RIP: native_queued_spin_lock_slowpath+1291]
RIP: ffffffff8127e72b RSP: ffff88aa841ef778 RFLAGS: 00000046
RAX: 0000000000000000 RBX: ffff88b01f849700 RCX: ffffffff8127e47e
RDX: 0000000000000000 RSI: 0000000000000004 RDI: ffffffff83857ec0
RBP: ffff88afe3e4efc8 R8: ffffed15fc7c9dfa R9: ffffed15fc7c9dfa
R10: 0000000000000001 R11: ffffed15fc7c9df9 R12: 0000000000740000
R13: ffff88b01f849708 R14: 0000000000000003 R15: ffffed1603f092e1
ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0000
--- <NMI exception stack> ---
#5 [ffff88aa841ef778] native_queued_spin_lock_slowpath at ffffffff8127e72b
#6 [ffff88aa841ef7b0] _raw_spin_lock_irqsave at ffffffff82c22aa4
#7 [ffff88aa841ef7c8] __wake_up_common_lock at ffffffff81257363
#8 [ffff88aa841ef888] irdma_free_pending_cqp_request at ffffffffa0ba12cc [irdma]
#9 [ffff88aa841ef958] irdma_cleanup_pending_cqp_op at ffffffffa0ba1469 [irdma]
#10 [ffff88aa841ef9c0] irdma_ctrl_deinit_hw at ffffffffa0b2989f [irdma]
#11 [ffff88aa841efa28] irdma_remove at ffffffffa0b252df [irdma]
#12 [ffff88aa841efae8] auxiliary_bus_remove at ffffffff8219afdb
#13 [ffff88aa841efb00] device_release_driver_internal at ffffffff821882e6
#14 [ffff88aa841efb38] bus_remove_device at ffffffff82184278
#15 [ffff88aa841efb88] device_del at ffffffff82179d23
#16 [ffff88aa841efc48] ice_unplug_aux_dev at ffffffffa0eb1c14 [ice]
#17 [ffff88aa841efc68] ice_service_task at ffffffffa0d88201 [ice]
#18 [ffff88aa841efde8] process_one_work at ffffffff811c589a
#19 [ffff88aa841efe60] worker_thread at ffffffff811c71ff
#20 [ffff88aa841eff10] kthread at ffffffff811d87a0
#21 [ffff88aa841eff50] ret_from_fork at ffffffff82e0022f

Fixes: 44d9e52977a1 ("RDMA/irdma: Implement device initialization definitions")
Suggested-by: Leon Romanovsky <[email protected]>
Signed-off-by: Shifeng Li <[email protected]>
---
drivers/infiniband/hw/irdma/hw.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

---
v1->v2: replace fix solution and massage the git log.

diff --git a/drivers/infiniband/hw/irdma/hw.c b/drivers/infiniband/hw/irdma/hw.c
index 8fa7e4a18e73..df6259c73d28 100644
--- a/drivers/infiniband/hw/irdma/hw.c
+++ b/drivers/infiniband/hw/irdma/hw.c
@@ -1180,7 +1180,6 @@ static int irdma_create_ceq(struct irdma_pci_f *rf, struct irdma_ceq *iwceq,
int status;
struct irdma_ceq_init_info info = {};
struct irdma_sc_dev *dev = &rf->sc_dev;
- u64 scratch;
u32 ceq_size;

info.ceq_id = ceq_id;
@@ -1201,14 +1200,13 @@ static int irdma_create_ceq(struct irdma_pci_f *rf, struct irdma_ceq *iwceq,
iwceq->sc_ceq.ceq_id = ceq_id;
info.dev = dev;
info.vsi = vsi;
- scratch = (uintptr_t)&rf->cqp.sc_cqp;
status = irdma_sc_ceq_init(&iwceq->sc_ceq, &info);
if (!status) {
if (dev->ceq_valid)
status = irdma_cqp_ceq_cmd(&rf->sc_dev, &iwceq->sc_ceq,
IRDMA_OP_CEQ_CREATE);
else
- status = irdma_sc_cceq_create(&iwceq->sc_ceq, scratch);
+ status = irdma_sc_cceq_create(&iwceq->sc_ceq, 0);
}

if (status) {
--
2.25.1


2023-11-30 08:35:16

by Shifeng Li

[permalink] [raw]
Subject: Re: [PATCH v2] RDMA/irdma: Avoid free the non-cqp_request scratch

On 2023/11/30 16:14, Shifeng Li wrote:
> When creating ceq_0 during probing irdma, cqp.sc_cqp will be sent as
> a cqp_request to cqp->sc_cqp.sq_ring. If the request is pending when
> removing the irdma driver or unplugging its aux device, cqp.sc_cqp
> will be dereferenced as wrong struct in irdma_free_pending_cqp_request().
>
> crash> bt 3669
> PID: 3669 TASK: ffff88aef892c000 CPU: 28 COMMAND: "kworker/28:0"
> #0 [fffffe0000549e38] crash_nmi_callback at ffffffff810e3a34
> #1 [fffffe0000549e40] nmi_handle at ffffffff810788b2
> #2 [fffffe0000549ea0] default_do_nmi at ffffffff8107938f
> #3 [fffffe0000549eb8] do_nmi at ffffffff81079582
> #4 [fffffe0000549ef0] end_repeat_nmi at ffffffff82e016b4
> [exception RIP: native_queued_spin_lock_slowpath+1291]
> RIP: ffffffff8127e72b RSP: ffff88aa841ef778 RFLAGS: 00000046
> RAX: 0000000000000000 RBX: ffff88b01f849700 RCX: ffffffff8127e47e
> RDX: 0000000000000000 RSI: 0000000000000004 RDI: ffffffff83857ec0
> RBP: ffff88afe3e4efc8 R8: ffffed15fc7c9dfa R9: ffffed15fc7c9dfa
> R10: 0000000000000001 R11: ffffed15fc7c9df9 R12: 0000000000740000
> R13: ffff88b01f849708 R14: 0000000000000003 R15: ffffed1603f092e1
> ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0000
> --- <NMI exception stack> ---
> #5 [ffff88aa841ef778] native_queued_spin_lock_slowpath at ffffffff8127e72b
> #6 [ffff88aa841ef7b0] _raw_spin_lock_irqsave at ffffffff82c22aa4
> #7 [ffff88aa841ef7c8] __wake_up_common_lock at ffffffff81257363
> #8 [ffff88aa841ef888] irdma_free_pending_cqp_request at ffffffffa0ba12cc [irdma]
> #9 [ffff88aa841ef958] irdma_cleanup_pending_cqp_op at ffffffffa0ba1469 [irdma]
> #10 [ffff88aa841ef9c0] irdma_ctrl_deinit_hw at ffffffffa0b2989f [irdma]
> #11 [ffff88aa841efa28] irdma_remove at ffffffffa0b252df [irdma]
> #12 [ffff88aa841efae8] auxiliary_bus_remove at ffffffff8219afdb
> #13 [ffff88aa841efb00] device_release_driver_internal at ffffffff821882e6
> #14 [ffff88aa841efb38] bus_remove_device at ffffffff82184278
> #15 [ffff88aa841efb88] device_del at ffffffff82179d23
> #16 [ffff88aa841efc48] ice_unplug_aux_dev at ffffffffa0eb1c14 [ice]
> #17 [ffff88aa841efc68] ice_service_task at ffffffffa0d88201 [ice]
> #18 [ffff88aa841efde8] process_one_work at ffffffff811c589a
> #19 [ffff88aa841efe60] worker_thread at ffffffff811c71ff
> #20 [ffff88aa841eff10] kthread at ffffffff811d87a0
> #21 [ffff88aa841eff50] ret_from_fork at ffffffff82e0022f
>
> Fixes: 44d9e52977a1 ("RDMA/irdma: Implement device initialization definitions")
> Suggested-by: Leon Romanovsky <[email protected]>

The Suggested-by should be "Ismail, Mustafa" <[email protected]>.
Could you help me correct it?

Thanks.

> Signed-off-by: Shifeng Li <[email protected]>
> ---
> drivers/infiniband/hw/irdma/hw.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> ---
> v1->v2: replace fix solution and massage the git log.
>
> diff --git a/drivers/infiniband/hw/irdma/hw.c b/drivers/infiniband/hw/irdma/hw.c
> index 8fa7e4a18e73..df6259c73d28 100644
> --- a/drivers/infiniband/hw/irdma/hw.c
> +++ b/drivers/infiniband/hw/irdma/hw.c
> @@ -1180,7 +1180,6 @@ static int irdma_create_ceq(struct irdma_pci_f *rf, struct irdma_ceq *iwceq,
> int status;
> struct irdma_ceq_init_info info = {};
> struct irdma_sc_dev *dev = &rf->sc_dev;
> - u64 scratch;
> u32 ceq_size;
>
> info.ceq_id = ceq_id;
> @@ -1201,14 +1200,13 @@ static int irdma_create_ceq(struct irdma_pci_f *rf, struct irdma_ceq *iwceq,
> iwceq->sc_ceq.ceq_id = ceq_id;
> info.dev = dev;
> info.vsi = vsi;
> - scratch = (uintptr_t)&rf->cqp.sc_cqp;
> status = irdma_sc_ceq_init(&iwceq->sc_ceq, &info);
> if (!status) {
> if (dev->ceq_valid)
> status = irdma_cqp_ceq_cmd(&rf->sc_dev, &iwceq->sc_ceq,
> IRDMA_OP_CEQ_CREATE);
> else
> - status = irdma_sc_cceq_create(&iwceq->sc_ceq, scratch);
> + status = irdma_sc_cceq_create(&iwceq->sc_ceq, 0);
> }
>
> if (status) {

2023-11-30 15:47:02

by Shiraz Saleem

[permalink] [raw]
Subject: RE: [PATCH v2] RDMA/irdma: Avoid free the non-cqp_request scratch

> Subject: [PATCH v2] RDMA/irdma: Avoid free the non-cqp_request scratch
>
> When creating ceq_0 during probing irdma, cqp.sc_cqp will be sent as a
> cqp_request to cqp->sc_cqp.sq_ring. If the request is pending when removing the
> irdma driver or unplugging its aux device, cqp.sc_cqp will be dereferenced as
> wrong struct in irdma_free_pending_cqp_request().
>
> crash> bt 3669
> PID: 3669 TASK: ffff88aef892c000 CPU: 28 COMMAND: "kworker/28:0"
> #0 [fffffe0000549e38] crash_nmi_callback at ffffffff810e3a34
> #1 [fffffe0000549e40] nmi_handle at ffffffff810788b2
> #2 [fffffe0000549ea0] default_do_nmi at ffffffff8107938f
> #3 [fffffe0000549eb8] do_nmi at ffffffff81079582
> #4 [fffffe0000549ef0] end_repeat_nmi at ffffffff82e016b4
> [exception RIP: native_queued_spin_lock_slowpath+1291]
> RIP: ffffffff8127e72b RSP: ffff88aa841ef778 RFLAGS: 00000046
> RAX: 0000000000000000 RBX: ffff88b01f849700 RCX: ffffffff8127e47e
> RDX: 0000000000000000 RSI: 0000000000000004 RDI: ffffffff83857ec0
> RBP: ffff88afe3e4efc8 R8: ffffed15fc7c9dfa R9: ffffed15fc7c9dfa
> R10: 0000000000000001 R11: ffffed15fc7c9df9 R12: 0000000000740000
> R13: ffff88b01f849708 R14: 0000000000000003 R15: ffffed1603f092e1
> ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0000
> --- <NMI exception stack> ---
> #5 [ffff88aa841ef778] native_queued_spin_lock_slowpath at ffffffff8127e72b
> #6 [ffff88aa841ef7b0] _raw_spin_lock_irqsave at ffffffff82c22aa4
> #7 [ffff88aa841ef7c8] __wake_up_common_lock at ffffffff81257363
> #8 [ffff88aa841ef888] irdma_free_pending_cqp_request at ffffffffa0ba12cc
> [irdma]
> #9 [ffff88aa841ef958] irdma_cleanup_pending_cqp_op at ffffffffa0ba1469 [irdma]
> #10 [ffff88aa841ef9c0] irdma_ctrl_deinit_hw at ffffffffa0b2989f [irdma]
> #11 [ffff88aa841efa28] irdma_remove at ffffffffa0b252df [irdma]
> #12 [ffff88aa841efae8] auxiliary_bus_remove at ffffffff8219afdb
> #13 [ffff88aa841efb00] device_release_driver_internal at ffffffff821882e6
> #14 [ffff88aa841efb38] bus_remove_device at ffffffff82184278
> #15 [ffff88aa841efb88] device_del at ffffffff82179d23
> #16 [ffff88aa841efc48] ice_unplug_aux_dev at ffffffffa0eb1c14 [ice]
> #17 [ffff88aa841efc68] ice_service_task at ffffffffa0d88201 [ice]
> #18 [ffff88aa841efde8] process_one_work at ffffffff811c589a
> #19 [ffff88aa841efe60] worker_thread at ffffffff811c71ff
> #20 [ffff88aa841eff10] kthread at ffffffff811d87a0
> #21 [ffff88aa841eff50] ret_from_fork at ffffffff82e0022f
>
> Fixes: 44d9e52977a1 ("RDMA/irdma: Implement device initialization definitions")
> Suggested-by: Leon Romanovsky <[email protected]>
> Signed-off-by: Shifeng Li <[email protected]>
> ---
> drivers/infiniband/hw/irdma/hw.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>

Other than the suggested-by tag being incorrect, looks good to me.

Reviewed-by: Shiraz Saleem <[email protected]>

2023-12-05 00:07:01

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH v2] RDMA/irdma: Avoid free the non-cqp_request scratch

On Thu, Nov 30, 2023 at 12:14:15AM -0800, Shifeng Li wrote:
> When creating ceq_0 during probing irdma, cqp.sc_cqp will be sent as
> a cqp_request to cqp->sc_cqp.sq_ring. If the request is pending when
> removing the irdma driver or unplugging its aux device, cqp.sc_cqp
> will be dereferenced as wrong struct in irdma_free_pending_cqp_request().
>
> crash> bt 3669
> PID: 3669 TASK: ffff88aef892c000 CPU: 28 COMMAND: "kworker/28:0"
> #0 [fffffe0000549e38] crash_nmi_callback at ffffffff810e3a34
> #1 [fffffe0000549e40] nmi_handle at ffffffff810788b2
> #2 [fffffe0000549ea0] default_do_nmi at ffffffff8107938f
> #3 [fffffe0000549eb8] do_nmi at ffffffff81079582
> #4 [fffffe0000549ef0] end_repeat_nmi at ffffffff82e016b4
> [exception RIP: native_queued_spin_lock_slowpath+1291]
> RIP: ffffffff8127e72b RSP: ffff88aa841ef778 RFLAGS: 00000046
> RAX: 0000000000000000 RBX: ffff88b01f849700 RCX: ffffffff8127e47e
> RDX: 0000000000000000 RSI: 0000000000000004 RDI: ffffffff83857ec0
> RBP: ffff88afe3e4efc8 R8: ffffed15fc7c9dfa R9: ffffed15fc7c9dfa
> R10: 0000000000000001 R11: ffffed15fc7c9df9 R12: 0000000000740000
> R13: ffff88b01f849708 R14: 0000000000000003 R15: ffffed1603f092e1
> ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0000
> --- <NMI exception stack> ---
> #5 [ffff88aa841ef778] native_queued_spin_lock_slowpath at ffffffff8127e72b
> #6 [ffff88aa841ef7b0] _raw_spin_lock_irqsave at ffffffff82c22aa4
> #7 [ffff88aa841ef7c8] __wake_up_common_lock at ffffffff81257363
> #8 [ffff88aa841ef888] irdma_free_pending_cqp_request at ffffffffa0ba12cc [irdma]
> #9 [ffff88aa841ef958] irdma_cleanup_pending_cqp_op at ffffffffa0ba1469 [irdma]
> #10 [ffff88aa841ef9c0] irdma_ctrl_deinit_hw at ffffffffa0b2989f [irdma]
> #11 [ffff88aa841efa28] irdma_remove at ffffffffa0b252df [irdma]
> #12 [ffff88aa841efae8] auxiliary_bus_remove at ffffffff8219afdb
> #13 [ffff88aa841efb00] device_release_driver_internal at ffffffff821882e6
> #14 [ffff88aa841efb38] bus_remove_device at ffffffff82184278
> #15 [ffff88aa841efb88] device_del at ffffffff82179d23
> #16 [ffff88aa841efc48] ice_unplug_aux_dev at ffffffffa0eb1c14 [ice]
> #17 [ffff88aa841efc68] ice_service_task at ffffffffa0d88201 [ice]
> #18 [ffff88aa841efde8] process_one_work at ffffffff811c589a
> #19 [ffff88aa841efe60] worker_thread at ffffffff811c71ff
> #20 [ffff88aa841eff10] kthread at ffffffff811d87a0
> #21 [ffff88aa841eff50] ret_from_fork at ffffffff82e0022f
>
> Fixes: 44d9e52977a1 ("RDMA/irdma: Implement device initialization definitions")
> Suggested-by: Leon Romanovsky <[email protected]>
> Signed-off-by: Shifeng Li <[email protected]>
> Reviewed-by: Shiraz Saleem <[email protected]>
> ---
> drivers/infiniband/hw/irdma/hw.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)

Applied to for-rc, thanks

Jason