Re: [PATCH v3 04/10] leds: pca995x: Fix fwnode handle leaks in error paths

[Nora Schiffer <nora.schiffer@ew.tq-group.com>]

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@ew.tq-group.com>
> > ---
> >  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", &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/