Messages in this thread | | | Date | Fri, 14 Oct 2022 14:58:17 +0800 (GMT+08:00) | From | duoming@zju ... | Subject | Re: [PATCH] mISDN: hfcpci: Fix use-after-free bug in hfcpci_Timer |
| |
Hello,
On Fri, 14 Oct 2022 09:07:07 +0300 Leon Romanovsky wrote:
> On Thu, Oct 13, 2022 at 03:20:05PM +0200, Greg KH wrote: > > On Thu, Oct 13, 2022 at 08:57:29PM +0800, Duoming Zhou wrote: > > > If the timer handler hfcpci_Timer() is running, the > > > del_timer(&hc->hw.timer) in release_io_hfcpci() could > > > not stop it. As a result, the use-after-free bug will > > > happen. The process is shown below: > > > > > > (cleanup routine) | (timer handler) > > > release_card() | hfcpci_Timer() > > > release_io_hfcpci | > > > del_timer(&hc->hw.timer) | > > > ... | ... > > > kfree(hc) //[1]FREE | > > > | hc->hw.timer.expires //[2]USE > > > > > > The hfc_pci is deallocated in position [1] and used in > > > position [2]. > > > > > > Fix by changing del_timer() in release_io_hfcpci() to > > > del_timer_sync(), which makes sure the hfcpci_Timer() > > > have finished before the hfc_pci is deallocated. > > > > > > Fixes: 1700fe1a10dc ("Add mISDN HFC PCI driver") > > > Signed-off-by: Duoming Zhou <duoming@zju.edu.cn> > > > --- > > > drivers/isdn/hardware/mISDN/hfcpci.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/isdn/hardware/mISDN/hfcpci.c b/drivers/isdn/hardware/mISDN/hfcpci.c > > > index af17459c1a5..5cf37fe7de2 100644 > > > --- a/drivers/isdn/hardware/mISDN/hfcpci.c > > > +++ b/drivers/isdn/hardware/mISDN/hfcpci.c > > > @@ -157,7 +157,7 @@ release_io_hfcpci(struct hfc_pci *hc) > > > { > > > /* disable memory mapped ports + busmaster */ > > > pci_write_config_word(hc->pdev, PCI_COMMAND, 0); > > > - del_timer(&hc->hw.timer); > > > + del_timer_sync(&hc->hw.timer); > > > > Nice, how did you test that this will work properly? Do you have this > > hardware for testing? How was this issue found and verified that this > > is the correct resolution?
I am trying to simulate the hardware to verified that this is the correct resolution. I will give feedback in a few weeks later.
> According to his previous response [1], the answer will be no. I'm not > super-excited that this unmaintained and old driver chosen as playground > for new tool. > > [1] https://lore.kernel.org/all/17ad6913.ff8e0.1838933840d.Coremail.duoming@zju.edu.cn/#t
Best regards, Duoming Zhou | |