From: Antoine Tenart <atenart@kernel.org>
To: davem@davemloft.net, kuba@kernel.org
Cc: Antoine Tenart <atenart@kernel.org>,
pabeni@redhat.com, netdev@vger.kernel.org
Subject: [net] net-sysfs: avoid registering new queue objects after device unregistration
Date: Tue, 26 Oct 2021 15:38:22 +0200 [thread overview]
Message-ID: <20211026133822.949135-1-atenart@kernel.org> (raw)
netdev_queue_update_kobjects can be called after device unregistration
started (and device_del was called) resulting in two issues: possible
registration of new queue kobjects (leading to the following trace) and
providing a wrong 'old_num' number (because real_num_tx_queues is not
updated in the unregistration path).
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 fix for both is to only allow unregistering queue kobjects after a
net device started its unregistration and to ensure we know the current
Tx queue number (we update dev->real_num_tx_queues before returning).
This relies on the fact that dev->real_num_tx_queues is used for
'old_num' expect when firstly allocating queues.
(Rx queues are not affected as net_rx_queue_update_kobjects can't be
called after a net device started its unregistration).
Fixes: 5c56580b74e5 ("net: Adjust TX queue kobjects if number of queues changes during unregister")
Signed-off-by: Antoine Tenart <atenart@kernel.org>
---
Hi,
The 'dev->real_num_tx_queues' update is usually done (or is not needed)
when calling netdev_queue_update_kobjects and I could have added one in
the unregistration path (where it's missing). But I thought this would
be easier to backport for stable and better to tie it with the call of
this function: a following-up patch for net-next cleaning the now
duplicate updates can be made, instead.
To avoid future issues like this and invalid number of queues given to
netdev_queue_update_kobjects we could introduce a warning like (and a
similar one for Rx queues):
WARN_ON(old_num != 0 && dev->real_num_tx_queues != old_num);
A better fix would be to remove 'old_num' from the function's parameters
and to always use dev->real_num_tx_queues; but it's not that
straightforward: the value can be set before the net device is
registered (and used elsewhere) and when firstly calling
netdev_queue_update_kobjects dev->real_num_tx_queues can not be relied
on. Anyway if possible that would not be a good candidate for a fix.
Thanks,
Antoine
net/core/net-sysfs.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index b2e49eb7001d..2e5c89c7e3c9 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -1651,6 +1651,12 @@ netdev_queue_update_kobjects(struct net_device *dev, int old_num, int new_num)
int i;
int error = 0;
+ /* When unregistering a net device we can't register new kobjects, as
+ * this would result in an UaF of the device's kobj.
+ */
+ if (dev->reg_state == NETREG_UNREGISTERING && new_num > old_num)
+ return -EINVAL;
+
for (i = old_num; i < new_num; i++) {
error = netdev_queue_add_kobject(dev, i);
if (error) {
@@ -1670,6 +1676,7 @@ netdev_queue_update_kobjects(struct net_device *dev, int old_num, int new_num)
kobject_put(&queue->kobj);
}
+ dev->real_num_tx_queues = new_num;
return error;
#else
return 0;
--
2.31.1
next reply other threads:[~2021-10-26 13:38 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-26 13:38 Antoine Tenart [this message]
2021-10-26 14:30 ` [net] net-sysfs: avoid registering new queue objects after device unregistration David Miller
2021-10-26 14:37 ` Antoine Tenart
2021-11-05 10:39 ` 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=20211026133822.949135-1-atenart@kernel.org \
--to=atenart@kernel.org \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.