From mboxrd@z Thu Jan 1 00:00:00 1970 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 714DCC433F5 for ; Wed, 15 Dec 2021 13:55:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S243013AbhLONzo (ORCPT ); Wed, 15 Dec 2021 08:55:44 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42352 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232338AbhLONzn (ORCPT ); Wed, 15 Dec 2021 08:55:43 -0500 Received: from ams.source.kernel.org (ams.source.kernel.org [IPv6:2604:1380:4601:e00::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D5A34C061574; Wed, 15 Dec 2021 05:55:42 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 91093B81F05; Wed, 15 Dec 2021 13:55:41 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B0CC8C34604; Wed, 15 Dec 2021 13:55:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1639576540; bh=Xw/T47Xd/1MGbXKzyKiQWGb6WlKQuClEoedkpf+Imv4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=K/WnvhvtWXHeiHZHHdrPqHT7Vy2uisx2DoMXd+WDk8SGeFGxZP08aktJR/iufmjtk lGUPk6MrT172tdni9yM3lJ3nI6y4ru8FBRM9q05EcSAdjElq85j0dNBo9kC7NQeCrV QORHS3xyP7nFdFdfTQJbWzogtuLtpN0cQOzmwrG4= Date: Wed, 15 Dec 2021 14:55:37 +0100 From: Greg KH To: Antoine Tenart Cc: davem@davemloft.net, kuba@kernel.org, netdev@vger.kernel.org, stable@vger.kernel.org Subject: Re: [PATCH 5.10] ethtool: do not perform operations on net devices being unregistered Message-ID: References: <20211213101506.118377-1-atenart@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20211213101506.118377-1-atenart@kernel.org> Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Mon, Dec 13, 2021 at 11:15:06AM +0100, Antoine Tenart wrote: > commit dde91ccfa25fd58f64c397d91b81a4b393100ffa upstream > > There is a short period between a net device starts to be unregistered > and when it is actually gone. In that time frame ethtool operations > could still be performed, which might end up in unwanted or undefined > behaviours[1]. > > Do not allow ethtool operations after a net device starts its > unregistration. This patch targets the netlink part as the ioctl one > isn't affected: the reference to the net device is taken and the > operation is executed within an rtnl lock section and the net device > won't be found after unregister. > > [1] For example adding Tx queues after unregister ends up in NULL > pointer exceptions and UaFs, such as: > > BUG: KASAN: use-after-free in kobject_get+0x14/0x90 > Read of size 1 at addr ffff88801961248c by task ethtool/755 > > CPU: 0 PID: 755 Comm: ethtool Not tainted 5.15.0-rc6+ #778 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-4.fc34 04/014 > Call Trace: > dump_stack_lvl+0x57/0x72 > print_address_description.constprop.0+0x1f/0x140 > kasan_report.cold+0x7f/0x11b > kobject_get+0x14/0x90 > kobject_add_internal+0x3d1/0x450 > kobject_init_and_add+0xba/0xf0 > netdev_queue_update_kobjects+0xcf/0x200 > netif_set_real_num_tx_queues+0xb4/0x310 > veth_set_channels+0x1c3/0x550 > ethnl_set_channels+0x524/0x610 > > [The patch differs from the upstream one as code was moved around by > commit 41107ac22fcf ("ethtool: move netif_device_present check from > ethnl_parse_header_dev_get to ethnl_ops_begin"). The check on the netdev > state is still done in ethnl_ops_begin as it must be done in an rtnl > section (the one which performs the op) to not race with > unregister_netdevice_many. > Also note the trace in [1] is not possible here as the channel ops for > veth were added later, but that was just one example.] > > Fixes: 041b1c5d4a53 ("ethtool: helper functions for netlink interface") > Suggested-by: Jakub Kicinski > Signed-off-by: Antoine Tenart > --- > > Hello, > > This patch is intended for the stable 5.10 tree. > > As reported by Greg, patch dde91ccfa25f ("ethtool: do not perform > operations on net devices being unregistered") did not apply correctly > on the 5.10 tree. The explanation of this and the approach taken here is > explained in the above commit log, between []. > > I removed the Link tag and Signed-off-by from Jakub from the original > patch as this one is slightly different in its implementation. > > Thanks, > Antoine > > net/ethtool/netlink.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h > index d8efec516d86..979dee6bb88c 100644 > --- a/net/ethtool/netlink.h > +++ b/net/ethtool/netlink.h > @@ -249,6 +249,9 @@ struct ethnl_reply_data { > > static inline int ethnl_ops_begin(struct net_device *dev) > { > + if (dev && dev->reg_state == NETREG_UNREGISTERING) > + return -ENODEV; > + > if (dev && dev->ethtool_ops->begin) > return dev->ethtool_ops->begin(dev); > else > -- > 2.33.1 > Now queued up, thanks. greg k-h