* [PATCH net 1/3] dt-bindings: can: microchip,mcp2510: Fix $id path
2025-05-20 9:11 [PATCH net 0/3] pull-request: can 2025-05-20 Marc Kleine-Budde
@ 2025-05-20 9:11 ` Marc Kleine-Budde
2025-05-20 14:10 ` patchwork-bot+netdevbpf
2025-05-20 9:11 ` [PATCH net 2/3] can: bcm: add locking for bcm_op runtime updates Marc Kleine-Budde
2025-05-20 9:11 ` [PATCH net 3/3] can: bcm: add missing rcu read protection for procfs content Marc Kleine-Budde
2 siblings, 1 reply; 5+ messages in thread
From: Marc Kleine-Budde @ 2025-05-20 9:11 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, Rob Herring (Arm), Conor Dooley,
Marc Kleine-Budde
From: "Rob Herring (Arm)" <robh@kernel.org>
The "$id" value must match the relative path under bindings/ and is
missing the "net" sub-directory.
Fixes: 09328600c2f9 ("dt-bindings: can: convert microchip,mcp251x.txt to yaml")
Signed-off-by: "Rob Herring (Arm)" <robh@kernel.org>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Link: https://patch.msgid.link/20250507154201.1589542-1-robh@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
.../devicetree/bindings/net/can/microchip,mcp2510.yaml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/can/microchip,mcp2510.yaml b/Documentation/devicetree/bindings/net/can/microchip,mcp2510.yaml
index e0ec53bc10c6..1525a50ded47 100644
--- a/Documentation/devicetree/bindings/net/can/microchip,mcp2510.yaml
+++ b/Documentation/devicetree/bindings/net/can/microchip,mcp2510.yaml
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
%YAML 1.2
---
-$id: http://devicetree.org/schemas/can/microchip,mcp2510.yaml#
+$id: http://devicetree.org/schemas/net/can/microchip,mcp2510.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
title: Microchip MCP251X stand-alone CAN controller
base-commit: 239af1970bcb039a1551d2c438d113df0010c149
--
2.47.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH net 1/3] dt-bindings: can: microchip,mcp2510: Fix $id path
2025-05-20 9:11 ` [PATCH net 1/3] dt-bindings: can: microchip,mcp2510: Fix $id path Marc Kleine-Budde
@ 2025-05-20 14:10 ` patchwork-bot+netdevbpf
0 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-05-20 14:10 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: netdev, davem, kuba, linux-can, kernel, robh, conor.dooley
Hello:
This series was applied to netdev/net.git (main)
by Marc Kleine-Budde <mkl@pengutronix.de>:
On Tue, 20 May 2025 11:11:01 +0200 you wrote:
> From: "Rob Herring (Arm)" <robh@kernel.org>
>
> The "$id" value must match the relative path under bindings/ and is
> missing the "net" sub-directory.
>
> Fixes: 09328600c2f9 ("dt-bindings: can: convert microchip,mcp251x.txt to yaml")
> Signed-off-by: "Rob Herring (Arm)" <robh@kernel.org>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Link: https://patch.msgid.link/20250507154201.1589542-1-robh@kernel.org
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>
> [...]
Here is the summary with links:
- [net,1/3] dt-bindings: can: microchip,mcp2510: Fix $id path
https://git.kernel.org/netdev/net/c/69c6d83d7173
- [net,2/3] can: bcm: add locking for bcm_op runtime updates
https://git.kernel.org/netdev/net/c/c2aba69d0c36
- [net,3/3] can: bcm: add missing rcu read protection for procfs content
https://git.kernel.org/netdev/net/c/dac5e6249159
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net 2/3] can: bcm: add locking for bcm_op runtime updates
2025-05-20 9:11 [PATCH net 0/3] pull-request: can 2025-05-20 Marc Kleine-Budde
2025-05-20 9:11 ` [PATCH net 1/3] dt-bindings: can: microchip,mcp2510: Fix $id path Marc Kleine-Budde
@ 2025-05-20 9:11 ` Marc Kleine-Budde
2025-05-20 9:11 ` [PATCH net 3/3] can: bcm: add missing rcu read protection for procfs content Marc Kleine-Budde
2 siblings, 0 replies; 5+ messages in thread
From: Marc Kleine-Budde @ 2025-05-20 9:11 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, Oliver Hartkopp,
Anderson Nascimento, Marc Kleine-Budde, stable
From: Oliver Hartkopp <socketcan@hartkopp.net>
The CAN broadcast manager (CAN BCM) can send a sequence of CAN frames via
hrtimer. The content and also the length of the sequence can be changed
resp reduced at runtime where the 'currframe' counter is then set to zero.
Although this appeared to be a safe operation the updates of 'currframe'
can be triggered from user space and hrtimer context in bcm_can_tx().
Anderson Nascimento created a proof of concept that triggered a KASAN
slab-out-of-bounds read access which can be prevented with a spin_lock_bh.
At the rework of bcm_can_tx() the 'count' variable has been moved into
the protected section as this variable can be modified from both contexts
too.
Fixes: ffd980f976e7 ("[CAN]: Add broadcast manager (bcm) protocol")
Reported-by: Anderson Nascimento <anderson@allelesecurity.com>
Tested-by: Anderson Nascimento <anderson@allelesecurity.com>
Reviewed-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://patch.msgid.link/20250519125027.11900-1-socketcan@hartkopp.net
Cc: stable@vger.kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
net/can/bcm.c | 66 +++++++++++++++++++++++++++++++++++----------------
1 file changed, 45 insertions(+), 21 deletions(-)
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 0bca1b9b3f70..871707dab7db 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -58,6 +58,7 @@
#include <linux/can/skb.h>
#include <linux/can/bcm.h>
#include <linux/slab.h>
+#include <linux/spinlock.h>
#include <net/sock.h>
#include <net/net_namespace.h>
@@ -122,6 +123,7 @@ struct bcm_op {
struct canfd_frame last_sframe;
struct sock *sk;
struct net_device *rx_reg_dev;
+ spinlock_t bcm_tx_lock; /* protect currframe/count in runtime updates */
};
struct bcm_sock {
@@ -285,13 +287,18 @@ static void bcm_can_tx(struct bcm_op *op)
{
struct sk_buff *skb;
struct net_device *dev;
- struct canfd_frame *cf = op->frames + op->cfsiz * op->currframe;
+ struct canfd_frame *cf;
int err;
/* no target device? => exit */
if (!op->ifindex)
return;
+ /* read currframe under lock protection */
+ spin_lock_bh(&op->bcm_tx_lock);
+ cf = op->frames + op->cfsiz * op->currframe;
+ spin_unlock_bh(&op->bcm_tx_lock);
+
dev = dev_get_by_index(sock_net(op->sk), op->ifindex);
if (!dev) {
/* RFC: should this bcm_op remove itself here? */
@@ -312,6 +319,10 @@ static void bcm_can_tx(struct bcm_op *op)
skb->dev = dev;
can_skb_set_owner(skb, op->sk);
err = can_send(skb, 1);
+
+ /* update currframe and count under lock protection */
+ spin_lock_bh(&op->bcm_tx_lock);
+
if (!err)
op->frames_abs++;
@@ -320,6 +331,11 @@ static void bcm_can_tx(struct bcm_op *op)
/* reached last frame? */
if (op->currframe >= op->nframes)
op->currframe = 0;
+
+ if (op->count > 0)
+ op->count--;
+
+ spin_unlock_bh(&op->bcm_tx_lock);
out:
dev_put(dev);
}
@@ -430,7 +446,7 @@ static enum hrtimer_restart bcm_tx_timeout_handler(struct hrtimer *hrtimer)
struct bcm_msg_head msg_head;
if (op->kt_ival1 && (op->count > 0)) {
- op->count--;
+ bcm_can_tx(op);
if (!op->count && (op->flags & TX_COUNTEVT)) {
/* create notification to user */
@@ -445,7 +461,6 @@ static enum hrtimer_restart bcm_tx_timeout_handler(struct hrtimer *hrtimer)
bcm_send_to_user(op, &msg_head, NULL, 0);
}
- bcm_can_tx(op);
} else if (op->kt_ival2) {
bcm_can_tx(op);
@@ -956,6 +971,27 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
}
op->flags = msg_head->flags;
+ /* only lock for unlikely count/nframes/currframe changes */
+ if (op->nframes != msg_head->nframes ||
+ op->flags & TX_RESET_MULTI_IDX ||
+ op->flags & SETTIMER) {
+
+ spin_lock_bh(&op->bcm_tx_lock);
+
+ if (op->nframes != msg_head->nframes ||
+ op->flags & TX_RESET_MULTI_IDX) {
+ /* potentially update changed nframes */
+ op->nframes = msg_head->nframes;
+ /* restart multiple frame transmission */
+ op->currframe = 0;
+ }
+
+ if (op->flags & SETTIMER)
+ op->count = msg_head->count;
+
+ spin_unlock_bh(&op->bcm_tx_lock);
+ }
+
} else {
/* insert new BCM operation for the given can_id */
@@ -963,9 +999,14 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
if (!op)
return -ENOMEM;
+ spin_lock_init(&op->bcm_tx_lock);
op->can_id = msg_head->can_id;
op->cfsiz = CFSIZ(msg_head->flags);
op->flags = msg_head->flags;
+ op->nframes = msg_head->nframes;
+
+ if (op->flags & SETTIMER)
+ op->count = msg_head->count;
/* create array for CAN frames and copy the data */
if (msg_head->nframes > 1) {
@@ -1023,22 +1064,8 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
} /* if ((op = bcm_find_op(&bo->tx_ops, msg_head->can_id, ifindex))) */
- if (op->nframes != msg_head->nframes) {
- op->nframes = msg_head->nframes;
- /* start multiple frame transmission with index 0 */
- op->currframe = 0;
- }
-
- /* check flags */
-
- if (op->flags & TX_RESET_MULTI_IDX) {
- /* start multiple frame transmission with index 0 */
- op->currframe = 0;
- }
-
if (op->flags & SETTIMER) {
/* set timer values */
- op->count = msg_head->count;
op->ival1 = msg_head->ival1;
op->ival2 = msg_head->ival2;
op->kt_ival1 = bcm_timeval_to_ktime(msg_head->ival1);
@@ -1055,11 +1082,8 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
op->flags |= TX_ANNOUNCE;
}
- if (op->flags & TX_ANNOUNCE) {
+ if (op->flags & TX_ANNOUNCE)
bcm_can_tx(op);
- if (op->count)
- op->count--;
- }
if (op->flags & STARTTIMER)
bcm_tx_start_timer(op);
--
2.47.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH net 3/3] can: bcm: add missing rcu read protection for procfs content
2025-05-20 9:11 [PATCH net 0/3] pull-request: can 2025-05-20 Marc Kleine-Budde
2025-05-20 9:11 ` [PATCH net 1/3] dt-bindings: can: microchip,mcp2510: Fix $id path Marc Kleine-Budde
2025-05-20 9:11 ` [PATCH net 2/3] can: bcm: add locking for bcm_op runtime updates Marc Kleine-Budde
@ 2025-05-20 9:11 ` Marc Kleine-Budde
2 siblings, 0 replies; 5+ messages in thread
From: Marc Kleine-Budde @ 2025-05-20 9:11 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, Oliver Hartkopp,
Anderson Nascimento, stable, Marc Kleine-Budde
From: Oliver Hartkopp <socketcan@hartkopp.net>
When the procfs content is generated for a bcm_op which is in the process
to be removed the procfs output might show unreliable data (UAF).
As the removal of bcm_op's is already implemented with rcu handling this
patch adds the missing rcu_read_lock() and makes sure the list entries
are properly removed under rcu protection.
Fixes: f1b4e32aca08 ("can: bcm: use call_rcu() instead of costly synchronize_rcu()")
Reported-by: Anderson Nascimento <anderson@allelesecurity.com>
Suggested-by: Anderson Nascimento <anderson@allelesecurity.com>
Tested-by: Anderson Nascimento <anderson@allelesecurity.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://patch.msgid.link/20250519125027.11900-2-socketcan@hartkopp.net
Cc: stable@vger.kernel.org # >= 5.4
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
net/can/bcm.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 871707dab7db..6bc1cc4c94c5 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -219,7 +219,9 @@ static int bcm_proc_show(struct seq_file *m, void *v)
seq_printf(m, " / bound %s", bcm_proc_getifname(net, ifname, bo->ifindex));
seq_printf(m, " <<<\n");
- list_for_each_entry(op, &bo->rx_ops, list) {
+ rcu_read_lock();
+
+ list_for_each_entry_rcu(op, &bo->rx_ops, list) {
unsigned long reduction;
@@ -275,6 +277,9 @@ static int bcm_proc_show(struct seq_file *m, void *v)
seq_printf(m, "# sent %ld\n", op->frames_abs);
}
seq_putc(m, '\n');
+
+ rcu_read_unlock();
+
return 0;
}
#endif /* CONFIG_PROC_FS */
@@ -858,7 +863,7 @@ static int bcm_delete_rx_op(struct list_head *ops, struct bcm_msg_head *mh,
REGMASK(op->can_id),
bcm_rx_handler, op);
- list_del(&op->list);
+ list_del_rcu(&op->list);
bcm_remove_op(op);
return 1; /* done */
}
@@ -878,7 +883,7 @@ static int bcm_delete_tx_op(struct list_head *ops, struct bcm_msg_head *mh,
list_for_each_entry_safe(op, n, ops, list) {
if ((op->can_id == mh->can_id) && (op->ifindex == ifindex) &&
(op->flags & CAN_FD_FRAME) == (mh->flags & CAN_FD_FRAME)) {
- list_del(&op->list);
+ list_del_rcu(&op->list);
bcm_remove_op(op);
return 1; /* done */
}
@@ -1296,7 +1301,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
bcm_rx_handler, op, "bcm", sk);
if (err) {
/* this bcm rx op is broken -> remove it */
- list_del(&op->list);
+ list_del_rcu(&op->list);
bcm_remove_op(op);
return err;
}
--
2.47.2
^ permalink raw reply related [flat|nested] 5+ messages in thread