Re: [PATCH net] net-sysfs: update the queue counts in the unregistration path

On Mon, 29 Nov 2021 16:45:20 +0100 Antoine Tenart wrote:

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.

Would you mind pointing where in the code that happens? I can't seem 
to find anything looking at real_num_.x_queues outside dev.c and
net-sysfs.c :S

Fixes: 5c56580b74e5 ("net: Adjust TX queue kobjects if number of queues changes during unregister")
Signed-off-by: Antoine Tenart <atenart@kernel.org>
---
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.