[PATCH v2] leds: is31fl32xx: Fix NULL pointer dereference when binding via I2C ID table

[PATCH v2] leds: is31fl32xx: Fix NULL pointer dereference when binding via I2C ID table

[Rosen Penev]

If the device binds via the I2C ID table (e.g. sysfs new_device, a
software node, or a firmware node without a matching compatible),
device_get_match_data() in is31fl32xx_probe() returns NULL, since only
the OF match table carries the chip definition. is31fl32xx_parse_dt()
then dereferences cdef->output_frequency_setting_reg and
is31fl32xx_parse_child_dt() dereferences cdef->channels, causing a
kernel oops.

Populate .driver_data in is31fl32xx_id with the same chip definitions
as the OF table and use i2c_get_match_data(), which falls back to the
I2C ID table when no firmware match data is available. Every entry
now carries match data, so cdef can no longer be NULL.

Drop the comment above the ID table, as the table is now used for
matching data as well.

Fixes: 2779f4724b2f ("leds: various: use device_get_match_data")
Cc: stable@vger.kernel.org
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 v2: use i2c_get_match_data()
 drivers/leds/leds-is31fl32xx.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/leds/leds-is31fl32xx.c b/drivers/leds/leds-is31fl32xx.c
index 285085d27453..5ef4fd649d60 100644
--- a/drivers/leds/leds-is31fl32xx.c
+++ b/drivers/leds/leds-is31fl32xx.c
@@ -574,7 +574,7 @@ static int is31fl32xx_probe(struct i2c_client *client)
 	int count;
 	int ret = 0;
 
-	cdef = device_get_match_data(dev);
+	cdef = i2c_get_match_data(client);
 
 	count = device_get_child_node_count(dev);
 	if (!count)
@@ -611,19 +611,15 @@ static void is31fl32xx_remove(struct i2c_client *client)
 			ERR_PTR(ret));
 }
 
-/*
- * i2c-core (and modalias) requires that id_table be properly filled,
- * even though it is not used for DeviceTree based instantiation.
- */
 static const struct i2c_device_id is31fl32xx_id[] = {
-	{ .name = "is31fl3293" },
-	{ .name = "is31fl3236" },
-	{ .name = "is31fl3236a" },
-	{ .name = "is31fl3235" },
-	{ .name = "is31fl3218" },
-	{ .name = "sn3218" },
-	{ .name = "is31fl3216" },
-	{ .name = "sn3216" },
+	{ "is31fl3293",  (kernel_ulong_t)&is31fl3293_cdef },
+	{ "is31fl3236",  (kernel_ulong_t)&is31fl3236_cdef },
+	{ "is31fl3236a", (kernel_ulong_t)&is31fl3236a_cdef },
+	{ "is31fl3235",  (kernel_ulong_t)&is31fl3235_cdef },
+	{ "is31fl3218",  (kernel_ulong_t)&is31fl3218_cdef },
+	{ "sn3218",      (kernel_ulong_t)&is31fl3218_cdef },
+	{ "is31fl3216",  (kernel_ulong_t)&is31fl3216_cdef },
+	{ "sn3216",      (kernel_ulong_t)&is31fl3216_cdef },
 	{ }
 };
 
-- 
2.56.0

Re: [PATCH v2] leds: is31fl32xx: Fix NULL pointer dereference when binding via I2C ID table

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] leds: is31fl32xx: positional initializers used in changed i2c_device_id table
--

commit 7fdebced20c3197505f19872c5b5fdcf8d9db517
Author: Rosen Penev <rosenp@gmail.com>

leds: is31fl32xx: Fix NULL pointer dereference when binding via I2C ID table

This commit populates driver_data in is31fl32xx_id and switches
is31fl32xx_probe() to use i2c_get_match_data() so that devices binding via
the I2C ID table receive a valid chip definition instead of NULL.

> diff --git a/drivers/leds/leds-is31fl32xx.c b/drivers/leds/leds-is31fl32xx.c
> index 285085d274538..5ef4fd649d608 100644
> --- a/drivers/leds/leds-is31fl32xx.c
> +++ b/drivers/leds/leds-is31fl32xx.c

[ ... ]

> @@ -611,19 +611,15 @@ static void is31fl32xx_remove(struct i2c_client *client)
>  			ERR_PTR(ret));
>  }
>  
> -/*
> - * i2c-core (and modalias) requires that id_table be properly filled,
> - * even though it is not used for DeviceTree based instantiation.
> - */
>  static const struct i2c_device_id is31fl32xx_id[] = {
> -	{ .name = "is31fl3293" },
> -	{ .name = "is31fl3236" },
> -	{ .name = "is31fl3236a" },
> -	{ .name = "is31fl3235" },
> -	{ .name = "is31fl3218" },
> -	{ .name = "sn3218" },
> -	{ .name = "is31fl3216" },
> -	{ .name = "sn3216" },
> +	{ "is31fl3293",  (kernel_ulong_t)&is31fl3293_cdef },
> +	{ "is31fl3236",  (kernel_ulong_t)&is31fl3236_cdef },
> +	{ "is31fl3236a", (kernel_ulong_t)&is31fl3236a_cdef },
> +	{ "is31fl3235",  (kernel_ulong_t)&is31fl3235_cdef },
> +	{ "is31fl3218",  (kernel_ulong_t)&is31fl3218_cdef },
> +	{ "sn3218",      (kernel_ulong_t)&is31fl3218_cdef },
> +	{ "is31fl3216",  (kernel_ulong_t)&is31fl3216_cdef },
> +	{ "sn3216",      (kernel_ulong_t)&is31fl3216_cdef },
>  	{ }
>  };
>  

[Severity: Low]
This isn't a bug, but commit a031b5fce526 ("leds: Use named initializers for
arrays of i2c_device_data") previously converted is31fl32xx_id[] in
drivers/leds/leds-is31fl32xx.c to use named initializers.

Should the updated entries in is31fl32xx_id[] keep using named initializers
for .name and .driver_data instead of switching back to positional
initialization?

For example:

	{ .name = "is31fl3293", .driver_data = (kernel_ulong_t)&is31fl3293_cdef },

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017004351.1249547-1-rosenp@gmail.com?part=1