Re: [PATCH v3 04/10] leds: pca995x: Fix fwnode handle leaks in error paths
From: Nora Schiffer
Date: Tue Sep 29 2026 - 05:34:02 EST
On Tue, 2026-09-29 at 10:20 +0100, Lee Jones wrote:
> --- checkpatch.pl: clean (0 issues) ---
>
> On Tue, 22 Sep 2026, Nora Schiffer wrote:
>
> > Each entry in led_fwnodes needs to be put as long as no LED device has
> > been created for it yet - not just in the creation loop, but also in the
> > first loop that iterates over the child nodes.
> >
> > By clearing entries in led_fwnodes once they have been used, the same
> > cleanup loop can be used to handle errors in both loops.
> >
> > Fixes: 82c5ada1f9d0 ("leds: pca995x: Fix device child node usage in pca995x_probe()")
> > Signed-off-by: Nora Schiffer <nora.schiffer@xxxxxxxxxxxxxxx>
> > ---
> > drivers/leds/leds-pca995x.c | 26 +++++++++++++++++---------
> > 1 file changed, 17 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/leds/leds-pca995x.c b/drivers/leds/leds-pca995x.c
> > index 8b05b3f12cf38..079ce3d8f1e49 100644
> > --- a/drivers/leds/leds-pca995x.c
> > +++ b/drivers/leds/leds-pca995x.c
> > @@ -123,7 +123,7 @@ static int pca995x_probe(struct i2c_client *client)
> > const struct pca995x_chipdef *chipdef;
> > struct pca995x_chip *chip;
> > struct pca995x_led *led;
> > - int i, j, reg, ret;
> > + int i, reg, ret;
> >
> > chipdef = device_get_match_data(&client->dev);
> >
> > @@ -144,10 +144,12 @@ static int pca995x_probe(struct i2c_client *client)
> > device_for_each_child_node_scoped(dev, child) {
> > ret = fwnode_property_read_u32(child, "reg", ®);
> > if (ret)
> > - return ret;
> > + goto err_put_nodes;
> >
> > - if (reg < 0 || reg >= chipdef->num_leds || led_fwnodes[reg])
> > - return -EINVAL;
> > + if (reg < 0 || reg >= chipdef->num_leds || led_fwnodes[reg]) {
> > + ret = -EINVAL;
> > + goto err_put_nodes;
> > + }
> >
> > led = &chip->leds[reg];
> > led_fwnodes[reg] = fwnode_handle_get(child);
> > @@ -169,12 +171,12 @@ static int pca995x_probe(struct i2c_client *client)
> > &chip->leds[i].ldev,
> > &init_data);
> > if (ret < 0) {
> > - for (j = i; j < chipdef->num_leds; j++)
> > - fwnode_handle_put(led_fwnodes[j]);
> > - return dev_err_probe(dev, ret,
> > - "Could not register LED %s\n",
> > - chip->leds[i].ldev.name);
> > + dev_err_probe(dev, ret, "Could not register LED %s\n",
> > + chip->leds[i].ldev.name);
> > + goto err_put_nodes;
> > }
> > +
> > + led_fwnodes[i] = NULL;
>
> Shouldn't we put this before removing it?
The handle is only consumed when the LED has been registered successfully. On
error (ret < 0), led_fwnodes[i] must not have been unset yet to avoid the leak.
>
> > }
> >
> > /* Disable LED all-call address and set normal mode */
> > @@ -184,6 +186,12 @@ static int pca995x_probe(struct i2c_client *client)
> >
> > /* IREF Output current value for all LEDn outputs */
> > return regmap_write(chip->regmap, chipdef->irefall, PCA995X_IREFALL_HALF_CFG);
> > +
> > +err_put_nodes:
> > + for (i = 0; i < chipdef->num_leds; i++)
> > + fwnode_handle_put(led_fwnodes[i]);
> > +
> > + return ret;
> > }
> >
> > static const struct i2c_device_id pca995x_id[] = {
> > --
> > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> > Amtsgericht München, HRB 105018
> > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> > https://www.tq-group.com/
> >
> >
>
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/