netdev - RE: [PATCH net 2/2] ravb: Fix use-after-free issue in ravb_remove and ravb_tx_timeout

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 5 Oct 2023 00:25:31 +0000
From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
To: Sergey Shtylyov <s.shtylyov@....ru>, "davem@...emloft.net"
	<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>,
	Zheng Wang <zyytlz.wz@....com>
Subject: RE: [PATCH net 2/2] ravb: Fix use-after-free issue in ravb_remove and
 ravb_tx_timeout_work

Hello Sergey,

> From: Sergey Shtylyov, Sent: Thursday, October 5, 2023 3:30 AM
> 
> Hello!
> 
>    Hm, concerning the subject: don't we actually have use-after-free in ravb_tx_timeout()
> only?

IIUC, the issue causes ravb_remove(), and is in ravb_tx_timeout_work().

> Also, you place () after the function names in patch #1 but not in this patch, why?

I thought that the subject was long so that remove the ()...

So, I'll fix the subject as the follow:

ravb: Fix use-after-free issue in ravb_tx_timeout_work()

> On 10/4/23 12:12 PM, Yoshihiro Shimoda wrote:
> 
> > The ravb_stop() should call cancel_work_sync(). Otherwise,
> > ravb_tx_timeout_work() is possible to use the freed priv after
> > ravb_remove() was called like below:
> >
> > CPU0			CPU1
> > 			ravb_tx_timeout()
> > ravb_remove()
> > unregister_netdev()
> > free_netdev(ndev)
> > // free priv
> > 			ravb_tx_timeout_work()
> > 			// use priv
> >
> > unregister_netdev() will call .ndo_stop() so that ravb_stop() is
> > called. And, after phy_stop() was called, netif_carrier_off()
> 
>    s/was/is/?

I'll fix it.

> > is also called. So that .ndo_tx_timeout() will be not called
> 
>    Will not be...

Oops. I'll fix it.

> > after phy_stop().
> >
> > Link:
<snip URL>
> > Fixes: c156633f1353 ("Renesas Ethernet AVB driver proper")
> > Reported-by: Zheng Wang <zyytlz.wz@....com>
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
> 
>    Otherwise:
> 
> Reviewed-by: Sergey Shtylyov <s.shtylyov@....ru>

Thank you for your review!

Best regards,
Yoshihiro Shimoda

> [...]
> 
> MBR, Sergey