netdev - RE: [PATCH v4] net: ravb: Fix possible UAF bug in ravb

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

Date: Thu, 12 Oct 2023 08:39:41 +0000
From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
To: "REE dirk.behme@...bosch.com" <dirk.behme@...bosch.com>
CC: "s.shtylyov@....ru" <s.shtylyov@....ru>, "lee@...nel.org"
	<lee@...nel.org>, "linyunsheng@...wei.com" <linyunsheng@...wei.com>,
	"davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com"
	<edumazet@...gle.com>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"richardcochran@...il.com" <richardcochran@...il.com>,
	"p.zabel@...gutronix.de" <p.zabel@...gutronix.de>, "geert+renesas@...der.be"
	<geert+renesas@...der.be>, "magnus.damm@...il.com" <magnus.damm@...il.com>,
	Biju Das <biju.das.jz@...renesas.com>, "wsa+renesas@...g-engineering.com"
	<wsa+renesas@...g-engineering.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "linux-renesas-soc@...r.kernel.org"
	<linux-renesas-soc@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "hackerzheng666@...il.com"
	<hackerzheng666@...il.com>, "1395428693sheep@...il.com"
	<1395428693sheep@...il.com>, "alex000young@...il.com"
	<alex000young@...il.com>, Zheng Wang <zyytlz.wz@....com>, Jakub Kicinski
	<kuba@...nel.org>, "REE dirk.behme@...bosch.com" <dirk.behme@...bosch.com>
Subject: RE: [PATCH v4] net: ravb: Fix possible UAF bug in ravb_remove

Hello Behme,

> From: Behme Dirk (CM/ESO2), Sent: Tuesday, October 10, 2023 9:59 PM
> 
> On 26.07.2023 05:19, Jakub Kicinski wrote:
> ...
> > The fact that ravb_tx_timeout_work doesn't take any locks seems much
> > more suspicious.
> Does anybody plan to look into this, too?

I believe my fixed patch [1] resolved this issue too. Let me explain it in detail below.

In the thread, Jakub also mentioned [2] like below:
---
Simplest fix I can think of is to take a reference on the netdev before
scheduling the work, and then check if it's still registered in the work
itself. Wrap the timeout work in rtnl_lock() to avoid any races there.
---

Sergey suggested to add cancel_work_sync() into the ravb_close () [3].
And I investigated calltrace, and then the ravb_close() is under rtnl_lock() [4]
like below:
-----------------------------------------------------------------------
ravb_remove() calls unregister_netdev().
 -> unregister_netdev() calls rtnl_lock() and unregister_netdevice().
 --> unregiter_netdevice_queue()
 ---> unregiter_netdevice_many()
 ----> unregiter_netdevice_many_notify().
 -----> dev_close_many()
 ------> __dev_close_many()
 -------> ops->ndo_stop()

ravb_close() calls phy_stop()
 -> phy_state_machine() with PHY_HALTED
 --> phy_link_down()
 ---> phy_link_change()
 ----> netif_carrier_off()
-----------------------------------------------------------------------

So, during cancel_work_sync() is waiting for canceling the workqueue in ravb_close(),
it's under rtnl_lock() so that no additional locks are needed in ravb_tx_timeout_work().

[1] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=3971442870713de527684398416970cf025b4f89
[2] https://lore.kernel.org/netdev/20230727164820.48c9e685@kernel.org/
[3] https://lore.kernel.org/netdev/607f4fe4-5a59-39dd-71c2-0cf769b48187@omp.ru/
[4] https://lore.kernel.org/netdev/OSYPR01MB53341CFDBB49A3BA41A6752CD8F9A@OSYPR01MB5334.jpnprd01.prod.outlook.com/

Best regards,
Yoshihiro Shimoda

> Best regards
> 
> Dirk