All of lore.kernel.org
 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 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.