Netdev List
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: atenart@kernel.org
Cc: kuba@kernel.org, pabeni@redhat.com, netdev@vger.kernel.org
Subject: Re: [net] net-sysfs: avoid registering new queue objects after device unregistration
Date: Tue, 26 Oct 2021 15:30:57 +0100 (BST)	[thread overview]
Message-ID: <20211026.153057.208749798584527471.davem@davemloft.net> (raw)
In-Reply-To: <20211026133822.949135-1-atenart@kernel.org>

From: Antoine Tenart <atenart@kernel.org>
Date: Tue, 26 Oct 2021 15:38:22 +0200

> 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>

netdev_queue_update_kobjects is a confusing function name, it sounds like it handles both rx and tx.
It only handles tx so net_tx_queue_update_kobjects is more appropriate.

Could you rename the function in this patch please?

Thank you.

  reply	other threads:[~2021-10-26 14:31 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-26 13:38 [net] net-sysfs: avoid registering new queue objects after device unregistration Antoine Tenart
2021-10-26 14:30 ` David Miller [this message]
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=20211026.153057.208749798584527471.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=atenart@kernel.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox