netdev - Re: netif_set_xps_queue() before register_netdev(): correct from an API perspective?

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: <CAKgT0UcDXUFDzVjOj4EkVRoz=zdro+hQx877dvhACMwVnjAagw@mail.gmail.com>
Date: Wed, 2 Aug 2023 08:04:01 -0700
From: Alexander Duyck <alexander.duyck@...il.com>
To: Vladimir Oltean <vladimir.oltean@....com>
Cc: Ioana Ciornei <ioana.ciornei@....com>, Antoine Tenart <atenart@...nel.org>, netdev@...r.kernel.org
Subject: Re: netif_set_xps_queue() before register_netdev(): correct from an
 API perspective?

On Wed, Aug 2, 2023 at 7:57 AM Vladimir Oltean <vladimir.oltean@....com> wrote:
>
> Hi,
>
> When drivers/net/ethernet/freescale/dpaa2/ fails to probe (including -EPROBE_DEFER),
> I see lots of these:
>
> $ cat /sys/kernel/debug/kmemleak
> unreferenced object 0xffff042845fbdac0 (size 64):
>   comm "kworker/u16:1", pid 56, jiffies 4294893690 (age 859.844s)
>   hex dump (first 32 bytes):
>     01 00 00 00 14 00 00 00 00 00 00 00 00 00 00 00  ................
>     00 00 00 00 00 00 00 00 06 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<ffffc754095a7dfc>] slab_post_alloc_hook+0xa4/0x330
>     [<ffffc754095a654c>] __kmem_cache_alloc_node+0x23c/0x308
>     [<ffffc7540952fec0>] __kmalloc_node+0xc0/0x240
>     [<ffffc7540a976934>] __netif_set_xps_queue+0x32c/0xa78
>     [<ffffc7540a9771e4>] netif_set_xps_queue+0x44/0x70
>     [<ffffc7540a1c3540>] update_xps+0xb0/0x158
>     [<ffffc7540a1c0290>] dpaa2_eth_probe+0xd10/0x1368
>     [<ffffc75409b1677c>] fsl_mc_driver_probe+0x2c/0x80
>     [<ffffc75409eb764c>] really_probe+0x13c/0x2f8
>     [<ffffc75409eb666c>] __driver_probe_device+0xac/0x140
>     [<ffffc75409eb7340>] driver_probe_device+0x48/0x218
>     [<ffffc75409eb71d8>] __device_attach_driver+0x128/0x158
>     [<ffffc75409eb33e4>] bus_for_each_drv+0x12c/0x160
>     [<ffffc75409eb63f4>] __device_attach+0xcc/0x1a0
>     [<ffffc75409eb64e8>] device_initial_probe+0x20/0x38
>     [<ffffc75409eb3620>] bus_probe_device+0xa0/0x118
>
> I see that netif_set_xps_queue() allocates dev->xps_maps[], which is
> eventually freed by reset_xps_maps() <- ... <- netif_reset_xps_queues_gt()
> from a number of call sites, including from unregister_netdevice_many_notify().
>
> This is nice, but dpaa2 (above) and emulex/benet/be_main.c call
> netif_set_xps_queue() prior to registering the net device. So no
> deregistration event will take place.
>
> How is the memory supposed to be freed in this case?

We really shouldn't be calling it before the netdev is registered.

That said though the easiest way to clean it up would probably be to
call netdev_reset_tc in case of a failure.