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 PHC | |
Open Source and information security mailing list archives
| ||
|
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>
Powered by blists - more mailing lists