github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-07-27 09:36:22 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

can: bcm: extend bcm_tx_lock usage for data and timer updates

Browse Source 
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: c2aba69d0c ("can: bcm: add locking for bcm_op runtime updates")
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://patch.msgid.link/20260714-bcm_fixes-v15-6-562f7e3e42da@hartkopp.net
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
This commit is contained in:
Oliver Hartkopp 2026-07-14 18:55:28 +02:00 committed by Marc Kleine-Budde
parent 7b2c3eabc4
commit 12ce799f7a
1 changed files with 77 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

108
net/can/bcm.c
View File

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