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

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: <bdf77222-efca-26da-fa37-d969b2aa6234@suse.com>
Date:   Thu, 23 Jun 2022 14:57:09 +0200
From:   Oliver Neukum <oneukum@...e.com>
To:     Lukas Wunner <lukas@...ner.de>, Oliver Neukum <oneukum@...e.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, Jann Horn <jannh@...gle.com>,
        Oleksij Rempel <o.rempel@...gutronix.de>,
        Oleksij Rempel <linux@...pel-privat.de>,
        Eric Dumazet <edumazet@...gle.com>
Cc:     netdev@...r.kernel.org, linux-usb@...r.kernel.org,
        Andrew Lunn <andrew@...n.ch>,
        Jacky Chou <jackychou@...x.com.tw>, Willy Tarreau <w@....eu>,
        Lino Sanfilippo <LinoSanfilippo@....de>,
        Philipp Rosenberger <p.rosenberger@...bus.com>,
        Heiner Kallweit <hkallweit1@...il.com>
Subject: Re: [PATCH net-next v2] usbnet: Fix linkwatch use-after-free on
 disconnect



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@...gle.com>
> Link: https://lore.kernel.org/netdev/CAG48ez0MHBbENX5gCdHAUXZ7h7s20LnepBF-pa5M=7Bi-jZrEA@mail.gmail.com/
> Reported-by: Oleksij Rempel <o.rempel@...gutronix.de>
> Link: https://lore.kernel.org/netdev/20220315113841.GA22337@pengutronix.de/
> Signed-off-by: Lukas Wunner <lukas@...ner.de>
> Cc: stable@...r.kernel.org
> Cc: Oliver Neukum <oneukum@...e.com>
Acked-by: Oliver Neukum <oneukum@...e.com>