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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20211130180839.285e31be@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Tue, 30 Nov 2021 18:08:39 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Antoine Tenart <atenart@...nel.org>
Cc:     davem@...emloft.net, alexander.duyck@...il.com,
        netdev@...r.kernel.org
Subject: 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@...nel.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.