On Thu, Feb 20, 2025 at 02:31:29PM +0100, Théo Lebrun wrote:
Hello Greg,
On Thu Feb 20, 2025 at 1:41 PM CET, Greg Kroah-Hartman wrote:
On Tue, Feb 18, 2025 at 12:00:11PM +0100, Théo Lebrun wrote:
The use-after-free bug appears when:
- A platform device is created from OF, by of_device_add();
- The same device's name is changed afterwards using dev_set_name(), by its probe for example.
Out of the 37 drivers that deal with platform devices and do a dev_set_name() call, only one might be affected. That driver is loongson-i2s-plat [0]. All other dev_set_name() calls are on children devices created on the spot. The issue was found on downstream kernels and we don't have what it takes to test loongson-i2s-plat.
Note: loongson-i2s-plat maintainers are CCed.
⟩ # Finding potential trouble-makers: ⟩ git grep -l 'struct platform_device' | xargs grep -l dev_set_name
The solution proposed is to add a flag to platform_device that tells if it is responsible for freeing its name. We can then duplicate the device name inside of_device_add() instead of copying the pointer.
Ick.
What is done elsewhere?
- Platform bus code does a copy of the argument name that is stored alongside the struct platform_device; see platform_device_alloc()[1].
- Other busses duplicate the device name; either through a dynamic allocation [2] or through an array embedded inside devices [3].
- Some busses don't have a separate name; when they want a name they take it from the device [4].
Really ick.
Let's do the right thing here and just get rid of the name pointer entirely in struct platform_device please. Isn't that the correct thing that way the driver core logic will work properly for all of this.
I would agree, if it wasn't for this consideration that is found in the commit message [0]:
What, that the of code is broken? Then it should be fixed, why does it need a pointer to a name at all anyway? It shouldn't be needed there either.
It is important to duplicate! pdev->name must not change to make sure the platform_match() return value is stable over time. If we updated pdev->name alongside dev->name, once a device probes and changes its name then the platform_match() return value would change.
I'd be fine sending a V2 that removes the field *and the fallback* [1], but I don't have the full scope in mind to know what would become broken.
The fallback will not need to be removed, properly point to the name of the device and it should work correctly.
thanks,
greg k-h