Linux LED subsystem development
 help / color / mirror / Atom feed
* [PATCH v2 0/2] leds: s2m: Wire up of_match_table in platform drivers
@ 2026-07-22 16:15 kr494167
  2026-07-22 16:15 ` [PATCH v2 1/2] leds: rgb: s2m: Wire up of_match_table in platform driver kr494167
  2026-07-22 16:15 ` [PATCH v2 2/2] leds: flash: " kr494167
  0 siblings, 2 replies; 6+ messages in thread
From: kr494167 @ 2026-07-22 16:15 UTC (permalink / raw)
  To: lee, pavel; +Cc: kauschluss, linux-leds, linux-kernel, Surendra

From: Surendra <kr494167@gmail.com>

This patch series fixes missing .of_match_table assignments in the
Samsung S2M PMIC RGB and Flash LED platform drivers.

Both drivers define and export of_match_table via MODULE_DEVICE_TABLE(of, ...)
but never assign it to .of_match_table in the platform_driver struct.

Changes in v2:
- Capitalize subject line descriptions after subsystem prefix per LED guidelines.
- Validate dev->parent in probe() to prevent NULL pointer dereferences if probed
  without parent MFD driver data.
- Update author name to full name.

Surendra (2):
  leds: rgb: s2m: Wire up of_match_table in platform driver
  leds: flash: s2m: Wire up of_match_table in platform driver

 drivers/leds/flash/leds-s2m-flash.c | 10 +++++++++-
 drivers/leds/rgb/leds-s2m-rgb.c     | 10 +++++++++-
 2 files changed, 18 insertions(+), 2 deletions(-)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/2] leds: rgb: s2m: Wire up of_match_table in platform driver
  2026-07-22 16:15 [PATCH v2 0/2] leds: s2m: Wire up of_match_table in platform drivers kr494167
@ 2026-07-22 16:15 ` kr494167
  2026-07-22 16:29   ` sashiko-bot
  2026-07-22 18:07   ` Kaustabh Chakraborty
  2026-07-22 16:15 ` [PATCH v2 2/2] leds: flash: " kr494167
  1 sibling, 2 replies; 6+ messages in thread
From: kr494167 @ 2026-07-22 16:15 UTC (permalink / raw)
  To: lee, pavel; +Cc: kauschluss, linux-leds, linux-kernel, Surendra

From: Surendra <kr494167@gmail.com>

The driver defines and registers s2m_rgb_of_match_table via
MODULE_DEVICE_TABLE(of, ...) but never assigns it to the platform
driver's .of_match_table field. As a result, the kernel never matches
this driver against a DT node with compatible "samsung,s2mu005-rgb",
making the MODULE_DEVICE_TABLE entry dead code.

Wire the table up so that DT-based probing and module auto-loading
work as intended. In addition, validate dev->parent in s2m_rgb_probe()
to prevent a NULL pointer dereference if probed without parent driver data.

Fixes: 63ccd117f425 ("leds: rgb: Add support for Samsung S2M series PMIC RGB LED device")
Signed-off-by: Surendra <kr494167@gmail.com>
---
 drivers/leds/rgb/leds-s2m-rgb.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/rgb/leds-s2m-rgb.c b/drivers/leds/rgb/leds-s2m-rgb.c
index d239f54eee90..7ac2b1160c1e 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,
 	},
 	.probe = s2m_rgb_probe,
 	.id_table = s2m_rgb_id_table,
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] leds: flash: s2m: Wire up of_match_table in platform driver
  2026-07-22 16:15 [PATCH v2 0/2] leds: s2m: Wire up of_match_table in platform drivers kr494167
  2026-07-22 16:15 ` [PATCH v2 1/2] leds: rgb: s2m: Wire up of_match_table in platform driver kr494167
@ 2026-07-22 16:15 ` kr494167
  2026-07-22 16:29   ` sashiko-bot
  1 sibling, 1 reply; 6+ messages in thread
From: kr494167 @ 2026-07-22 16:15 UTC (permalink / raw)
  To: lee, pavel; +Cc: kauschluss, linux-leds, linux-kernel, Surendra

From: Surendra <kr494167@gmail.com>

The driver defines and registers s2m_fled_of_match_table via
MODULE_DEVICE_TABLE(of, ...) but never assigns it to the platform
driver's .of_match_table field. As a result, the kernel never matches
this driver against a DT node with compatible "samsung,s2mu005-flash",
making the MODULE_DEVICE_TABLE entry dead code.

Wire the table up so that DT-based probing and module auto-loading
work as intended. In addition, validate dev->parent in s2m_fled_probe()
to prevent a NULL pointer dereference if probed without parent driver data.

Fixes: 02149db273a9 ("leds: flash: Add support for Samsung S2M series PMIC flash LED device")
Signed-off-by: Surendra <kr494167@gmail.com>
---
 drivers/leds/flash/leds-s2m-flash.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/flash/leds-s2m-flash.c b/drivers/leds/flash/leds-s2m-flash.c
index 6ee8db094611..6097c75b681b 100644
--- a/drivers/leds/flash/leds-s2m-flash.c
+++ b/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);
+	if (!ddata)
+		return -ENODEV;
+
 	led = devm_kzalloc(dev, sizeof(*led) * MAX_CHANNELS, GFP_KERNEL);
 	if (!led)
 		return -ENOMEM;
@@ -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,
 	},
 	.probe = s2m_fled_probe,
 	.id_table = s2m_fled_id_table,
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] leds: flash: s2m: Wire up of_match_table in platform driver
  2026-07-22 16:15 ` [PATCH v2 2/2] leds: flash: " kr494167
