[PATCH v2] RDMA/cma: Fix workqueue crash in cma_netevent_work_handler

Sharath Srinivasan posted 1 patch 8 months, 3 weeks ago
drivers/infiniband/core/cma.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
[PATCH v2] RDMA/cma: Fix workqueue crash in cma_netevent_work_handler
Posted by Sharath Srinivasan 8 months, 3 weeks ago
struct rdma_cm_id has member "struct work_struct net_work"
that is reused for enqueuing cma_netevent_work_handler()s
onto cma_wq.

Below crash[1] can occur if more than one call to
cma_netevent_callback() occurs in quick succession,
which further enqueues cma_netevent_work_handler()s for the
same rdma_cm_id, overwriting any previously queued work-item(s)
that was just scheduled to run i.e. there is no guarantee
the queued work item may run between two successive calls
to cma_netevent_callback() and the 2nd INIT_WORK would overwrite
the 1st work item (for the same rdma_cm_id), despite grabbing
id_table_lock during enqueue.

Also drgn analysis [2] indicates the work item was likely overwritten.

Fix this by moving the INIT_WORK() to __rdma_create_id(),
so that it doesn't race with any existing queue_work() or
its worker thread.

[1] Trimmed crash stack:
=============================================
BUG: kernel NULL pointer dereference, address: 0000000000000008
kworker/u256:6 ... 6.12.0-0...
Workqueue:  cma_netevent_work_handler [rdma_cm] (rdma_cm)
RIP: 0010:process_one_work+0xba/0x31a
Call Trace:
 worker_thread+0x266/0x3a0
 kthread+0xcf/0x100
 ret_from_fork+0x31/0x50
 ret_from_fork_asm+0x1a/0x30
=============================================

[2] drgn crash analysis:

>>> trace = prog.crashed_thread().stack_trace()
>>> trace
(0)  crash_setup_regs (./arch/x86/include/asm/kexec.h:111:15)
(1)  __crash_kexec (kernel/crash_core.c:122:4)
(2)  panic (kernel/panic.c:399:3)
(3)  oops_end (arch/x86/kernel/dumpstack.c:382:3)
...
(8)  process_one_work (kernel/workqueue.c:3168:2)
(9)  process_scheduled_works (kernel/workqueue.c:3310:3)
(10) worker_thread (kernel/workqueue.c:3391:4)
(11) kthread (kernel/kthread.c:389:9)

Line workqueue.c:3168 for this kernel version is in process_one_work():
3168	strscpy(worker->desc, pwq->wq->name, WORKER_DESC_LEN);

>>> trace[8]["work"]
*(struct work_struct *)0xffff92577d0a21d8 = {
	.data = (atomic_long_t){
		.counter = (s64)536870912,    <=== Note
	},
	.entry = (struct list_head){
		.next = (struct list_head *)0xffff924d075924c0,
		.prev = (struct list_head *)0xffff924d075924c0,
	},
	.func = (work_func_t)cma_netevent_work_handler+0x0 = 0xffffffffc2cec280,
}

Suspicion is that pwq is NULL:
>>> trace[8]["pwq"]
(struct pool_workqueue *)<absent>

In process_one_work(), pwq is assigned from:
struct pool_workqueue *pwq = get_work_pwq(work);

and get_work_pwq() is:
static struct pool_workqueue *get_work_pwq(struct work_struct *work)
{
 	unsigned long data = atomic_long_read(&work->data);

 	if (data & WORK_STRUCT_PWQ)
 		return work_struct_pwq(data);
 	else
 		return NULL;
}

WORK_STRUCT_PWQ is 0x4:
>>> print(repr(prog['WORK_STRUCT_PWQ']))
Object(prog, 'enum work_flags', value=4)

But work->data is 536870912 which is 0x20000000.
So, get_work_pwq() returns NULL and we crash in process_one_work():
3168	strscpy(worker->desc, pwq->wq->name, WORKER_DESC_LEN);
=============================================

