| Message ID | 20221009063731.22733-1-duoming@zju.edu.cn (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 175302f6b79ebbb207c2d58d6d3e679465de23b0 |
| Headers | show |
| Series | mISDN: hfcpci: Fix use-after-free bug in hfcpci_softirq | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
Hello: This patch was applied to netdev/net.git (master) by David S. Miller <davem@davemloft.net>: On Sun, 9 Oct 2022 14:37:31 +0800 you wrote: > The function hfcpci_softirq() is a timer handler. If it > is running, the timer_pending() will return 0 and the > del_timer_sync() in HFC_cleanup() will not be executed. > As a result, the use-after-free bug will happen. The > process is shown below: > > (cleanup routine) | (timer handler) > HFC_cleanup() | hfcpci_softirq() > if (timer_pending(&hfc_tl)) | > del_timer_sync() | > ... | ... > pci_unregister_driver(hc) | > driver_unregister | driver_for_each_device > bus_remove_driver | _hfcpci_softirq > driver_detach | ... > put_device(dev) //[1]FREE | > | dev_get_drvdata(dev) //[2]USE > > [...] Here is the summary with links: - mISDN: hfcpci: Fix use-after-free bug in hfcpci_softirq https://git.kernel.org/netdev/net/c/175302f6b79e You are awesome, thank you!
On Sun, Oct 09, 2022 at 02:37:31PM +0800, Duoming Zhou wrote: > The function hfcpci_softirq() is a timer handler. If it > is running, the timer_pending() will return 0 and the > del_timer_sync() in HFC_cleanup() will not be executed. > As a result, the use-after-free bug will happen. The > process is shown below: > > (cleanup routine) | (timer handler) > HFC_cleanup() | hfcpci_softirq() > if (timer_pending(&hfc_tl)) | > del_timer_sync() | > ... | ... > pci_unregister_driver(hc) | > driver_unregister | driver_for_each_device > bus_remove_driver | _hfcpci_softirq > driver_detach | ... > put_device(dev) //[1]FREE | > | dev_get_drvdata(dev) //[2]USE > > The device is deallocated is position [1] and used in > position [2]. > > Fix by removing the "timer_pending" check in HFC_cleanup(), > which makes sure that the hfcpci_softirq() have finished > before the resource is deallocated. > > Fixes: 009fc857c5f6 ("mISDN: fix possible use-after-free in HFC_cleanup()") > Signed-off-by: Duoming Zhou <duoming@zju.edu.cn> > --- > drivers/isdn/hardware/mISDN/hfcpci.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/isdn/hardware/mISDN/hfcpci.c b/drivers/isdn/hardware/mISDN/hfcpci.c > index af17459c1a5..e964a8dd851 100644 > --- a/drivers/isdn/hardware/mISDN/hfcpci.c > +++ b/drivers/isdn/hardware/mISDN/hfcpci.c > @@ -2345,8 +2345,7 @@ HFC_init(void) > static void __exit > HFC_cleanup(void) > { > - if (timer_pending(&hfc_tl)) > - del_timer_sync(&hfc_tl); > + del_timer_sync(&hfc_tl); How was this tested? Do you have this hardware? thanks, greg k-h
diff --git a/drivers/isdn/hardware/mISDN/hfcpci.c b/drivers/isdn/hardware/mISDN/hfcpci.c index af17459c1a5..e964a8dd851 100644 --- a/drivers/isdn/hardware/mISDN/hfcpci.c +++ b/drivers/isdn/hardware/mISDN/hfcpci.c @@ -2345,8 +2345,7 @@ HFC_init(void) static void __exit HFC_cleanup(void) { - if (timer_pending(&hfc_tl)) - del_timer_sync(&hfc_tl); + del_timer_sync(&hfc_tl); pci_unregister_driver(&hfc_driver); }
The function hfcpci_softirq() is a timer handler. If it is running, the timer_pending() will return 0 and the del_timer_sync() in HFC_cleanup() will not be executed. As a result, the use-after-free bug will happen. The process is shown below: (cleanup routine) | (timer handler) HFC_cleanup() | hfcpci_softirq() if (timer_pending(&hfc_tl)) | del_timer_sync() | ... | ... pci_unregister_driver(hc) | driver_unregister | driver_for_each_device bus_remove_driver | _hfcpci_softirq driver_detach | ... put_device(dev) //[1]FREE | | dev_get_drvdata(dev) //[2]USE The device is deallocated is position [1] and used in position [2]. Fix by removing the "timer_pending" check in HFC_cleanup(), which makes sure that the hfcpci_softirq() have finished before the resource is deallocated. Fixes: 009fc857c5f6 ("mISDN: fix possible use-after-free in HFC_cleanup()") Signed-off-by: Duoming Zhou <duoming@zju.edu.cn> --- drivers/isdn/hardware/mISDN/hfcpci.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)