Re: [PATCH net-next v2] usbnet: Fix linkwatch use-after-free on disconnect

[Oliver Neukum <oneukum@suse.com>]

On 23.06.22 14:50, Lukas Wunner wrote:
> usbnet uses the work usbnet_deferred_kevent() to perform tasks which may
> sleep.  On disconnect, completion of the work was originally awaited in
> ->ndo_stop().  But in 2003, that was moved to ->disconnect() by historic
> commit "[PATCH] USB: usbnet, prevent exotic rtnl deadlock":
> 
>   https://git.kernel.org/tglx/history/c/0f138bbfd83c
> 
> The change was made because back then, the kernel's workqueue
> implementation did not allow waiting for a single work.  One had to wait
> for completion of *all* work by calling flush_scheduled_work(), and that
> could deadlock when waiting for usbnet_deferred_kevent() with rtnl_mutex
> held in ->ndo_stop().
> 
> The commit solved one problem but created another:  It causes a
> use-after-free in USB Ethernet drivers aqc111.c, asix_devices.c,
> ax88179_178a.c, ch9200.c and smsc75xx.c:
> 
> * If the drivers receive a link change interrupt immediately before
>   disconnect, they raise EVENT_LINK_RESET in their (non-sleepable)
>   ->status() callback and schedule usbnet_deferred_kevent().
> * usbnet_deferred_kevent() invokes the driver's ->link_reset() callback,
>   which calls netif_carrier_{on,off}().
> * That in turn schedules the work linkwatch_event().
> 
> Because usbnet_deferred_kevent() is awaited after unregister_netdev(),
> netif_carrier_{on,off}() may operate on an unregistered netdev and
> linkwatch_event() may run after free_netdev(), causing a use-after-free.
> 
> In 2010, usbnet was changed to only wait for a single instance of
> usbnet_deferred_kevent() instead of *all* work by commit 23f333a2bfaf
> ("drivers/net: don't use flush_scheduled_work()").
> 
> Unfortunately the commit neglected to move the wait back to
> ->ndo_stop().  Rectify that omission at long last.
> 
> Reported-by: Jann Horn <jannh@google.com>
> Link: https://lore.kernel.org/netdev/CAG48ez0MHBbENX5gCdHAUXZ7h7s20LnepBF-pa5M=7Bi-jZrEA@mail.gmail.com/
> Reported-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Link: https://lore.kernel.org/netdev/20220315113841.GA22337@pengutronix.de/
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Cc: stable@vger.kernel.org
> Cc: Oliver Neukum <oneukum@suse.com>
Acked-by: Oliver Neukum <oneukum@suse.com>