From patchwork Wed Feb 16 06:31:36 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Oliver Hartkopp X-Patchwork-Id: 543334 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 32D9DC433F5 for ; Wed, 16 Feb 2022 07:22:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229968AbiBPHWy (ORCPT ); Wed, 16 Feb 2022 02:22:54 -0500 Received: from gmail-smtp-in.l.google.com ([23.128.96.19]:38338 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230002AbiBPHWw (ORCPT ); Wed, 16 Feb 2022 02:22:52 -0500 X-Greylist: delayed 572 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Tue, 15 Feb 2022 23:22:36 PST Received: from mo4-p00-ob.smtp.rzone.de (mo4-p00-ob.smtp.rzone.de [81.169.146.216]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C25EAD5 for ; Tue, 15 Feb 2022 23:22:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1644993151; s=strato-dkim-0002; d=hartkopp.net; h=Message-Id:Date:Subject:Cc:To:From:Cc:Date:From:Subject:Sender; bh=XW7Vp0hhhdyNox3vT96fEpwyFA9OtgyB00lFK37Dzyw=; b=Sd/s4sCa+p1qIJYYMwuXDmvV8DLVSjQZSMq41baUUkkM7d0DQjs0UIguQhvS5XSyzg ozP2aBU5rs/+5Wj/LK7VWXr0by066Vr6cLe/l2/x2stf+G8Q3J1rwVbhBdaorjno/Skk zFZIGpWFvWr10lj/wJumdf4hyLqAlqW36rQnb0YFbj7hsstut/sTxh/qyISKOfy9SsFO saYcf8sEmasRA64rh35LFNn0LQe+6rxQHmBd0QQjSOFCRoNMo67Xp22dtcAreeFzoeMI YBpbmdG+rzwxQwuEkWG15D0rt33H+LiIur01CZIDwOcEpQZCqzHuNIzXj+hf7I3hky22 H5xg== Authentication-Results: strato.com; dkim=none X-RZG-AUTH: ":P2MHfkW8eP4Mre39l357AZT/I7AY/7nT2yrDxb8mjGrp7owjzFK3JbFk1mS/xvEBL7X5sbo3UIh9IyLecSWJafUvprl4" X-RZG-CLASS-ID: mo00 Received: from silver.lan by smtp.strato.de (RZmta 47.39.0 AUTH) with ESMTPSA id L7379cy1G6WUfoi (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits)) (Client did not present a certificate); Wed, 16 Feb 2022 07:32:30 +0100 (CET) From: Oliver Hartkopp To: gregkh@linuxfoundation.org, stable@vger.kernel.org Cc: Oliver Hartkopp , Norbert Slusarek , Thadeu Lima de Souza Cascardo , Marc Kleine-Budde Subject: [BACKPORT stable Linux-5.10.y 1/2] can: isotp: prevent race between isotp_bind() and isotp_setsockopt() Date: Wed, 16 Feb 2022 07:31:36 +0100 Message-Id: <20220216063137.2023-1-socketcan@hartkopp.net> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org Upstream commit 2b17c400aeb44daf041627722581ade527bb3c1d The fixes tag of the uptream patch points to commit 921ca574cd38 ("can: isotp: add SF_BROADCAST support for functional addressing") which showed up in Linux 5.11 but the described issue already existed in Linux 5.10. Norbert Slusarek writes: A race condition was found in isotp_setsockopt() which allows to change socket options after the socket was bound. For the specific case of SF_BROADCAST support, this might lead to possible use-after-free because can_rx_unregister() is not called. Checking for the flag under the socket lock in isotp_bind() and taking the lock in isotp_setsockopt() fixes the issue. Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol") Link: https://lore.kernel.org/r/trinity-e6ae9efa-9afb-4326-84c0-f3609b9b8168-1620773528307@3c-app-gmx-bs06 Reported-by: Norbert Slusarek Signed-off-by: Thadeu Lima de Souza Cascardo Signed-off-by: Norbert Slusarek Acked-by: Oliver Hartkopp Signed-off-by: Marc Kleine-Budde Signed-off-by: Oliver Hartkopp --- net/can/isotp.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/net/can/isotp.c b/net/can/isotp.c index 37db4d232313..3f11d2b314b6 100644 --- a/net/can/isotp.c +++ b/net/can/isotp.c @@ -1191,20 +1191,17 @@ static int isotp_getname(struct socket *sock, struct sockaddr *uaddr, int peer) addr->can_addr.tp.tx_id = so->txid; return ISOTP_MIN_NAMELEN; } -static int isotp_setsockopt(struct socket *sock, int level, int optname, +static int isotp_setsockopt_locked(struct socket *sock, int level, int optname, sockptr_t optval, unsigned int optlen) { struct sock *sk = sock->sk; struct isotp_sock *so = isotp_sk(sk); int ret = 0; - if (level != SOL_CAN_ISOTP) - return -EINVAL; - if (so->bound) return -EISCONN; switch (optname) { case CAN_ISOTP_OPTS: @@ -1275,10 +1272,26 @@ static int isotp_setsockopt(struct socket *sock, int level, int optname, } return ret; } +static int isotp_setsockopt(struct socket *sock, int level, int optname, + sockptr_t optval, unsigned int optlen) + +{ + struct sock *sk = sock->sk; + int ret; + + if (level != SOL_CAN_ISOTP) + return -EINVAL; + + lock_sock(sk); + ret = isotp_setsockopt_locked(sock, level, optname, optval, optlen); + release_sock(sk); + return ret; +} + static int isotp_getsockopt(struct socket *sock, int level, int optname, char __user *optval, int __user *optlen) { struct sock *sk = sock->sk; struct isotp_sock *so = isotp_sk(sk); From patchwork Wed Feb 16 06:31:37 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Oliver Hartkopp X-Patchwork-Id: 543335 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 63E6DC433F5 for ; Wed, 16 Feb 2022 07:13:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229599AbiBPHNY (ORCPT ); Wed, 16 Feb 2022 02:13:24 -0500 Received: from gmail-smtp-in.l.google.com ([23.128.96.19]:40348 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229754AbiBPHNW (ORCPT ); Wed, 16 Feb 2022 02:13:22 -0500 Received: from mo4-p01-ob.smtp.rzone.de (mo4-p01-ob.smtp.rzone.de [85.215.255.53]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5E49DA41B1 for ; Tue, 15 Feb 2022 23:13:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1644993151; 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=Yw44AETHr1cA85Pdp4DVNrbEVYyJ2DcIQvFytrlyT8o=; b=gu2GFbObVBCVZ1IsEZ8uyhr9uDReMY58MPo4CKx6A8biw3qCJs8SVFpYk0fuDNKt/U cEhq+HEDCCBazZrU20kkDx8YFXSdP0jZfhrV6n31pZ/6rQiJYna0j/XPHSU3ty+pMcnS kRmEVv40jOjwfnbPQcyHFdhYIIWXDTh3wDrN1mxGWv/YkLsPq3sBautwg0ju9FFPIO8A od3vnjzICKGNCZ4VebgcDh+v4zTDf0IjK+zisj1YdI/htN4an9i3CSAZSfUdnFsnhc4A GBjGl5QZDrDfnpsrbPyFFmcN6Ehy3eci0eLMuLggFtlRMFCQIGNg7cOiTFZyCYhqYsH+ eRFw== Authentication-Results: strato.com; dkim=none X-RZG-AUTH: ":P2MHfkW8eP4Mre39l357AZT/I7AY/7nT2yrDxb8mjGrp7owjzFK3JbFk1mS/xvEBL7X5sbo3UIh9IyLecSWJafUvprl4" X-RZG-CLASS-ID: mo00 Received: from silver.lan by smtp.strato.de (RZmta 47.39.0 AUTH) with ESMTPSA id L7379cy1G6WVfoj (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits)) (Client did not present a certificate); Wed, 16 Feb 2022 07:32:31 +0100 (CET) From: Oliver Hartkopp To: gregkh@linuxfoundation.org, stable@vger.kernel.org Cc: Oliver Hartkopp , Thomas Wagner , Marc Kleine-Budde Subject: [BACKPORT stable Linux-5.10.y 2/2] can: isotp: add SF_BROADCAST support for functional addressing Date: Wed, 16 Feb 2022 07:31:37 +0100 Message-Id: <20220216063137.2023-2-socketcan@hartkopp.net> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220216063137.2023-1-socketcan@hartkopp.net> References: <20220216063137.2023-1-socketcan@hartkopp.net> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org Upstream commit 921ca574cd382142add8b12d0a7117f495510de5 The patch was intended for 5.10 but missed the merge window by some days. This missing patch continously breaks the backport of stable fixes and is the only missing feature of upstream isotp in Linux 5.10 e.g. for RasPi. When CAN_ISOTP_SF_BROADCAST is set in the CAN_ISOTP_OPTS flags the CAN_ISOTP socket is switched into functional addressing mode, where only single frame (SF) protocol data units can be send on the specified CAN interface and the given tp.tx_id after bind(). In opposite to normal and extended addressing this socket does not register a CAN-ID for reception which would be needed for a 1-to-1 ISOTP connection with a segmented bi-directional data transfer. Sending SFs on this socket is therefore a TX-only 'broadcast' operation. Signed-off-by: Oliver Hartkopp Signed-off-by: Thomas Wagner Link: https://lore.kernel.org/r/20201206144731.4609-1-socketcan@hartkopp.net Signed-off-by: Marc Kleine-Budde --- include/uapi/linux/can/isotp.h | 2 +- net/can/isotp.c | 50 ++++++++++++++++++++++++---------- 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/include/uapi/linux/can/isotp.h b/include/uapi/linux/can/isotp.h index 7793b26aa154..c55935b64ccc 100644 --- a/include/uapi/linux/can/isotp.h +++ b/include/uapi/linux/can/isotp.h @@ -133,11 +133,11 @@ struct can_isotp_ll_options { #define CAN_ISOTP_HALF_DUPLEX 0x040 /* half duplex error state handling */ #define CAN_ISOTP_FORCE_TXSTMIN 0x080 /* ignore stmin from received FC */ #define CAN_ISOTP_FORCE_RXSTMIN 0x100 /* ignore CFs depending on rx stmin */ #define CAN_ISOTP_RX_EXT_ADDR 0x200 /* different rx extended addressing */ #define CAN_ISOTP_WAIT_TX_DONE 0x400 /* wait for tx completion */ - +#define CAN_ISOTP_SF_BROADCAST 0x800 /* 1-to-N functional addressing */ /* default values */ #define CAN_ISOTP_DEFAULT_FLAGS 0 #define CAN_ISOTP_DEFAULT_EXT_ADDRESS 0x00 diff --git a/net/can/isotp.c b/net/can/isotp.c index 3f11d2b314b6..d0581dc6a65f 100644 --- a/net/can/isotp.c +++ b/net/can/isotp.c @@ -886,10 +886,20 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) if (!size || size > MAX_MSG_LENGTH) { err = -EINVAL; goto err_out_drop; } + /* take care of a potential SF_DL ESC offset for TX_DL > 8 */ + off = (so->tx.ll_dl > CAN_MAX_DLEN) ? 1 : 0; + + /* does the given data fit into a single frame for SF_BROADCAST? */ + if ((so->opt.flags & CAN_ISOTP_SF_BROADCAST) && + (size > so->tx.ll_dl - SF_PCI_SZ4 - ae - off)) { + err = -EINVAL; + goto err_out_drop; + } + err = memcpy_from_msg(so->tx.buf, msg, size); if (err < 0) goto err_out_drop; dev = dev_get_by_index(sock_net(sk), so->ifindex); @@ -913,13 +923,10 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) so->tx.idx = 0; cf = (struct canfd_frame *)skb->data; skb_put_zero(skb, so->ll.mtu); - /* take care of a potential SF_DL ESC offset for TX_DL > 8 */ - off = (so->tx.ll_dl > CAN_MAX_DLEN) ? 1 : 0; - /* check for single frame transmission depending on TX_DL */ if (size <= so->tx.ll_dl - SF_PCI_SZ4 - ae - off) { /* The message size generally fits into a SingleFrame - good. * * SF_DL ESC offset optimization: @@ -1055,11 +1062,11 @@ static int isotp_release(struct socket *sock) spin_unlock(&isotp_notifier_lock); lock_sock(sk); /* remove current filters & unregister */ - if (so->bound) { + if (so->bound && (!(so->opt.flags & CAN_ISOTP_SF_BROADCAST))) { if (so->ifindex) { struct net_device *dev; dev = dev_get_by_index(net, so->ifindex); if (dev) { @@ -1095,26 +1102,40 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len) struct net *net = sock_net(sk); int ifindex; struct net_device *dev; int err = 0; int notify_enetdown = 0; + int do_rx_reg = 1; if (len < ISOTP_MIN_NAMELEN) return -EINVAL; - if (addr->can_addr.tp.rx_id == addr->can_addr.tp.tx_id) - return -EADDRNOTAVAIL; - - if ((addr->can_addr.tp.rx_id | addr->can_addr.tp.tx_id) & - (CAN_ERR_FLAG | CAN_RTR_FLAG)) + if (addr->can_addr.tp.tx_id & (CAN_ERR_FLAG | CAN_RTR_FLAG)) return -EADDRNOTAVAIL; if (!addr->can_ifindex) return -ENODEV; lock_sock(sk); + /* do not register frame reception for functional addressing */ + if (so->opt.flags & CAN_ISOTP_SF_BROADCAST) + do_rx_reg = 0; + + /* do not validate rx address for functional addressing */ + if (do_rx_reg) { + if (addr->can_addr.tp.rx_id == addr->can_addr.tp.tx_id) { + err = -EADDRNOTAVAIL; + goto out; + } + + if (addr->can_addr.tp.rx_id & (CAN_ERR_FLAG | CAN_RTR_FLAG)) { + err = -EADDRNOTAVAIL; + goto out; + } + } + if (so->bound && addr->can_ifindex == so->ifindex && addr->can_addr.tp.rx_id == so->rxid && addr->can_addr.tp.tx_id == so->txid) goto out; @@ -1136,17 +1157,18 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len) if (!(dev->flags & IFF_UP)) notify_enetdown = 1; ifindex = dev->ifindex; - can_rx_register(net, dev, addr->can_addr.tp.rx_id, - SINGLE_MASK(addr->can_addr.tp.rx_id), isotp_rcv, sk, - "isotp", sk); + if (do_rx_reg) + can_rx_register(net, dev, addr->can_addr.tp.rx_id, + SINGLE_MASK(addr->can_addr.tp.rx_id), + isotp_rcv, sk, "isotp", sk); dev_put(dev); - if (so->bound) { + if (so->bound && do_rx_reg) { /* unregister old filter */ if (so->ifindex) { dev = dev_get_by_index(net, so->ifindex); if (dev) { can_rx_unregister(net, dev, so->rxid, @@ -1355,11 +1377,11 @@ static void isotp_notify(struct isotp_sock *so, unsigned long msg, switch (msg) { case NETDEV_UNREGISTER: lock_sock(sk); /* remove current filters & unregister */ - if (so->bound) + if (so->bound && (!(so->opt.flags & CAN_ISOTP_SF_BROADCAST))) can_rx_unregister(dev_net(dev), dev, so->rxid, SINGLE_MASK(so->rxid), isotp_rcv, sk); so->ifindex = 0;