From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mo4-p03-ob.smtp.rzone.de (mo4-p03-ob.smtp.rzone.de [81.169.146.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9B838421230 for ; Fri, 24 Jul 2026 10:26:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=pass smtp.client-ip=81.169.146.174 ARC-Seal:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784888775; cv=pass; b=NVrXTKvjN895EOf8T8JpnBk/Qb/X55IhoTKs3xpo5C+OwvaoRix6MBkOLaIE8jl8wK3UilZhzXmvm9j6wgzK13PsnLZQ4/usSOZsShFQjbgYcYDZNCCQv0HiSOE4d6hTyuZwbpmz/U+M6VMI8NChmVY56azcCtaX0iGdsBRWuP8= ARC-Message-Signature:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784888775; c=relaxed/simple; bh=c07b5Lmbf95XLaD5omF4phnfUz1E9e2Mb4ylJ0Gmggw=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=rUNp3AMVuxGMQIgmOSMBERJWxjRlixbKqLs4nSY0ZTGut92QkSHoXYP1N8m2WOL/50g1CzjH12qtgLcr/MxnyvJQKFb+EJPqV9lYjNYGmDbFaq4coy+Y5qik+It0dvAOS8AQ/N/XeN12SGMwHBI7PYX3xxpe31E7if9WtjN+t+w= ARC-Authentication-Results:i=2; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=hartkopp.net; spf=fail smtp.mailfrom=hartkopp.net; dkim=pass (2048-bit key) header.d=hartkopp.net header.i=@hartkopp.net header.b=t0CipRoz; dkim=permerror (0-bit key) header.d=hartkopp.net header.i=@hartkopp.net header.b=Qrmh2kUb; arc=pass smtp.client-ip=81.169.146.174 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=hartkopp.net Authentication-Results: smtp.subspace.kernel.org; spf=fail smtp.mailfrom=hartkopp.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=hartkopp.net header.i=@hartkopp.net header.b="t0CipRoz"; dkim=permerror (0-bit key) header.d=hartkopp.net header.i=@hartkopp.net header.b="Qrmh2kUb" ARC-Seal: i=1; a=rsa-sha256; t=1784888766; cv=none; d=strato.com; s=strato-dkim-0002; b=lbMaUFS923rp0VqPBNM+DXF42Jv3MuBjRGpyweg1pQt8rZjkr2BihBCMw4vLzFgCKg 40W3eUMp5yNXjM/VJLpYYRBRpTozNHo4cLD1h4C+kupj3FlxN4696qZZ6ssFFXNzOjUH tIB5v41G+scoTL6/nVPeG1pVkPTrA2HRjjsMRcIkLbH4VoenEvuNAwIk/+MsVkQwkJKx kwCYZ8VxTiqKwv+TIvWThTzB8iFu+QYhfKt6MUDVNNDjAvW8QYjifWaKFj6zPDISA5/V XYmSrDq/hI24cPrrhcQ3hJFn0wbAIjJchQpaKIXphwbfFqxV3DGq/T3HuF+0uf9plRjp R0Mw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; t=1784888766; s=strato-dkim-0002; d=strato.com; h=References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From:Cc:Date: From:Subject:Sender; bh=B9wfXl5yTrDzRCVnPCfI5sfw4l6fNo1U1Ozd4whZghg=; b=kbQnN8ozztp08FXheLiEAXADm0ZBJV/FGP59HQp37Ebbs0NQHfLKEeLpTR4HsamAXC vU4JjKFju6PJenGFdhxlnX6UORjKzd15oyyasenJ1AtKSKfANlszZBbdSb/hX7wGXDLU DEC5LwL1jlnANWARfLUsADCMy8IhZLZtdynMhqpXwMPA7tipZ1UVVWzbF4K8uvMjCCOB RE6waprPeJc4F5O/5A6FTRWZ50wBTGavhcKPHZIvQbSMNNJsywERT4njRSm3Ustpabhq YsgCl6azk/WmNv6qzW+pZ2M0MN7VKzZlBXzejOj5vCIhPaYO9uvdHFb61JOUrCJquJjc jH1Q== ARC-Authentication-Results: i=1; strato.com; arc=none; dkim=none X-RZG-CLASS-ID: mo03 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1784888766; s=strato-dkim-0002; d=hartkopp.net; h=References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From:Cc:Date: From:Subject:Sender; bh=B9wfXl5yTrDzRCVnPCfI5sfw4l6fNo1U1Ozd4whZghg=; b=t0CipRoz5JgvkE4WkWHFEbyBeEcvaGRDcjU/tjOK0uBUO5Gd1anH5uLr6HAGLfnErh yLm+0ez/VWURG/mBYXydRZaUiG7axTyNNvrt3QUbr+4mVR0YclYOuy7mzcoYPDd2Y+vU rhh6zuKJHLdHICj/ZZktBggavRPzb4uAagGo6oDQFU/vwlQIzKAJ2Ss4T20uqxv/k+C+ t00DDyNEMWDsB/EiPOSgm0N8SPrx1RnsrRuJzLM/eU283kz/Bv6EI7ad9kjhXTSNKcDw ziiTIMhdyvUvDQuN1iRD9DNnhZTU732veIyYf+0PbOSYPVfOnVCsLVwQFLncyHBRdzg0 w8Dg== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1784888766; s=strato-dkim-0003; d=hartkopp.net; h=References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From:Cc:Date: From:Subject:Sender; bh=B9wfXl5yTrDzRCVnPCfI5sfw4l6fNo1U1Ozd4whZghg=; b=Qrmh2kUbU2cfZ0pYYRn9YAjVgCKzRwjh6O8wUegRAR+r388IUeTO0iukfRtD1oLpRv oW3Z2u3K/hZOH1vbAaDw== X-RZG-AUTH: ":P2MHfkW8eP4Mre39l357AZT/I7AY/7nT2yrDxb8mjH4JKvMdQv2tRkI16oOSW1Ti/f4PoH8=" Received: from vivo.lan by smtp.strato.de (RZmta 55.5.6 SBL|AUTH) with ESMTPSA id K5281926OAQ6tJL (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits)) (Client did not present a certificate); Fri, 24 Jul 2026 12:26:06 +0200 (CEST) From: Oliver Hartkopp To: stable@vger.kernel.org Cc: Oliver Hartkopp , Marc Kleine-Budde Subject: [PATCH 6.18.y 3/8] can: bcm: extend bcm_tx_lock usage for data and timer updates Date: Fri, 24 Jul 2026 12:25:53 +0200 Message-ID: <20260724102558.17056-3-socketcan@hartkopp.net> X-Mailer: git-send-email 2.53.0 In-Reply-To: <20260724102558.17056-1-socketcan@hartkopp.net> References: <2026072010-winner-hurried-07df@gregkh> <20260724102558.17056-1-socketcan@hartkopp.net> Precedence: bulk X-Mailing-List: stable@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="us-ascii" 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 Link: https://patch.msgid.link/20260714-bcm_fixes-v15-6-562f7e3e42da@hartkopp.net Cc: stable@kernel.org Signed-off-by: Marc Kleine-Budde Signed-off-by: Oliver Hartkopp --- net/can/bcm.c | 104 ++++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 75 insertions(+), 29 deletions(-) diff --git a/net/can/bcm.c b/net/can/bcm.c index 24aca35..0f435d6 100644 --- a/net/can/bcm.c +++ b/net/can/bcm.c @@ -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 */ }; @@ -467,12 +467,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; @@ -489,25 +495,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); } @@ -1031,6 +1059,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 */ /* @@ -1041,11 +1071,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) @@ -1055,36 +1097,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) -- 2.43.0