From: Oliver Hartkopp <socketcan@hartkopp.net>
To: stable@vger.kernel.org
Cc: Oliver Hartkopp <socketcan@hartkopp.net>,
Marc Kleine-Budde <mkl@pengutronix.de>
Subject: [PATCH 1/9] can: bcm: defer rx_op deallocation to workqueue to fix thrtimer UAF
Date: Fri, 24 Jul 2026 10:37:48 +0200 [thread overview]
Message-ID: <20260724083756.12430-2-socketcan@hartkopp.net> (raw)
In-Reply-To: <2026072036-giving-knee-3a33@gregkh>
From: Lee Jones <lee@kernel.org>
commit 68973f9db76144825e4f35dfdc80fb8279eb2d57 upstream.
Commit f1b4e32aca08 ("can: bcm: use call_rcu() instead of costly
synchronize_rcu()") replaced synchronize_rcu() in bcm_delete_rx_op()
with call_rcu() and introduced the RX_NO_AUTOTIMER flag.
However, this flag check was omitted for thrtimer in the packet rx
fast-path. During BCM RX operation teardown, a concurrent RCU reader
(bcm_rx_handler) can race and re-arm thrtimer via
bcm_rx_update_and_send() after call_rcu() has been scheduled. Once
the RCU grace period elapses, bcm_op is freed. The subsequently
firing thrtimer then dereferences the deallocated op, causing a UAF.
Adding flag checks to the rx fast-path (bcm_rx_update_and_send) does not
fully close the TOCTOU race and introduces latency for every CAN frame.
Conversely, calling hrtimer_cancel() directly inside the RCU callback
(softirq context) is fatal as hrtimer_cancel() can sleep, triggering
a "scheduling while atomic" panic.
Resolve this by deferring the timer cancellation and memory free to a
dedicated unbound workqueue (bcm_wq). The RCU callback now queues a
work item to bcm_wq, which safely cancels both timers and deallocates
memory in sleepable process context. A dedicated workqueue is used to
prevent system-wide WQ saturation and is cleanly flushed/destroyed
on module unload to avoid rmmod page faults.
Since the deferred work can now outlive the calling context by an
unbounded amount, also take a reference on op->sk when it is assigned
and drop it only once the deferred work has cancelled both timers, so a
socket can no longer be freed out from under a still-armed timer whose
callback (bcm_send_to_user()) dereferences op->sk.
Fixes: f1b4e32aca08 ("can: bcm: use call_rcu() instead of costly synchronize_rcu()")
Tested-by: Feng Xue <feng.xue@outlook.com>
Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Lee Jones <lee@kernel.org>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://patch.msgid.link/20260714-bcm_fixes-v15-1-562f7e3e42da@hartkopp.net
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
net/can/bcm.c | 36 +++++++++++++++++++++++++++++++++---
1 file changed, 33 insertions(+), 3 deletions(-)
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 317cd4c..05ef139 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -58,6 +58,7 @@
#include <linux/can/skb.h>
#include <linux/can/bcm.h>
#include <linux/slab.h>
+#include <linux/workqueue.h>
#include <linux/spinlock.h>
#include <net/sock.h>
#include <net/net_namespace.h>
@@ -89,6 +90,8 @@ MODULE_ALIAS("can-proto-2");
#define BCM_MIN_NAMELEN CAN_REQUIRED_SIZE(struct sockaddr_can, can_ifindex)
+static struct workqueue_struct *bcm_wq;
+
/*
* easy access to the first 64 bit of can(fd)_frame payload. cp->data is
* 64 bit aligned so the offset has to be multiples of 8 which is ensured
@@ -102,6 +105,7 @@ static inline u64 get_u64(const struct canfd_frame *cp, int offset)
struct bcm_op {
struct list_head list;
struct rcu_head rcu;
+ struct work_struct work;
int ifindex;
canid_t can_id;
u32 flags;
@@ -740,9 +744,12 @@ static struct bcm_op *bcm_find_op(struct list_head *ops,
return NULL;
}
-static void bcm_free_op_rcu(struct rcu_head *rcu_head)
+static void bcm_free_op_work(struct work_struct *work)
{
- struct bcm_op *op = container_of(rcu_head, struct bcm_op, rcu);
+ struct bcm_op *op = container_of(work, struct bcm_op, work);
+
+ hrtimer_cancel(&op->timer);
+ hrtimer_cancel(&op->thrtimer);
if ((op->frames) && (op->frames != &op->sframe))
kfree(op->frames);
@@ -750,9 +757,23 @@ static void bcm_free_op_rcu(struct rcu_head *rcu_head)
if ((op->last_frames) && (op->last_frames != &op->last_sframe))
kfree(op->last_frames);
+ /* the last possible access to op->timer/op->thrtimer has now
+ * happened above via hrtimer_cancel() - op->sk is no longer
+ * needed by any pending timer callback, so drop our reference
+ */
+ sock_put(op->sk);
+
kfree(op);
}
+static void bcm_free_op_rcu(struct rcu_head *rcu_head)
+{
+ struct bcm_op *op = container_of(rcu_head, struct bcm_op, rcu);
+
+ INIT_WORK(&op->work, bcm_free_op_work);
+ queue_work(bcm_wq, &op->work);
+}
+
static void bcm_remove_op(struct bcm_op *op)
{
hrtimer_cancel(&op->timer);
@@ -1008,6 +1029,7 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
/* bcm_can_tx / bcm_tx_timeout_handler needs this */
op->sk = sk;
+ sock_hold(sk);
op->ifindex = ifindex;
/* initialize uninitialized (kzalloc) structure */
@@ -1173,6 +1195,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
/* bcm_can_tx / bcm_tx_timeout_handler needs this */
op->sk = sk;
+ sock_hold(sk);
op->ifindex = ifindex;
/* ifindex for timeout events w/o previous frame reception */
@@ -1699,7 +1722,7 @@ static int bcm_connect(struct socket *sock, struct sockaddr *uaddr, int len,
#if IS_ENABLED(CONFIG_PROC_FS)
if (net->can.bcmproc_dir) {
/* unique socket address as filename */
- sprintf(bo->procname, "%lu", sock_i_ino(sk));
+ sprintf(bo->procname, "%llu", sock_i_ino(sk));
bo->bcm_proc_read = proc_create_net_single(bo->procname, 0644,
net->can.bcmproc_dir,
bcm_proc_show, sk);
@@ -1830,11 +1853,16 @@ static int __init bcm_module_init(void)
{
int err;
+ bcm_wq = alloc_workqueue("can-bcm-wq", WQ_UNBOUND, 0);
+ if (!bcm_wq)
+ return -ENOMEM;
+
pr_info("can: broadcast manager protocol\n");
err = can_proto_register(&bcm_can_proto);
if (err < 0) {
printk(KERN_ERR "can: registration of bcm protocol failed\n");
+ destroy_workqueue(bcm_wq);
return err;
}
@@ -1848,6 +1876,8 @@ static void __exit bcm_module_exit(void)
can_proto_unregister(&bcm_can_proto);
unregister_netdevice_notifier(&canbcm_notifier);
unregister_pernet_subsys(&canbcm_pernet_ops);
+ rcu_barrier();
+ destroy_workqueue(bcm_wq);
}
module_init(bcm_module_init);
--
2.43.0
next prev parent reply other threads:[~2026-07-24 8:38 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-20 15:12 FAILED: patch "[PATCH] can: bcm: defer rx_op deallocation to workqueue to fix" failed to apply to 5.15-stable tree gregkh
2026-07-24 8:37 ` Oliver Hartkopp [this message]
2026-07-24 10:21 ` [PATCH 5.15.y 1/9] can: bcm: defer rx_op deallocation to workqueue to fix thrtimer UAF Oliver Hartkopp
2026-07-24 10:21 ` [PATCH 5.15.y 2/9] can: bcm: add locking when updating filter and timer values Oliver Hartkopp
2026-07-24 10:21 ` [PATCH 5.15.y 3/9] can: bcm: fix CAN frame rx/tx statistics Oliver Hartkopp
2026-07-24 10:21 ` [PATCH 5.15.y 4/9] can: bcm: extend bcm_tx_lock usage for data and timer updates Oliver Hartkopp
2026-07-24 10:21 ` [PATCH 5.15.y 5/9] can: bcm: validate frame length in bcm_rx_setup() for RTR replies Oliver Hartkopp
2026-07-24 10:21 ` [PATCH 5.15.y 6/9] can: bcm: add missing device refcount for CAN filter removal Oliver Hartkopp
2026-07-24 10:21 ` [PATCH 5.15.y 7/9] can: bcm: fix stale rx/tx ops after device removal Oliver Hartkopp
2026-07-24 10:21 ` [PATCH 5.15.y 8/9] can: bcm: fix data race on rx_stamp/rx_ifindex in bcm_rx_handler() Oliver Hartkopp
2026-07-24 10:21 ` [PATCH 5.15.y 9/9] can: bcm: track a single source interface for ANYDEV timeout/throttle ops Oliver Hartkopp
2026-07-28 16:09 ` [PATCH 5.15.y 1/9] can: bcm: defer rx_op deallocation to workqueue to fix thrtimer UAF Sasha Levin
2026-07-28 16:47 ` Oliver Hartkopp
-- strict thread matches above, loose matches on Subject: below --
2026-07-20 15:12 FAILED: patch "[PATCH] can: bcm: defer rx_op deallocation to workqueue to fix" failed to apply to 5.10-stable tree gregkh
2026-07-24 8:35 ` [PATCH 1/9] can: bcm: defer rx_op deallocation to workqueue to fix thrtimer UAF Oliver Hartkopp
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260724083756.12430-2-socketcan@hartkopp.net \
--to=socketcan@hartkopp.net \
--cc=mkl@pengutronix.de \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox