[PATCH 02/12] phy: phy-mtk-dp: Rename regs to regmap in struct mtk_dp_phy

[AngeloGioacchino Del Regno]

On 7/1/26 14:26, sashiko-bot at kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Missing NULL check for `dev->platform_data` before dereferencing.
> - [High] Missing `remove` callback to clean up PHY lookup leads to a memory leak and use-after-free.
> --
> 
> commit a1a96e429cf3345472aae1b06f6ffe5b31edee81
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno at collabora.com>
> 
> phy: phy-mtk-dp: Rename regs to regmap in struct mtk_dp_phy
> 
> In preparation to perform further cleanups and to extend the driver to
> support more SoCs, rename the regs member to regmap to improve readability,
> as this is a common name across many kernel drivers for a struct regmap.
> 
>> diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
>> index d7024a1443358..bf7b3a95e72d2 100644
>> --- a/drivers/phy/mediatek/phy-mtk-dp.c
>> +++ b/drivers/phy/mediatek/phy-mtk-dp.c
> [ ... ]
>> @@ -176,7 +176,7 @@ static int mtk_dp_phy_probe(struct platform_device *pdev)
> 
> [Severity: High]
> This is a pre-existing issue, but is it possible for dev->platform_data to be
> NULL here?
> 
> Earlier in mtk_dp_phy_probe(), dev->platform_data is dereferenced directly
> without a check:
> 
> drivers/phy/mediatek/phy-mtk-dp.c:mtk_dp_phy_probe() {
> 	...
> 	struct regmap *regs;
> 
> 	regs = *(struct regmap **)dev->platform_data;
> 	if (!regs)
> 	...
> }
> 
> If the driver is manually bound via sysfs (or otherwise instantiated without
> platform data), dev->platform_data will be NULL.
> 
> Should the code validate that dev->platform_data is not NULL before
> dereferencing it to avoid a kernel panic?
> 

No, that's not crashing it, because there's a NULL check anyway just after that.

> [Severity: High]
> This isn't a bug introduced by this patch, but does this driver leak the
> phy_lookup struct and leave a dangling pointer if it gets unbound?
> 
> In mtk_dp_phy_probe(), when !dev->of_node is true, a lookup entry is
> registered:
> 
> drivers/phy/mediatek/phy-mtk-dp.c:mtk_dp_phy_probe() {
> 	...
> 	if (!dev->of_node)
> 		phy_create_lookup(phy, "dp", dev_name(dev));
> 
> 	return 0;
> }
> 
> However, mtk_dp_phy_driver does not have a .remove callback to call
> phy_remove_lookup():
> 
> drivers/phy/mediatek/phy-mtk-dp.c:
> static struct platform_driver mtk_dp_phy_driver = {
> 	.probe = mtk_dp_phy_probe,
> 	.driver = {
> 		.name = "mediatek-dp-phy",
> 	},
> };
> 
> If the driver is unbound, the phy_lookup struct is leaked and remains linked
> in the global phys list. Since the phy struct gets freed by the
> device-managed subsystem, subsequent device probes iterating the PHY lookup
> list would trigger a use-after-free.
> 
> Could a .remove callback be added to unregister the lookup and prevent this?
> 

That's right, but I didn't create this issue. I can fix that in a later commit
just for that, even though this code path is dead when using new style devicetree.

>>   	if (!dp_phy)
>>   		return -ENOMEM;
>>   
>> -	dp_phy->regs = regs;
>> +	dp_phy->regmap = regs;
>>   	phy = devm_phy_create(dev, NULL, &mtk_dp_phy_dev_ops);
>>   	if (IS_ERR(phy))
>>   		return dev_err_probe(dev, PTR_ERR(phy),
>