Fixes: 925d046e7e52 ("RDMA/core: Add a netevent notifier to cma")
Cc: stable@vger.kernel.org
Co-developed-by: Håkon Bugge <haakon.bugge@oracle.com>
Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
Signed-off-by: Sharath Srinivasan <sharath.srinivasan@oracle.com>
---
v1->v2 cc:stable@vger.kernel.org
---
 drivers/infiniband/core/cma.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 91db10515d74..176d0b3e4488 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -72,6 +72,8 @@ static const char * const cma_events[] = {
 static void cma_iboe_set_mgid(struct sockaddr *addr, union ib_gid *mgid,
 			      enum ib_gid_type gid_type);
 
+static void cma_netevent_work_handler(struct work_struct *_work);
+
 const char *__attribute_const__ rdma_event_msg(enum rdma_cm_event_type event)
 {
 	size_t index = event;
@@ -1033,6 +1035,7 @@ __rdma_create_id(struct net *net, rdma_cm_event_handler event_handler,
 	get_random_bytes(&id_priv->seq_num, sizeof id_priv->seq_num);
 	id_priv->id.route.addr.dev_addr.net = get_net(net);
 	id_priv->seq_num &= 0x00ffffff;
+	INIT_WORK(&id_priv->id.net_work, cma_netevent_work_handler);
 
 	rdma_restrack_new(&id_priv->res, RDMA_RESTRACK_CM_ID);
 	if (parent)
@@ -5227,7 +5230,6 @@ static int cma_netevent_callback(struct notifier_block *self,
 		if (!memcmp(current_id->id.route.addr.dev_addr.dst_dev_addr,
 			   neigh->ha, ETH_ALEN))
 			continue;
-		INIT_WORK(&current_id->id.net_work, cma_netevent_work_handler);
 		cma_id_get(current_id);
 		queue_work(cma_wq, &current_id->id.net_work);
 	}
-- 
2.39.5 (Apple Git-154)

Re: [PATCH v2] RDMA/cma: Fix workqueue crash in cma_netevent_work_handler
Posted by Leon Romanovsky 8 months, 1 week ago
On Wed, 26 Mar 2025 14:05:32 -0700, Sharath Srinivasan wrote:
> struct rdma_cm_id has member "struct work_struct net_work"
> that is reused for enqueuing cma_netevent_work_handler()s
> onto cma_wq.
> 
> Below crash[1] can occur if more than one call to
> cma_netevent_callback() occurs in quick succession,
> which further enqueues cma_netevent_work_handler()s for the
> same rdma_cm_id, overwriting any previously queued work-item(s)
> that was just scheduled to run i.e. there is no guarantee
> the queued work item may run between two successive calls
> to cma_netevent_callback() and the 2nd INIT_WORK would overwrite
> the 1st work item (for the same rdma_cm_id), despite grabbing
> id_table_lock during enqueue.
> 
> [...]

Applied, thanks!

[1/1] RDMA/cma: Fix workqueue crash in cma_netevent_work_handler
      https://git.kernel.org/rdma/rdma/c/052996ebc39e3e

Best regards,
-- 
Leon Romanovsky <leon@kernel.org>
Re: [PATCH v2] RDMA/cma: Fix workqueue crash in cma_netevent_work_handler
Posted by Sharath Srinivasan 8 months, 1 week ago

On 2025-04-09 4:20 a.m., Leon Romanovsky wrote:
> 
> On Wed, 26 Mar 2025 14:05:32 -0700, Sharath Srinivasan wrote:
>> struct rdma_cm_id has member "struct work_struct net_work"
>> that is reused for enqueuing cma_netevent_work_handler()s
>> onto cma_wq.
>>
>> Below crash[1] can occur if more than one call to
>> cma_netevent_callback() occurs in quick succession,
>> which further enqueues cma_netevent_work_handler()s for the
>> same rdma_cm_id, overwriting any previously queued work-item(s)
>> that was just scheduled to run i.e. there is no guarantee
>> the queued work item may run between two successive calls
>> to cma_netevent_callback() and the 2nd INIT_WORK would overwrite
>> the 1st work item (for the same rdma_cm_id), despite grabbing
>> id_table_lock during enqueue.
>>
>> [...]
> 
> Applied, thanks!
> 
> [1/1] RDMA/cma: Fix workqueue crash in cma_netevent_work_handler
>       https://urldefense.com/v3/__https://git.kernel.org/rdma/rdma/c/052996ebc39e3e__;!!ACWV5N9M2RV99hQ!LLGepcc45JwMb6s1VnxWb4Y2hyAGH7AZaE1BoDtVREqdciSOHOIlKiu4RvKD9bL5NYAEtIMG6oVL2cU7PQ$ 
> 
> Best regards,

Thank you, Leon and Patrisious!

Best Regards,
Sharath
Re: [PATCH v2] RDMA/cma: Fix workqueue crash in cma_netevent_work_handler
Posted by Haakon Bugge 8 months, 1 week ago
> struct rdma_cm_id has member "struct work_struct net_work"
> that is reused for enqueuing cma_netevent_work_handler()s
> onto cma_wq.
> 
> Below crash[1] can occur if more than one call to
> cma_netevent_callback() occurs in quick succession,
> which further enqueues cma_netevent_work_handler()s for the
> same rdma_cm_id, overwriting any previously queued work-item(s)
> that was just scheduled to run i.e. there is no guarantee
> the queued work item may run between two successive calls
> to cma_netevent_callback() and the 2nd INIT_WORK would overwrite
> the 1st work item (for the same rdma_cm_id), despite grabbing
> id_table_lock during enqueue.
> 
> Also drgn analysis [2] indicates the work item was likely overwritten.
> 
> Fix this by moving the INIT_WORK() to __rdma_create_id(),
> so that it doesn't race with any existing queue_work() or
> its worker thread.
> 
> [1] Trimmed crash stack:
> =============================================
> BUG: kernel NULL pointer dereference, address: 0000000000000008
> kworker/u256:6 ... 6.12.0-0...
> Workqueue:  cma_netevent_work_handler [rdma_cm] (rdma_cm)
> RIP: 0010:process_one_work+0xba/0x31a
> Call Trace:
> worker_thread+0x266/0x3a0
> kthread+0xcf/0x100
> ret_from_fork+0x31/0x50
> ret_from_fork_asm+0x1a/0x30
> =============================================
> 
> [2] drgn crash analysis:
> 
> trace = prog.crashed_thread().stack_trace()
> trace
> (0)  crash_setup_regs (./arch/x86/include/asm/kexec.h:111:15)
> (1)  __crash_kexec (kernel/crash_core.c:122:4)
> (2)  panic (kernel/panic.c:399:3)
> (3)  oops_end (arch/x86/kernel/dumpstack.c:382:3)
> ...
> (8)  process_one_work (kernel/workqueue.c:3168:2)
> (9)  process_scheduled_works (kernel/workqueue.c:3310:3)
> (10) worker_thread (kernel/workqueue.c:3391:4)
> (11) kthread (kernel/kthread.c:389:9)
> 
> Line workqueue.c:3168 for this kernel version is in process_one_work():
> 3168	strscpy(worker->desc, pwq->wq->name, WORKER_DESC_LEN);
> 
> trace[8]["work"]
> *(struct work_struct *)0xffff92577d0a21d8 = {
> 	.data = (atomic_long_t){
> 		.counter = (s64)536870912,    <=== Note
> 	},
> 	.entry = (struct list_head){
> 		.next = (struct list_head *)0xffff924d075924c0,
> 		.prev = (struct list_head *)0xffff924d075924c0,
> 	},
> 	.func = (work_func_t)cma_netevent_work_handler+0x0 = 0xffffffffc2cec280,
> }
> 
> Suspicion is that pwq is NULL:
> trace[8]["pwq"]
> (struct pool_workqueue *)<absent>
> 
> In process_one_work(), pwq is assigned from:
> struct pool_workqueue *pwq = get_work_pwq(work);
> 
> and get_work_pwq() is:
> static struct pool_workqueue *get_work_pwq(struct work_struct *work)
> {
> 	unsigned long data = atomic_long_read(&work->data);
> 
> 	if (data & WORK_STRUCT_PWQ)
> 		return work_struct_pwq(data);
> 	else
> 		return NULL;
> }
> 
> WORK_STRUCT_PWQ is 0x4:
> print(repr(prog['WORK_STRUCT_PWQ']))
> Object(prog, 'enum work_flags', value=4)
> 
> But work->data is 536870912 which is 0x20000000.
> So, get_work_pwq() returns NULL and we crash in process_one_work():
> 3168	strscpy(worker->desc, pwq->wq->name, WORKER_DESC_LEN);
> =============================================
> 
> Fixes: 925d046e7e52 ("RDMA/core: Add a netevent notifier to cma")
> Cc: stable@vger.kernel.org
> Co-developed-by: Håkon Bugge <haakon.bugge@oracle.com>
> Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
> Signed-off-by: Sharath Srinivasan <sharath.srinivasan@oracle.com>

A gentle ping on this patch.


Thxs, Håkon


> ---
> v1->v2 cc:stable@vger.kernel.org
> ---
> drivers/infiniband/core/cma.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index 91db10515d74..176d0b3e4488 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -72,6 +72,8 @@ static const char * const cma_events[] = {
> static void cma_iboe_set_mgid(struct sockaddr *addr, union ib_gid *mgid,
> 			      enum ib_gid_type gid_type);
> 
> +static void cma_netevent_work_handler(struct work_struct *_work);
> +
> const char *__attribute_const__ rdma_event_msg(enum rdma_cm_event_type event)
> {
> 	size_t index = event;
> @@ -1033,6 +1035,7 @@ __rdma_create_id(struct net *net, rdma_cm_event_handler event_handler,
> 	get_random_bytes(&id_priv->seq_num, sizeof id_priv->seq_num);
> 	id_priv->id.route.addr.dev_addr.net = get_net(net);
> 	id_priv->seq_num &= 0x00ffffff;
> +	INIT_WORK(&id_priv->id.net_work, cma_netevent_work_handler);
> 
> 	rdma_restrack_new(&id_priv->res, RDMA_RESTRACK_CM_ID);
> 	if (parent)
> @@ -5227,7 +5230,6 @@ static int cma_netevent_callback(struct notifier_block *self,
> 		if (!memcmp(current_id->id.route.addr.dev_addr.dst_dev_addr,
> 			   neigh->ha, ETH_ALEN))
> 			continue;
> -		INIT_WORK(&current_id->id.net_work, cma_netevent_work_handler);
> 		cma_id_get(current_id);
> 		queue_work(cma_wq, &current_id->id.net_work);
> 	}
> --
> 2.39.5 (Apple Git-154)
Re: [PATCH v2] RDMA/cma: Fix workqueue crash in cma_netevent_work_handler
Posted by Leon Romanovsky 8 months, 1 week ago
On Tue, Apr 08, 2025 at 01:17:39PM +0000, Haakon Bugge wrote:

<...>

> > Fixes: 925d046e7e52 ("RDMA/core: Add a netevent notifier to cma")
> > Cc: stable@vger.kernel.org
> > Co-developed-by: Håkon Bugge <haakon.bugge@oracle.com>
> > Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
> > Signed-off-by: Sharath Srinivasan <sharath.srinivasan@oracle.com>
> 
> A gentle ping on this patch.

Sharath sent this patch during merge window. RDMA is closed during that period.

Thanks

> 
> 
> Thxs, Håkon
>