linux-kernel - Re: [PATCH] mISDN: hfcpci: Fix use-after-free bug in hfcpci

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] [day] [month] [year] [list]

Message-ID: <24615c1e.123e96.183d5495a77.Coremail.duoming@zju.edu.cn>
Date:   Fri, 14 Oct 2022 14:58:17 +0800 (GMT+08:00)
From:   duoming@....edu.cn
To:     "Leon Romanovsky" <leon@...nel.org>
Cc:     "Greg KH" <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        isdn@...ux-pingi.de, kuba@...nel.org, andrii@...nel.org,
        davem@...emloft.net, axboe@...nel.dk
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@....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