From: Antoine Tenart <atenart@kernel.org>
To: davem@davemloft.net, kuba@kernel.org
Cc: Antoine Tenart <atenart@kernel.org>,
alexander.duyck@gmail.com, netdev@vger.kernel.org
Subject: [PATCH net] net-sysfs: update the queue counts in the unregistration path
Date: Mon, 29 Nov 2021 16:45:20 +0100 [thread overview]
Message-ID: <20211129154520.295823-1-atenart@kernel.org> (raw)
When updating Rx and Tx queue kobjects, the queue count should always be
updated to match the queue kobjects count. This was not done in the net
device unregistration path and due to the Tx queue logic allowing
updates when unregistering (for qdisc cleanup) it was possible with
ethtool to manually add new queues after unregister, leading to 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
Updating the queue counts in the unregistration path solve the above
issue, as the ethtool path updating the queue counts makes sanity checks
and a queue count of 0 should prevent any update.
Fixes: 5c56580b74e5 ("net: Adjust TX queue kobjects if number of queues changes during unregister")
Signed-off-by: Antoine Tenart <atenart@kernel.org>
---
Hello,
Following a previous thread[1] I had another look at this issue and now
have a better fix (this patch). In this previous thread we also
discussed preventing ethtool operations after unregister and adding a
warning in netdev_queue_update_kobjects; I'll send two patches for this
but targetting net-next.
Thanks!
Antoine
[1] https://lore.kernel.org/all/20211122162007.303623-1-atenart@kernel.org/T/
net/core/net-sysfs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 9c01c642cf9e..d7f9ee830d34 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -1820,6 +1820,9 @@ static void remove_queue_kobjects(struct net_device *dev)
net_rx_queue_update_kobjects(dev, real_rx, 0);
netdev_queue_update_kobjects(dev, real_tx, 0);
+
+ dev->real_num_rx_queues = 0;
+ dev->real_num_tx_queues = 0;
#ifdef CONFIG_SYSFS
kset_unregister(dev->queues_kset);
#endif
--
2.33.1
next reply other threads:[~2021-11-29 19:27 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-29 15:45 Antoine Tenart [this message]
2021-12-01 2:08 ` [PATCH net] net-sysfs: update the queue counts in the unregistration path Jakub Kicinski
2021-12-01 9:32 ` Antoine Tenart
2021-12-01 15:05 ` Jakub Kicinski
2021-12-01 17:03 ` Antoine Tenart
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211129154520.295823-1-atenart@kernel.org \
--to=atenart@kernel.org \
--cc=alexander.duyck@gmail.com \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox