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

On Tue, 21 Jul 2026, [email protected] wrote:

> 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]>

Please use your full name.

> 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

-- 
Lee Jones