On Wed, Jul 19, 2023 at 05:13:58PM +0800, David Gow wrote:
On Wed, 28 Jun 2023 at 17:50, Maxime Ripard mripard@kernel.org wrote:
From: David Gow davidgow@google.com
In the current code, devres_release_all() only gets called if the device has a bus and has been probed.
This leads to issues when using bus-less or driver-less devices where the device might never get freed if a managed resource holds a reference to the device. This is happening in the DRM framework for example.
We should thus call devres_release_all() in the device_del() function to make sure that the device-managed actions are properly executed when the device is unregistered, even if it has neither a bus nor a driver.
This is effectively the same change than commit 2f8d16a996da ("devres: release resources on device_del()") that got reverted by commit a525a3ddeaca ("driver core: free devres in device_release") over use-after-free concerns.
It's not clear whether those concerns are legitimate though, but I would expect drivers not to register new resources in their device-managed actions.
It might be clearer to notice that this patch effectively combines the two patches above, freeing _both_ on device_del() and device_release(). This should give us the best of both worlds.
You're right I'll add that part to the commit log.
I'm not aware of a use-after-free issue that could result here, though it's possible there's a double free I'm missing now that we are freeing things twice. My understanding is that commit a525a3ddeaca ("driver core: free devres in device_release") was more to avoid a leak than a use-after-free, but I could be wrong.
Yeah, I'm not sure where I got the UAF from. I probably misread/misremembered.
Maxime