[PATCH 0/2] leds-st1202 patches for -rc release of 6.14

[PATCH 0/2] leds-st1202 patches for -rc release of 6.14

[Manuel Fombuena]

Following the feedback received on the set of patches for leds-st1202 that
I sent previously, I am sending separately for your consideration those
that I would like to propose for an upcoming -rc release of 6.14.

Since leds-st1202 was added on 6.14-rc1, this would avoid having to send
[PATCH 1/2] to -stable after 6.14 is released.

[PATCH 1/2] leds: leds-st1202: fix NULL pointer access on race condition

leds-st1202 calls devm_led_classdev_register_ext early in its 
initialization, even before it has filled the led_classdev struct.
Afterwards, it initializes the hardware that, additional to its normal
operations, requires a delay for it to initialize itself. Finally,
the probe function activates the channels on the devicetree, and clears
any existing pattern. 

The issue is that, with such delay between the LEDs being available on
user space after calling devm_led_classdev_register_ext and the internal
data structures being set up, there is a time in which accessing them
causes a NULL pointer access.

This predominantly manifests on systems in which leds-st1202 is loaded as
a module during the boot up process, while other processes are also
starting and interact with the LEDs while doing so. For example, routers.

While the window in which the NULL pointer access can occur is small and
some other circumstances have to concur, it is preventable with not
compromises observed.

Looking at a number of other LED drivers in the linux kernel, it is
seemingly commonplace to call devm_led_classdev_register_ext at the end of
probing which makes sense as all drivers should try to avoid a situation
similar to the one described.

If the patch is not applied, downstream projects using leds-st1202 will
need to take this issue into account if they want to avoid the possibility
of it happening, or apply the patch themselves. However, the drive will
work just the same if left alone just enough time to complete its
initialization.

[PATCH 2/2] Documentation: leds: remove .rst extension for leds-st1202 on
index

The only driver listed with the .rst extension on 
Documentation/leds/index.rst is leds-st1202. Moreover, I haven't been able
to find other entries with the extension on other index.rst files.

While this is more of a cosmetic issue, once it has been spotted I believe
it would be better to fix it before the driver is included on a final
release for the first time.

In practical terms, not applying the patch now won't have any impact so it
could be deferred as well.

Manuel Fombuena (2):
  leds: leds-st1202: fix NULL pointer access on race condition
  Documentation: leds: remove .rst extension for leds-st1202 on index

 Documentation/leds/index.rst |  2 +-
 drivers/leds/leds-st1202.c   | 21 ++++++++++-----------
 2 files changed, 11 insertions(+), 12 deletions(-)

-- 
2.48.1

[PATCH 1/2] leds: leds-st1202: fix NULL pointer access on race condition

[Manuel Fombuena]

st1202_dt_init() calls devm_led_classdev_register_ext() before the
internal data structures are properly set up, so the LEDs become visible
to user space while being partially initialized, leading to a window
where trying to access them causes a NULL pointer access.

Move devm_led_classdev_register_ext() from DT initialization
to the end of the probe function when DT and hardware are fully
initialized and ready to interact with user space.

Fixes: 259230378c65 ("leds: Add LED1202 I2C driver")
Signed-off-by: Manuel Fombuena <fombuena@outlook.com>
---
 drivers/leds/leds-st1202.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c
index 360e9db78dc1..9f275f7fb159 100644
--- a/drivers/leds/leds-st1202.c
+++ b/drivers/leds/leds-st1202.c
@@ -260,8 +260,6 @@ static int st1202_dt_init(struct st1202_chip *chip)
 	int err, reg;
 
 	for_each_available_child_of_node_scoped(dev_of_node(dev), child) {
-		struct led_init_data init_data = {};
-
 		err = of_property_read_u32(child, "reg", &reg);
 		if (err)
 			return dev_err_probe(dev, err, "Invalid register\n");
@@ -275,15 +273,6 @@ static int st1202_dt_init(struct st1202_chip *chip)
 		led->led_cdev.pattern_set = st1202_led_pattern_set;
 		led->led_cdev.pattern_clear = st1202_led_pattern_clear;
 		led->led_cdev.default_trigger = "pattern";
-
-		init_data.fwnode = led->fwnode;
-		init_data.devicename = "st1202";
-		init_data.default_label = ":";
-
-		err = devm_led_classdev_register_ext(dev, &led->led_cdev, &init_data);
-		if (err < 0)
-			return dev_err_probe(dev, err, "Failed to register LED class device\n");
-
 		led->led_cdev.brightness_set = st1202_brightness_set;
 		led->led_cdev.brightness_get = st1202_brightness_get;
 	}
@@ -369,6 +358,7 @@ static int st1202_probe(struct i2c_client *client)
 		return ret;
 
 	for (int i = 0; i < ST1202_MAX_LEDS; i++) {
+		struct led_init_data init_data = {};
 		led = &chip->leds[i];
 		led->chip = chip;
 		led->led_num = i;
@@ -385,6 +375,15 @@ static int st1202_probe(struct i2c_client *client)
 		if (ret < 0)
 			return dev_err_probe(&client->dev, ret,
 					"Failed to clear LED pattern\n");
+
+		init_data.fwnode = led->fwnode;
+		init_data.devicename = "st1202";
+		init_data.default_label = ":";
+
+		ret = devm_led_classdev_register_ext(&client->dev, &led->led_cdev, &init_data);
+		if (ret < 0)
+			return dev_err_probe(&client->dev, ret,
+					"Failed to register LED class device\n");
 	}
 
 	return 0;
