Re: [PATCH v2 2/2] leds: flash: 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] Wiring up `.of_match_table` enables direct Device Tree instantiation, exposing a type confusion vulnerability when fetching parent driver data.
--

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

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

This commit assigns s2m_fled_of_match_table to the platform driver's
of_match_table to enable DT-based probing and module auto-loading. It also
adds validation for the parent device and its driver data in the probe
function to prevent null pointer dereferences.

> --- drivers/leds/flash/leds-s2m-flash.c
> +++ drivers/leds/flash/leds-s2m-flash.c
> @@ -279,10 +279,17 @@ static int s2mu005_fled_init_channel(struct s2m_led *led, struct device *dev,
>  static int s2m_fled_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> -	struct sec_pmic_dev *ddata = dev_get_drvdata(dev->parent);
> +	struct sec_pmic_dev *ddata;
>  	struct s2m_led *led;
>  	int ret;
>  
> +	if (!dev->parent)
> +		return -ENODEV;
> +
> +	ddata = dev_get_drvdata(dev->parent);

[Severity: Medium]
Since the driver can now be instantiated directly via the device tree
because of the of_match_table addition below, is it safe to blindly cast
the parent's driver data to struct sec_pmic_dev here?

If the device tree node is placed under a generic bus like simple-pm-bus
instead of the expected sec-core MFD parent, the parent will be valid but
will have unrelated driver data. Accessing ddata->regmap_pmic would then
read from an arbitrary offset, potentially causing memory corruption or a
kernel panic.

Could this be safely mitigated by using dev_get_regmap(dev->parent, NULL)
instead of assuming the exact driver data layout?

> +	if (!ddata)
> +		return -ENODEV;
> +

[ ... ]

> @@ -339,6 +346,7 @@ MODULE_DEVICE_TABLE(of, s2m_fled_of_match_table);
>  static struct platform_driver s2m_fled_driver = {
>  	.driver = {
>  		.name = "s2m-flash",
> +		.of_match_table = s2m_fled_of_match_table,

[Severity: Medium]
Does wiring up the of_match_table here expose the driver to generic OF
matching, allowing direct DT instantiation without enforcing the expected
parent type? 

This appears to bypass the MFD core's controlled instantiation, making the
type confusion issue mentioned above possible.

>  	},
>  	.probe = s2m_fled_probe,
>  	.id_table = s2m_fled_id_table,

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