FAILED: patch "[PATCH] can: bcm: extend bcm_tx_lock usage for data and timer updates" failed to apply to 6.12-stable tree
Subsystems (2)
[email protected]
maintainerThe patch below does not apply to the 6.12-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.12.y
git checkout FETCH_HEAD
git cherry-pick -x 12ce799f7ab1e05bd8fbf79e46f403bfe5597ebc
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026072034-name-yahoo-51a6@gregkh' --subject-prefix 'PATCH 6.12.y' 'HEAD^..'
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 12ce799f7ab1e05bd8fbf79e46f403bfe5597ebc Mon Sep 17 00:00:00 2001
From: Oliver Hartkopp <[email protected]>
Date: Tue, 14 Jul 2026 18:55:28 +0200
Subject: [PATCH] can: bcm: extend bcm_tx_lock usage for data and timer updates
Stage new CAN frame content for an existing tx op into a kmalloc()'d buffer and validate it there, mirroring the approach already used in bcm_rx_setup(). Only copy the validated data into op->frames while holding op->bcm_tx_lock, so bcm_can_tx() and bcm_tx_timeout_handler() can no longer observe a partially updated or unvalidated frame.
Add a missing error path for memcpy_from_msg() when copying CAN frame data from userspace.
Also move the kt_ival1/kt_ival2/ival1/ival2 updates in bcm_tx_setup() under op->bcm_tx_lock, and read kt_ival1/kt_ival2/count under the same lock in bcm_tx_set_expiry() and bcm_tx_timeout_handler(), closing the torn 64-bit ktime_t read on 32-bit platforms.
Fixes c2aba69d0c36 ("can: bcm: add locking for bcm_op runtime updates")
Signed-off-by Oliver Hartkopp <[email protected]>
Signed-off-by Marc Kleine-Budde <[email protected]>
net/can/bcm.c +77 -31
@@ -128,7 +128,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 */ +spinlock_t bcm_tx_lock;/* protect tx data and timer updates */ spinlock_t bcm_rx_update_lock;/* protect filter/timer data updates */ };
@@ -472,12 +472,18 @@ static bool bcm_tx_set_expiry(struct bcm_op *op, struct hrtimer *hrt) {ktime_t ival; +spin_lock_bh (&op->bcm_tx_lock); +if (op->kt_ival1 && op->count) ival = op->kt_ival1; -else if (op->kt_ival2) +else if (op->kt_ival2) { ival = op->kt_ival2; -else + }else { +spin_unlock_bh (&op->bcm_tx_lock);return false; + } + +spin_unlock_bh (&op->bcm_tx_lock);hrtimer_set_expires (hrt,ktime_add (ktime_get (), ival));return true;
@@ -494,25 +500,47 @@ static enum hrtimer_restart bcm_tx_timeout_handler(struct hrtimer *hrtimer) { struct bcm_op *op = container_of(hrtimer, struct bcm_op, timer);struct bcm_msg_head msg_head; +bool tx_ival1, tx_ival2; + +/* snapshot kt_ival1/kt_ival2/count under lock to avoid torn +* ktime_t reads racing with concurrent bcm_tx_setup() updates +*/ +spin_lock_bh (&op->bcm_tx_lock); + tx_ival1 = op->kt_ival1 && (op->count >0 ); + tx_ival2 = !!op->kt_ival2; +spin_unlock_bh (&op->bcm_tx_lock); + +if (tx_ival1) { +u32 flags, count; +struct bcm_timeval ival1, ival2; -if (op->kt_ival1 && (op->count >0 )) {bcm_can_tx (op,NULL ); -if (!op->count && (op->flags &TX_COUNTEVT )) { +/* snapshot variables under lock to avoid torn reads racing +* with concurrent bcm_tx_setup() updates +*/ +spin_lock_bh (&op->bcm_tx_lock); + flags = op->flags; + count = op->count; + ival1 = op->ival1; + ival2 = op->ival2; +spin_unlock_bh (&op->bcm_tx_lock); + +if (!count && (flags &TX_COUNTEVT )) {/* create notification to user */ memset (&msg_head,0 ,sizeof (msg_head)); msg_head.opcode =TX_EXPIRED ; - msg_head.flags = op->flags; - msg_head.count = op->count; - msg_head.ival1 = op->ival1; - msg_head.ival2 = op->ival2; + msg_head.flags = flags; + msg_head.count = count; + msg_head.ival1 = ival1; + msg_head.ival2 = ival2; msg_head.can_id = op->can_id; msg_head.nframes =0 ;bcm_send_to_user (op, &msg_head,NULL ,0 ); } - }else if (op->kt_ival2) { + }else if (tx_ival2) {bcm_can_tx (op,NULL ); }
@@ -1036,6 +1064,8 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,/* check the given can_id */ op =bcm_find_op (&bo->tx_ops, msg_head, ifindex);if (op) { +void *new_frames; +/* update existing BCM operation */ /*
@@ -1046,11 +1076,23 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,if (msg_head->nframes > op->nframes)return -E2BIG ; -/* update CAN frames content */ +/* get new CAN frames content into a staging buffer before +* locking: validate and normalize the frames there so that +* bcm_can_tx() / bcm_tx_timeout_handler() never observe a +* partially updated or unvalidated frame in op->frames +*/ + new_frames =kmalloc (msg_head->nframes * op->cfsiz,GFP_KERNEL ); +if (!new_frames) +return -ENOMEM ; +for (i =0 ; i < msg_head->nframes; i++) { - cf = op->frames + op->cfsiz * i; + cf = new_frames + op->cfsiz * i; err =memcpy_from_msg ((u8 *)cf, msg, op->cfsiz); +if (err <0 ) { +kfree (new_frames); +return err; + }if (op->flags &CAN_FD_FRAME ) {if (cf->len >64 )
@@ -1060,37 +1102,39 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, err = -EINVAL ; } -if (err <0 ) +if (err <0 ) { +kfree (new_frames);return err; + }if (msg_head->flags &TX_CP_CAN_ID ) {/* copy can_id into frame */ cf->can_id = msg_head->can_id; } } + +spin_lock_bh (&op->bcm_tx_lock); + +/* update CAN frames content */ +memcpy (op->frames, new_frames, msg_head->nframes * op->cfsiz); + 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); + 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); + +kfree (new_frames); + }else {/* insert new BCM operation for the given can_id */
@@ -1165,10 +1209,12 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,if (op->flags &SETTIMER ) {/* set timer values */ +spin_lock_bh (&op->bcm_tx_lock); op->ival1 = msg_head->ival1; op->ival2 = msg_head->ival2; op->kt_ival1 =bcm_timeval_to_ktime (msg_head->ival1); op->kt_ival2 =bcm_timeval_to_ktime (msg_head->ival2); +spin_unlock_bh (&op->bcm_tx_lock);/* disable an active timer due to zero values? */ if (!op->kt_ival1 && !op->kt_ival2)
Oliver Hartkopp
maintainerPATCH 3/8 can: bcm: extend bcm_tx_lock usage for data and timer updates
commit 12ce799f7ab1e05bd8fbf79e46f403bfe5597ebc upstream.
Stage new CAN frame content for an existing tx op into a kmalloc()'d buffer and validate it there, mirroring the approach already used in bcm_rx_setup(). Only copy the validated data into op->frames while holding op->bcm_tx_lock, so bcm_can_tx() and bcm_tx_timeout_handler() can no longer observe a partially updated or unvalidated frame.
Add a missing error path for memcpy_from_msg() when copying CAN frame data from userspace.
Also move the kt_ival1/kt_ival2/ival1/ival2 updates in bcm_tx_setup() under op->bcm_tx_lock, and read kt_ival1/kt_ival2/count under the same lock in bcm_tx_set_expiry() and bcm_tx_timeout_handler(), closing the torn 64-bit ktime_t read on 32-bit platforms.
Fixes c2aba69d0c36 ("can: bcm: add locking for bcm_op runtime updates")
Signed-off-by Oliver Hartkopp <[email protected]>
Signed-off-by Marc Kleine-Budde <[email protected]>
Signed-off-by Oliver Hartkopp <[email protected]>
--- net/can/bcm.c | 104 ++++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 75 insertions(+), 29 deletions(-)
net/can/bcm.c +75 -29
@@ -127,7 +127,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 */ +spinlock_t bcm_tx_lock;/* protect tx data and timer updates */ spinlock_t bcm_rx_update_lock;/* protect filter/timer data updates */ };
@@ -466,12 +466,18 @@ static bool bcm_tx_set_expiry(struct bcm_op *op, struct hrtimer *hrt) {ktime_t ival; +spin_lock_bh (&op->bcm_tx_lock); +if (op->kt_ival1 && op->count) ival = op->kt_ival1; -else if (op->kt_ival2) +else if (op->kt_ival2) { ival = op->kt_ival2; -else + }else { +spin_unlock_bh (&op->bcm_tx_lock);return false; + } + +spin_unlock_bh (&op->bcm_tx_lock);hrtimer_set_expires (hrt,ktime_add (ktime_get (), ival));return true;
@@ -488,25 +494,47 @@ static enum hrtimer_restart bcm_tx_timeout_handler(struct hrtimer *hrtimer) { struct bcm_op *op = container_of(hrtimer, struct bcm_op, timer);struct bcm_msg_head msg_head; +bool tx_ival1, tx_ival2; + +/* snapshot kt_ival1/kt_ival2/count under lock to avoid torn +* ktime_t reads racing with concurrent bcm_tx_setup() updates +*/ +spin_lock_bh (&op->bcm_tx_lock); + tx_ival1 = op->kt_ival1 && (op->count >0 ); + tx_ival2 = !!op->kt_ival2; +spin_unlock_bh (&op->bcm_tx_lock); + +if (tx_ival1) { +u32 flags, count; +struct bcm_timeval ival1, ival2; -if (op->kt_ival1 && (op->count >0 )) {bcm_can_tx (op,NULL ); -if (!op->count && (op->flags &TX_COUNTEVT )) { +/* snapshot variables under lock to avoid torn reads racing +* with concurrent bcm_tx_setup() updates +*/ +spin_lock_bh (&op->bcm_tx_lock); + flags = op->flags; + count = op->count; + ival1 = op->ival1; + ival2 = op->ival2; +spin_unlock_bh (&op->bcm_tx_lock); + +if (!count && (flags &TX_COUNTEVT )) {/* create notification to user */ memset (&msg_head,0 ,sizeof (msg_head)); msg_head.opcode =TX_EXPIRED ; - msg_head.flags = op->flags; - msg_head.count = op->count; - msg_head.ival1 = op->ival1; - msg_head.ival2 = op->ival2; + msg_head.flags = flags; + msg_head.count = count; + msg_head.ival1 = ival1; + msg_head.ival2 = ival2; msg_head.can_id = op->can_id; msg_head.nframes =0 ;bcm_send_to_user (op, &msg_head,NULL ,0 ); } - }else if (op->kt_ival2) { + }else if (tx_ival2) {bcm_can_tx (op,NULL ); }
@@ -1030,6 +1058,8 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,/* check the given can_id */ op =bcm_find_op (&bo->tx_ops, msg_head, ifindex);if (op) { +void *new_frames; +/* update existing BCM operation */ /*
@@ -1040,11 +1070,23 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,if (msg_head->nframes > op->nframes)return -E2BIG ; -/* update CAN frames content */ +/* get new CAN frames content into a staging buffer before +* locking: validate and normalize the frames there so that +* bcm_can_tx() / bcm_tx_timeout_handler() never observe a +* partially updated or unvalidated frame in op->frames +*/ + new_frames =kmalloc (msg_head->nframes * op->cfsiz,GFP_KERNEL ); +if (!new_frames) +return -ENOMEM ; +for (i =0 ; i < msg_head->nframes; i++) { - cf = op->frames + op->cfsiz * i; + cf = new_frames + op->cfsiz * i; err =memcpy_from_msg ((u8 *)cf, msg, op->cfsiz); +if (err <0 ) { +kfree (new_frames); +return err; + }if (op->flags &CAN_FD_FRAME ) {if (cf->len >64 )
@@ -1054,36 +1096,38 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, err = -EINVAL ; } -if (err <0 ) +if (err <0 ) { +kfree (new_frames);return err; + }if (msg_head->flags &TX_CP_CAN_ID ) {/* copy can_id into frame */ cf->can_id = msg_head->can_id; } } + +spin_lock_bh (&op->bcm_tx_lock); + +/* update CAN frames content */ +memcpy (op->frames, new_frames, msg_head->nframes * op->cfsiz); + 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); + op->flags &TX_RESET_MULTI_IDX ) { +/* potentially update changed nframes */ + op->nframes = msg_head->nframes; +/* restart multiple frame transmission */ + op->currframe =0 ; + } -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; -if (op->flags &SETTIMER ) - op->count = msg_head->count; +spin_unlock_bh (&op->bcm_tx_lock); -spin_unlock_bh (&op->bcm_tx_lock); - } +kfree (new_frames); }else {/* insert new BCM operation for the given can_id */
@@ -1160,10 +1204,12 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,if (op->flags &SETTIMER ) {/* set timer values */ +spin_lock_bh (&op->bcm_tx_lock); op->ival1 = msg_head->ival1; op->ival2 = msg_head->ival2; op->kt_ival1 =bcm_timeval_to_ktime (msg_head->ival1); op->kt_ival2 =bcm_timeval_to_ktime (msg_head->ival2); +spin_unlock_bh (&op->bcm_tx_lock);/* disable an active timer due to zero values? */ if (!op->kt_ival1 && !op->kt_ival2)
Signature · 1 line
2.43.0