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

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Wiring up `.of_match_table` without validating `dev->parent` allows arbitrary device tree instantiation, leading to a NULL pointer dereference or type confusion.
- [Low] The commit message description after the subsystem prefix is not capitalized, violating the LED subsystem guidelines.
--

--- Patch [1]: [PATCH 1/2] leds: rgb: s2m: wire up of_match_table in platform driver ---
commit b11896849c3c9816b4973c7b324b0ca877cb4766
Author: surendra <[email protected]>

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

This commit wires up the of_match_table in the s2m-rgb platform driver.
This enables device tree based probing and module auto-loading, fixing an
issue where the MODULE_DEVICE_TABLE entry was dead code.

> leds: rgb: s2m: wire up of_match_table in platform driver

[Severity: Low]
This isn't a bug, but the commit description after the subsystem prefix
should be capitalized according to LED subsystem guidelines.

> diff --git a/drivers/leds/rgb/leds-s2m-rgb.c b/drivers/leds/rgb/leds-s2m-rgb.c
> index d239f54eee901..28964de6920ca 100644
> --- a/drivers/leds/rgb/leds-s2m-rgb.c
> +++ b/drivers/leds/rgb/leds-s2m-rgb.c
> @@ -415,6 +415,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: High]
Does wiring up the of_match_table here allow the driver to be probed via
an arbitrary device tree node?

If the driver is instantiated via a standalone DT node instead of through
the expected MFD parent, dev->parent might not contain the expected driver
data.

In s2m_rgb_probe(), the driver relies on the parent's drvdata:

drivers/leds/rgb/leds-s2m-rgb.c:s2m_rgb_probe() {
	...
	struct sec_pmic_dev *pmic_drvdata = dev_get_drvdata(dev->parent);
	...
	rgb->regmap = pmic_drvdata->regmap_pmic;
	...
}

Could this result in a NULL pointer dereference or type confusion if
pmic_drvdata is NULL or points to an unrelated structure?

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

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