Re: [PATCH v2 1/2] leds: rgb: s2m: Wire up of_match_table in platform driver

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Type confusion and potential crash due to unsafe cast of parent drvdata when probed via Device Tree.
--

--- Patch [1]: [PATCH v2 1/2] leds: rgb: s2m: Wire up of_match_table in platform driver ---
commit fe8555b3e7add30597af4722b7f9538af69ac8af
Author: Surendra <[email protected]>

leds: rgb: s2m: Wire up of_match_table in platform driver

This commit assigns the of_match_table in the platform driver to enable
Device Tree matching. It also adds validation for the parent device and its
driver data to prevent a NULL pointer dereference during probe.

> diff --git a/drivers/leds/rgb/leds-s2m-rgb.c b/drivers/leds/rgb/leds-s2m-rgb.c
> index d239f54eee901..7ac2b1160c1e3 100644
> --- a/drivers/leds/rgb/leds-s2m-rgb.c
> +++ b/drivers/leds/rgb/leds-s2m-rgb.c
> @@ -363,11 +363,18 @@ static const struct mc_subled s2mu005_rgb_subled_info[] = {
>  static int s2m_rgb_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> -	struct sec_pmic_dev *pmic_drvdata = dev_get_drvdata(dev->parent);
> +	struct sec_pmic_dev *pmic_drvdata;
>  	struct s2m_rgb *rgb;
>  	struct led_init_data init_data = {};
>  	int ret;
>  
> +	if (!dev->parent)
> +		return -ENODEV;
> +
> +	pmic_drvdata = dev_get_drvdata(dev->parent);
> +	if (!pmic_drvdata)
> +		return -ENODEV;
> +
>  	rgb = devm_kzalloc(dev, sizeof(*rgb), GFP_KERNEL);
>  	if (!rgb)
>  		return -ENOMEM;
> @@ -415,6 +422,7 @@ MODULE_DEVICE_TABLE(of, s2m_rgb_of_match_table);
>  static struct platform_driver s2m_rgb_driver = {
>  	.driver = {
>  		.name = "s2m-rgb",
> +		.of_match_table = s2m_rgb_of_match_table,

[Severity: Medium]
By enabling Device Tree matching here, the driver can be probed natively 
via DT. This exposes the driver to being probed under arbitrary parents 
rather than strictly the expected MFD parent.

If probed under an unexpected parent (such as an I2C controller or another
platform device), that parent's driver data is blindly cast to 
struct sec_pmic_dev * earlier in s2m_rgb_probe():

    pmic_drvdata = dev_get_drvdata(dev->parent);

Could this lead to memory corruption or a crash when the type-confused 
pointer is later dereferenced in s2m_rgb_probe() to fetch the regmap?

    rgb->regmap = pmic_drvdata->regmap_pmic;

Would it be safer to retrieve the regmap using dev_get_regmap() instead, 
to ensure we safely get a valid regmap pointer regardless of the parent 
device type?

>  	},
>  	.probe = s2m_rgb_probe,
>  	.id_table = s2m_rgb_id_table,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1