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 <[email protected]>
---
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);
pci_unregister_driver(&hfc_driver);
}
--
2.17.1
Hello:
This patch was applied to netdev/net.git (master)
by David S. Miller <[email protected]>:
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!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
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 <[email protected]>
> ---
> 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