-- 
2.48.1

[PATCH 2/2] Documentation: leds: remove .rst extension for leds-st1202 on index

[Manuel Fombuena]

No other LED driver is listed on index.rst with the extension .rst.
Remove it.

Fixes: b1816b22381b ("Documentation:leds: Add leds-st1202.rst")
Signed-off-by: Manuel Fombuena <fombuena@outlook.com>
---
 Documentation/leds/index.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/leds/index.rst b/Documentation/leds/index.rst
index 0ab0a2128a11..76fae171039c 100644
--- a/Documentation/leds/index.rst
+++ b/Documentation/leds/index.rst
@@ -28,5 +28,5 @@ LEDs
    leds-mlxcpld
    leds-mt6370-rgb
    leds-sc27xx
-   leds-st1202.rst
+   leds-st1202
    leds-qcom-lpg
-- 
2.48.1

Re: (subset) [PATCH 0/2] leds-st1202 patches for -rc release of 6.14

[Lee Jones]

On Tue, 25 Feb 2025 21:58:09 +0000, Manuel Fombuena wrote:
> Following the feedback received on the set of patches for leds-st1202 that
> I sent previously, I am sending separately for your consideration those
> that I would like to propose for an upcoming -rc release of 6.14.
> 
> Since leds-st1202 was added on 6.14-rc1, this would avoid having to send
> [PATCH 1/2] to -stable after 6.14 is released.
> 
> [...]

Applied, thanks!

[1/2] leds: leds-st1202: fix NULL pointer access on race condition
      commit: c72e455b89f216b43cd0dbb518036ec4c98f5c46

--
Lee Jones [李琼斯]

Re: [PATCH 2/2] Documentation: leds: remove .rst extension for leds-st1202 on index

[Manuel Fombuena]

On Tue, 25 Feb 2025, Manuel Fombuena wrote:

> No other LED driver is listed on index.rst with the extension .rst.
> Remove it.
> 
> Fixes: b1816b22381b ("Documentation:leds: Add leds-st1202.rst")
> Signed-off-by: Manuel Fombuena <fombuena@outlook.com>
> ---
>  Documentation/leds/index.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/leds/index.rst b/Documentation/leds/index.rst
> index 0ab0a2128a11..76fae171039c 100644
> --- a/Documentation/leds/index.rst
> +++ b/Documentation/leds/index.rst
> @@ -28,5 +28,5 @@ LEDs
>     leds-mlxcpld
>     leds-mt6370-rgb
>     leds-sc27xx
> -   leds-st1202.rst
> +   leds-st1202
>     leds-qcom-lpg
> -- 
> 2.48.1
> 
> 

Now that 6.14 is out, should I re-send this for 'for-leds-next' or there 
is no need?

--
Manuel Fombuena

Re: [PATCH 2/2] Documentation: leds: remove .rst extension for leds-st1202 on index

[Lee Jones]

On Wed, 26 Mar 2025, Manuel Fombuena wrote:

> On Tue, 25 Feb 2025, Manuel Fombuena wrote:
> 
> > No other LED driver is listed on index.rst with the extension .rst.
> > Remove it.
> > 
> > Fixes: b1816b22381b ("Documentation:leds: Add leds-st1202.rst")
> > Signed-off-by: Manuel Fombuena <fombuena@outlook.com>
> > ---
> >  Documentation/leds/index.rst | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/leds/index.rst b/Documentation/leds/index.rst
> > index 0ab0a2128a11..76fae171039c 100644
> > --- a/Documentation/leds/index.rst
> > +++ b/Documentation/leds/index.rst
> > @@ -28,5 +28,5 @@ LEDs
> >     leds-mlxcpld
> >     leds-mt6370-rgb
> >     leds-sc27xx
> > -   leds-st1202.rst
> > +   leds-st1202
> >     leds-qcom-lpg
> > -- 
> > 2.48.1
> > 
> > 
> 
> Now that 6.14 is out, should I re-send this for 'for-leds-next' or there 
> is no need?

If this is still relevant and still applies cleanly, I'll take it from here.

-- 
Lee Jones [李琼斯]

Re: [PATCH 2/2] Documentation: leds: remove .rst extension for leds-st1202 on index

[Manuel Fombuena]

On Fri, 28 Mar 2025, Lee Jones wrote:
> 
> If this is still relevant and still applies cleanly, I'll take it from here.
> 

Thanks. As of now, it still applies cleanly and it is still the only 
driver in the list that includes .rst.

--
Manuel Fombuena

Re: (subset) [PATCH 2/2] Documentation: leds: remove .rst extension for leds-st1202 on index

[Lee Jones]

On Tue, 25 Feb 2025 22:02:28 +0000, Manuel Fombuena wrote:
> No other LED driver is listed on index.rst with the extension .rst.
> Remove it.
> 
> 

Applied, thanks!

[2/2] Documentation: leds: remove .rst extension for leds-st1202 on index
      commit: e9902304002133708fbbfa940c1cc200d249b143

--
Lee Jones [李琼斯]