@ 2026-07-22 16:29   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-07-22 16:29 UTC (permalink / raw)
  To: kr494167; +Cc: linux-leds, lee

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 <kr494167@gmail.com>

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/20260722161519.9610-1-kr494167@gmail.com?part=2

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] leds: rgb: s2m: Wire up of_match_table in platform driver
  2026-07-22 16:15 ` [PATCH v2 1/2] leds: rgb: s2m: Wire up of_match_table in platform driver kr494167
@ 2026-07-22 16:29   ` sashiko-bot
  2026-07-22 18:07   ` Kaustabh Chakraborty
  1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-07-22 16:29 UTC (permalink / raw)
  To: kr494167; +Cc: lee, linux-leds

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 <kr494167@gmail.com>

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/20260722161519.9610-1-kr494167@gmail.com?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] leds: rgb: s2m: Wire up of_match_table in platform driver
  2026-07-22 16:15 ` [PATCH v2 1/2] leds: rgb: s2m: Wire up of_match_table in platform driver kr494167
  2026-07-22 16:29   ` sashiko-bot
@ 2026-07-22 18:07   ` Kaustabh Chakraborty
  1 sibling, 0 replies; 6+ messages in thread
From: Kaustabh Chakraborty @ 2026-07-22 18:07 UTC (permalink / raw)
  To: kr494167, lee, pavel; +Cc: kauschluss, linux-leds, linux-kernel

On 2026-07-22 21:45 +05:30, kr494167 wrote:
> From: Surendra <kr494167@gmail.com>
>
> The driver defines and registers s2m_rgb_of_match_table via
> MODULE_DEVICE_TABLE(of, ...) but never assigns it to the platform
> driver's .of_match_table field. As a result, the kernel never matches
> this driver against a DT node with compatible "samsung,s2mu005-rgb",
> making the MODULE_DEVICE_TABLE entry dead code.
>
> Wire the table up so that DT-based probing and module auto-loading
> work as intended. In addition, validate dev->parent in s2m_rgb_probe()
> to prevent a NULL pointer dereference if probed without parent driver data.
>
> Fixes: 63ccd117f425 ("leds: rgb: Add support for Samsung S2M series PMIC RGB LED device")
> Signed-off-by: Surendra <kr494167@gmail.com>
> ---
>  drivers/leds/rgb/leds-s2m-rgb.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/leds/rgb/leds-s2m-rgb.c b/drivers/leds/rgb/leds-s2m-rgb.c
> index d239f54eee90..7ac2b1160c1e 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;

This seems to be a separate change. I guess it's preferable having it
as a separate patch.

>  	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;
> +

1. This is a MFD sub-device, dev->parent shall always be present. But
there's no harm in being extra careful here.

2. For error codes other than -ENOMEM, we do not return from probe this
way, instead:

	return dev_err_probe(dev, -ENODEV, "error message\n");

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

By the way, it works just fine without it, when built as a module.

The reason it wasn't added was in a comment in a previous revision [1],
but it was removed after reviews.

[1] https://lore.kernel.org/all/20260225-s2mu005-pmic-v3-9-b4afee947603@disroot.org/

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-07-22 18:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-22 16:15 [PATCH v2 0/2] leds: s2m: Wire up of_match_table in platform drivers kr494167
2026-07-22 16:15 ` [PATCH v2 1/2] leds: rgb: s2m: Wire up of_match_table in platform driver kr494167
2026-07-22 16:29   ` sashiko-bot
2026-07-22 18:07   ` Kaustabh Chakraborty
2026-07-22 16:15 ` [PATCH v2 2/2] leds: flash: " kr494167
2026-07-22 16:29   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox