From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (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 2ADE7305669 for ; Tue, 7 Jul 2026 14:47:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783435660; cv=none; b=TbR1LUS2WGFJbs/Hs5df6ZFjdnL0XG3jTLvZzCAjwmdJaAObCdrdmcY+6jO74hGg8dED/8RvE3EEtwwdKrbSHIZlFKbJmP4tPHw3Ap+JcCAABwWM7Ggb0VGh9L9D3XcB+bdzmnKOA6bNqY/wtefGkyhmS76tSge9qnhmzS7VkX8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783435660; c=relaxed/simple; bh=U2qCjzmIWTLhsKQ4ced5vlppAezraALGxV0+O2+O9KA=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=I53YSupSXEgeneBInFrGpESTVdTgJtntn1ne6NcAkm/V2MNxSZ6M1euGbN8sRg7c78dFxBcG5ny1ogtBIUJAKPALQkE9ADksx/8/QoYgawWOh62niDpC9QGRUyjHrw7+fRg/ewJJsTLXimRlEnI404B7TlhJvsarkulafbfTWR0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=fw8aSxrQ; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="fw8aSxrQ" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 735321F00A3F; Tue, 7 Jul 2026 14:47:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1783435658; bh=zBKhJ+AQ2j8+UYq/206u9PU6DC5knbJVwu2hey1SZOE=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date; b=fw8aSxrQWuuDKwfP/IfIlmjuoL8vSqTybNU27WyLnCS5kAxsvSrJFp27mZmWXJMcc QrtXW+GKzMyl62NFLWw3BtvZeSRpcER1YlylFU6bGGTPXZEzTJG0NPtfqzx54IhcAu c5/LtqxBUcw2nMmVg1g/2bspPyJQjqUhtk+29OXtSTBFrL2im1wpC5IwM/5UXP82tP 3JoP0PQHIoGwEs6S5iSbiFUEy05igpB89zBR1wsF7o6kI+GzW/1hUyJ/yaOAxzGrED aKMP5YbpoapUZcnihmvWqBnSvj0n9GMB1qyT+lZZ9bHJ+Dacf3OB+Gp5LFx+8qah2Q LkacKQmxIJWHw== From: sashiko-bot@kernel.org Subject: Re: [PATCH v4 1/7] can: bcm: defer rx_op deallocation to workqueue to fix thrtimer UAF Reply-To: sashiko-reviews@lists.linux.dev To: "Oliver Hartkopp" Cc: "Oliver Hartkopp" , linux-can@vger.kernel.org, "Marc Kleine-Budde" , "Vincent Mailhol" , "Oleksij Rempel" In-Reply-To: <20260707-bcm_fixes-v4-1-aa87b8b9d33f@hartkopp.net> References: <20260707-bcm_fixes-v4-0-aa87b8b9d33f@hartkopp.net> <20260707-bcm_fixes-v4-1-aa87b8b9d33f@hartkopp.net> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Tue, 07 Jul 2026 14:47:38 +0000 Message-Id: <20260707144738.735321F00A3F@smtp.kernel.org> Precedence: bulk X-Mailing-List: linux-can@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Thank you for your contribution! Sashiko AI review found 1 potential issue(= s) to consider: Pre-existing issues: - [High] Concurrent procfs reads can crash the kernel due to unsafe lockles= s list operations on `bo->rx_ops` and `bo->tx_ops`. -- --- Patch [1]: [PATCH v4 1/7] can: bcm: defer rx_op deallocation to workque= ue to fix thrtimer UAF --- commit 784fd8a5d6989051af26ae5075b37f80eb5ecce7 Author: Lee Jones can: bcm: defer rx_op deallocation to workqueue to fix thrtimer UAF This commit resolves a use-after-free issue by deferring the cancellation of timers and the deallocation of BCM operations to an unbound workqueue. It takes an additional socket reference to ensure the socket outlives the deferred work. > diff --git a/net/can/bcm.c b/net/can/bcm.c > index a4bef2c48a559..bdf53241bd7b3 100644 > --- a/net/can/bcm.c > +++ b/net/can/bcm.c [ ... ] > @@ -1060,6 +1081,7 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_he= ad, struct msghdr *msg, > =20 > /* bcm_can_tx / bcm_tx_timeout_handler needs this */ > op->sk =3D sk; > + sock_hold(sk); > op->ifindex =3D ifindex; > =20 > /* initialize uninitialized (kzalloc) structure */ [Severity: High] This is a pre-existing issue, but can concurrent procfs reads crash the ker= nel due to unsafe lockless list operations on bo->tx_ops? Later in bcm_tx_setup(), the new op is added to the list: list_add(&op->list, &bo->tx_ops); However, bcm_proc_show() locklessly iterates bo->tx_ops using the non-RCU macro list_for_each_entry() instead of list_for_each_entry_rcu(), and bcm_tx_setup() uses list_add() instead of list_add_rcu(). Because list_add() lacks the smp_store_release() memory barrier of list_add_rcu(), and because the tx_ops traversal lacks rcu_dereference() to enforce data dependencies, the reader CPU can observe the new list node before its fields or next pointer are initialized. Could this lead to dereferencing uninitialized memory or a corrupted next pointer if a local user repeatedly reads /proc/net/can-bcm/ while a thread creates new TX operations via sendmsg()? [ ... ] > @@ -1221,6 +1243,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_he= ad, struct msghdr *msg, > =20 > /* bcm_can_tx / bcm_tx_timeout_handler needs this */ > op->sk =3D sk; > + sock_hold(sk); > op->ifindex =3D ifindex; [Severity: High] This is also a pre-existing issue, but a similar hazard exists for bo->rx_o= ps. Later in bcm_rx_setup(), the code inserts into the list without RCU barrier= s: list_add(&op->list, &bo->rx_ops); While bcm_proc_show() does use list_for_each_entry_rcu() for rx_ops (unlike tx_ops), bcm_rx_setup() still uses list_add() instead of list_add_rcu(). Without the smp_store_release() memory barrier provided by list_add_rcu(), concurrent procfs reads can still observe the new node before its contents are fully initialized. Should these insertions be upgraded to use list_add_rcu() to match the lockless RCU iteration in procfs? --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260707-bcm_fixes-= v4-0-aa87b8b9d33f@hartkopp.net?part=3D1