From patchwork Mon Aug 21 14:45:46 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Oliver Hartkopp X-Patchwork-Id: 13359541 X-Patchwork-Delegate: kuba@kernel.org Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (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 4DFA079D6 for ; Mon, 21 Aug 2023 14:46:28 +0000 (UTC) Received: from mo4-p01-ob.smtp.rzone.de (mo4-p01-ob.smtp.rzone.de [85.215.255.50]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 49DABE7; Mon, 21 Aug 2023 07:46:27 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1692629174; cv=none; d=strato.com; s=strato-dkim-0002; b=I1TpIIGr91ipPaWCXE8ultNor9nPzv+Qx2qo60ESMEmZxnb6QlGD1t9MCAmBUVkWsU da277VGixedQsmVhq7F1C1sW7H9KpsTPIJSBKRNyCZUmg6OWy5U8YdqoluQGUO3GeSsX NLqZfedIRy/U16ntGMgT/HUOWx+qvtSwP+br94it4hJt0R++oGfmtAJ7MuMBphHXTHRO Aa3qZW7VsfHrd11mn9tgQUpyQkRV9MI8R0Z8LfXfmc8wwLPfTLPHoTkf55IfjIqdmw0G m52bfN+oEKBDwRcP8qD1rWSn58cEjEW303cyzOTfUQDbH8RHuiFMkWh0m5KSN1FdDiBe C//w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; t=1692629174; 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=ex9cVWZ2rWOuhDT3R/h8fa2ohi4PGlTH7ZxVbD9iUzU=; b=CNxHr3vb5YJd90fIMG7bJxnlNO3UlcsPmS7LSPF+JUWC3vJu89BJ4fyHO5teoCCNzg P77bto738xca/BzldmgZYyhUVidHTXiFyc0fjCXQ6/0FvOQ0SrruW+wdAocoH6kEd1+T BxYQMS6C/cEPOU8t9EAvsG3WyCgCE0lGjGOrbg/Sz1Tpe1KV6ClREJ5P4Tqym2iX5tH9 2GarSQUoSYXcwnhJZLbHmJRFfhOXSD5+kkq/LpcuDrPvD/lrdYcumfTkjROPvTFbWBlN mSYE9ePcOliCbHn9q8+E3UyN+uR8gXrj9jOYVf9o4vTnOrds3idDTQsMFwEFrOIZIsgO 8q6g== ARC-Authentication-Results: i=1; strato.com; arc=none; dkim=none X-RZG-CLASS-ID: mo01 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1692629174; 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=ex9cVWZ2rWOuhDT3R/h8fa2ohi4PGlTH7ZxVbD9iUzU=; b=kcPyz0RBj53p/7r11RFJfyykvahS5/TuopppYNUXiShZnH+SkkcpBys57CmkmUSCwQ B8PMg/Cvm7suhT5II9KAvegqg15NLqNiCmuUlNPOmjsUiyl+kCy67gMQ6PLj/a+SU4Do VRLmhYGVeNNYU7U4aXO2eeN6f/FGXXVw/FOVmoiCgR/+PYrnn7DgvOhtZZ7x0cpNPnN/ bDHFgI7oFZvZ0rkO9tmalpb+eIR/Kvs94uxVdYR8Q5kqgYDw7s/4yXbCe3SL3IS4AsSR VYCai+FZqocVMDRJyDJcWZWEteileNUw7XMhnOL+jGAd0vaeT34n2tMZccaxUz86cflB lhMA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1692629174; 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=ex9cVWZ2rWOuhDT3R/h8fa2ohi4PGlTH7ZxVbD9iUzU=; b=YIt/1Q1LkBJOVNcCbywiRLKlbM6zZWqQOs8OSwDeq5YjkuBuNnwrVU3X7Xr4Vh7TZy 36s90Ytn/cwix2QqpTBQ== X-RZG-AUTH: ":P2MHfkW8eP4Mre39l357AZT/I7AY/7nT2yrDxb8mjGrp7owjzFK3JbFk1mS/xvEBL7X5sbo3VYpXsQi7qV3cmcZPR3l4" Received: from silver.lan by smtp.strato.de (RZmta 49.8.1 AUTH) with ESMTPSA id K723f1z7LEkE0hN (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits)) (Client did not present a certificate); Mon, 21 Aug 2023 16:46:14 +0200 (CEST) From: Oliver Hartkopp To: linux-can@vger.kernel.org, netdev@vger.kernel.org, kuba@kernel.org, edumazet@google.com, mkl@pengutronix.de Cc: Oliver Hartkopp Subject: [NET 1/2] can: isotp: fix support for transmission of SF without flow control Date: Mon, 21 Aug 2023 16:45:46 +0200 Message-Id: <20230821144547.6658-2-socketcan@hartkopp.net> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230821144547.6658-1-socketcan@hartkopp.net> References: <20230821144547.6658-1-socketcan@hartkopp.net> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net X-Patchwork-Delegate: kuba@kernel.org The original implementation had a very simple handling for single frame transmissions as it just sent the single frame without a timeout handling. With the new echo frame handling the echo frame was also introduced for single frames but the former exception ('simple without timers') has been maintained by accident. This leads to a 1 second timeout when closing the socket and to an -ECOMM error when CAN_ISOTP_WAIT_TX_DONE is selected. As the echo handling is always active (also for single frames) remove the wrong extra condition for single frames. Fixes: 9f39d36530e5 ("can: isotp: add support for transmission without flow control") Signed-off-by: Oliver Hartkopp --- net/can/isotp.c | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/net/can/isotp.c b/net/can/isotp.c index 99770ed28531..f02b5d3e4733 100644 --- a/net/can/isotp.c +++ b/net/can/isotp.c @@ -186,16 +186,10 @@ static bool isotp_register_rxid(struct isotp_sock *so) { /* no broadcast modes => register rx_id for FC frame reception */ return (isotp_bc_flags(so) == 0); } -static bool isotp_register_txecho(struct isotp_sock *so) -{ - /* all modes but SF_BROADCAST register for tx echo skbs */ - return (isotp_bc_flags(so) != CAN_ISOTP_SF_BROADCAST); -} - static enum hrtimer_restart isotp_rx_timer_handler(struct hrtimer *hrtimer) { struct isotp_sock *so = container_of(hrtimer, struct isotp_sock, rxtimer); struct sock *sk = &so->sk; @@ -1207,11 +1201,11 @@ static int isotp_release(struct socket *sock) spin_unlock(&isotp_notifier_lock); lock_sock(sk); /* remove current filters & unregister */ - if (so->bound && isotp_register_txecho(so)) { + if (so->bound) { if (so->ifindex) { struct net_device *dev; dev = dev_get_by_index(net, so->ifindex); if (dev) { @@ -1330,18 +1324,16 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len) if (isotp_register_rxid(so)) can_rx_register(net, dev, rx_id, SINGLE_MASK(rx_id), isotp_rcv, sk, "isotp", sk); - if (isotp_register_txecho(so)) { - /* no consecutive frame echo skb in flight */ - so->cfecho = 0; + /* no consecutive frame echo skb in flight */ + so->cfecho = 0; - /* register for echo skb's */ - can_rx_register(net, dev, tx_id, SINGLE_MASK(tx_id), - isotp_rcv_echo, sk, "isotpe", sk); - } + /* register for echo skb's */ + can_rx_register(net, dev, tx_id, SINGLE_MASK(tx_id), + isotp_rcv_echo, sk, "isotpe", sk); dev_put(dev); /* switch to new settings */ so->ifindex = ifindex; @@ -1558,11 +1550,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 && isotp_register_txecho(so)) { + if (so->bound) { if (isotp_register_rxid(so)) can_rx_unregister(dev_net(dev), dev, so->rxid, SINGLE_MASK(so->rxid), isotp_rcv, sk); From patchwork Mon Aug 21 14:45:47 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Oliver Hartkopp X-Patchwork-Id: 13359562 X-Patchwork-Delegate: kuba@kernel.org Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (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 238466FB8 for ; Mon, 21 Aug 2023 14:46:45 +0000 (UTC) Received: from mo4-p01-ob.smtp.rzone.de (mo4-p01-ob.smtp.rzone.de [85.215.255.52]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 154ACE8; Mon, 21 Aug 2023 07:46:42 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1692629175; cv=none; d=strato.com; s=strato-dkim-0002; b=aQCsGcbRfmfGl/q+7g/wiE3yu2ZOGOFEVInQsXO9r6FMTkJKfOUtJq+UAO17jPVNpW 3hzLQelvILzXckVToJgk3XwaUk7Wr6i2SeO7laZvqtfFrICy1s483Kv+rg7XiHQ2elln iyqHkdJ4j4bkPT92lHR8OEQpJunUeIZl6mTvgIaIiJA1TCVdDLt/Wcn0LkOVOJoef8nh 04e9+Dwv4P8gXpsu7I+FWkcUTQ3uzzlWkGvelyPXiuReSd/CFzOPLI0TYp33zKKIhmDc w1J493+xITBwM50nlgA7Pkp3yxlvRJRz+wo64csjwt0RfiHtwkSmgbHJwOY8R2Bjslkb uizQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; t=1692629175; 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=TOZWLA4hvvYY6SCL4NWlwxYl/OkEsC4lztwb1CXM6WY=; b=IfQ6/8pLx6fGv9YnZ0T/XUaROlpvoNURZk3I71r2RtF6NvOSyVZb9hZTaB83hIHLcQ +FZQGS0OTSat/QGrDy9LromM1f22BiUqv/VIzIjby4FVWCjJjvoaGcQRahzyFf6hYjMo 24P10BO75PX5mXb+IJGCUMkFslo8EaYK6+xZDsliDOF3CG31W4EG900jq/EKi550HBcR GiUcBmImyhUB0yu1HkN86ANnnmofSiSYZuc9vQT7kAxQzwGHSRdqugZuXASrZHZjcJRT 6DZVo/Nay8DGI/6mPNCxC1WgYj1/z43rYxcPZBm2EEleuM8HhFnnzQdIOEQmAXye+hGp +WTQ== ARC-Authentication-Results: i=1; strato.com; arc=none; dkim=none X-RZG-CLASS-ID: mo01 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1692629175; 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=TOZWLA4hvvYY6SCL4NWlwxYl/OkEsC4lztwb1CXM6WY=; b=WmPY7xK2R2H0huC4K0Rf4f1O7WuCuXIs4T0SakviJ6szLCLcA50DUj5jTpOyknEvT5 aStSjGH5ZYyX31ILUDk8lX0Csf6AztaAXYNkLchDCHVnnjyYn43J73AVomWtJa8cTIil oKbhySum0J7Tl3WeX6OOjN5CYZFCQVL380LvfpgE2Pyq8O20AgBwpcwuGWzrEPkGxafe cBadKiOb55OzxCYM2KPibC/Fmit1CaM5k8ufnPFG/+KhSXuOSNhUZSUss7pm3I2lLsgL 4f21eGl6y9s20rlxlHiqtXo5EHr+ypP3064md7/swKdtHxV4Gr3kq0CIhwu3XMteaWV4 eSig== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1692629175; 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=TOZWLA4hvvYY6SCL4NWlwxYl/OkEsC4lztwb1CXM6WY=; b=l247Gq9qYmr8QE1AkRo5IeCaeeVPAesYLY/E0FizI0xez8YRrOr7CkVzRrbe7MM4My 00NTYv7Dl3iSpsvJj2Dw== X-RZG-AUTH: ":P2MHfkW8eP4Mre39l357AZT/I7AY/7nT2yrDxb8mjGrp7owjzFK3JbFk1mS/xvEBL7X5sbo3VYpXsQi7qV3cmcZPR3l4" Received: from silver.lan by smtp.strato.de (RZmta 49.8.1 AUTH) with ESMTPSA id K723f1z7LEkE0hO (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits)) (Client did not present a certificate); Mon, 21 Aug 2023 16:46:14 +0200 (CEST) From: Oliver Hartkopp To: linux-can@vger.kernel.org, netdev@vger.kernel.org, kuba@kernel.org, edumazet@google.com, mkl@pengutronix.de Cc: Oliver Hartkopp , Ziyang Xuan Subject: [NET 2/2] can: raw: add missing refcount for memory leak fix Date: Mon, 21 Aug 2023 16:45:47 +0200 Message-Id: <20230821144547.6658-3-socketcan@hartkopp.net> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230821144547.6658-1-socketcan@hartkopp.net> References: <20230821144547.6658-1-socketcan@hartkopp.net> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net X-Patchwork-Delegate: kuba@kernel.org Commit ee8b94c8510c ("can: raw: fix receiver memory leak") introduced a new reference to the CAN netdevice that has assigned CAN filters. But this new ro->dev reference did not maintain its own refcount which lead to another KASAN use-after-free splat found by Eric Dumazet. This patch ensures a proper refcount for the CAN nedevice. Fixes: ee8b94c8510c ("can: raw: fix receiver memory leak") Reported-by: Eric Dumazet Cc: Ziyang Xuan Signed-off-by: Oliver Hartkopp --- net/can/raw.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/net/can/raw.c b/net/can/raw.c index e10f59375659..d50c3f3d892f 100644 --- a/net/can/raw.c +++ b/net/can/raw.c @@ -83,10 +83,11 @@ struct uniqframe { struct raw_sock { struct sock sk; int bound; int ifindex; struct net_device *dev; + netdevice_tracker dev_tracker; struct list_head notifier; int loopback; int recv_own_msgs; int fd_frames; int xl_frames; @@ -283,12 +284,14 @@ static void raw_notify(struct raw_sock *ro, unsigned long msg, switch (msg) { case NETDEV_UNREGISTER: lock_sock(sk); /* remove current filters & unregister */ - if (ro->bound) + if (ro->bound) { raw_disable_allfilters(dev_net(dev), dev, sk); + netdev_put(dev, &ro->dev_tracker); + } if (ro->count > 1) kfree(ro->filter); ro->ifindex = 0; @@ -389,14 +392,16 @@ static int raw_release(struct socket *sock) rtnl_lock(); lock_sock(sk); /* remove current filters & unregister */ if (ro->bound) { - if (ro->dev) + if (ro->dev) { raw_disable_allfilters(dev_net(ro->dev), ro->dev, sk); - else + netdev_put(ro->dev, &ro->dev_tracker); + } else { raw_disable_allfilters(sock_net(sk), NULL, sk); + } } if (ro->count > 1) kfree(ro->filter); @@ -443,44 +448,56 @@ static int raw_bind(struct socket *sock, struct sockaddr *uaddr, int len) if (!dev) { err = -ENODEV; goto out; } if (dev->type != ARPHRD_CAN) { - dev_put(dev); err = -ENODEV; - goto out; + goto out_put_dev; } + if (!(dev->flags & IFF_UP)) notify_enetdown = 1; ifindex = dev->ifindex; /* filters set by default/setsockopt */ err = raw_enable_allfilters(sock_net(sk), dev, sk); - dev_put(dev); + if (err) + goto out_put_dev; + } else { ifindex = 0; /* filters set by default/setsockopt */ err = raw_enable_allfilters(sock_net(sk), NULL, sk); } if (!err) { if (ro->bound) { /* unregister old filters */ - if (ro->dev) + if (ro->dev) { raw_disable_allfilters(dev_net(ro->dev), ro->dev, sk); - else + /* drop reference to old ro->dev */ + netdev_put(ro->dev, &ro->dev_tracker); + } else { raw_disable_allfilters(sock_net(sk), NULL, sk); + } } ro->ifindex = ifindex; ro->bound = 1; + /* bind() ok -> hold a reference for new ro->dev */ ro->dev = dev; + if (ro->dev) + netdev_hold(ro->dev, &ro->dev_tracker, GFP_KERNEL); } - out: +out_put_dev: + /* remove potential reference from dev_get_by_index() */ + if (dev) + dev_put(dev); +out: release_sock(sk); rtnl_unlock(); if (notify_enetdown) { sk->sk_err = ENETDOWN;