Re: [PATCH 1/2] iio: ltc2983: Fix of_node refcounting

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 25 Sep 2020 11:10:44 +0200
Nuno Sá <nuno.sa@analog.com> wrote:

> When returning or breaking early from a
> `for_each_available_child_of_node()` loop, we need to explicitly call
> `of_node_put()` on the child node to possibly release the node.
> 
> Fixes: f110f3188e563 ("iio: temperature: Add support for LTC2983")
> Cc: stable@vger.kernel.org
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>

I've tweaked this a tiny bit and applied it to the fixes-togreg branch
of iio.git on assumption it won't now go in until after the merge
window.  If merge window is delayed, I may sneak it in for that.

Thanks,

Jonathan

> ---
>  drivers/iio/temperature/ltc2983.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
> index 55ff28a0f1c7..c6641bc185cb 100644
> --- a/drivers/iio/temperature/ltc2983.c
> +++ b/drivers/iio/temperature/ltc2983.c
> @@ -1285,18 +1285,19 @@ static int ltc2983_parse_dt(struct ltc2983_data *st)
>  		ret = of_property_read_u32(child, "reg", &sensor.chan);
>  		if (ret) {
>  			dev_err(dev, "reg property must given for child nodes\n");
> -			return ret;
> +			goto put_child;
>  		}
>  
>  		/* check if we have a valid channel */
> +		ret = -EINVAL;

I'm not keen on this being here rather than in the error path.
It will obvious work, but is harder to read and more
fragile if future changes are made.

I've moved it down into the two cases this covers.

>  		if (sensor.chan < LTC2983_MIN_CHANNELS_NR ||
>  		    sensor.chan > LTC2983_MAX_CHANNELS_NR) {
>  			dev_err(dev,
>  				"chan:%d must be from 1 to 20\n", sensor.chan);
> -			return -EINVAL;
> +			goto put_child;
>  		} else if (channel_avail_mask & BIT(sensor.chan)) {
>  			dev_err(dev, "chan:%d already in use\n", sensor.chan);
> -			return -EINVAL;
> +			goto put_child;
>  		}
>  
>  		ret = of_property_read_u32(child, "adi,sensor-type",
> @@ -1304,7 +1305,7 @@ static int ltc2983_parse_dt(struct ltc2983_data *st)
>  		if (ret) {
>  			dev_err(dev,
>  				"adi,sensor-type property must given for child nodes\n");
> -			return ret;
> +			goto put_child;
>  		}
>  
>  		dev_dbg(dev, "Create new sensor, type %u, chann %u",
> @@ -1334,13 +1335,15 @@ static int ltc2983_parse_dt(struct ltc2983_data *st)
>  			st->sensors[chan] = ltc2983_adc_new(child, st, &sensor);
>  		} else {
>  			dev_err(dev, "Unknown sensor type %d\n", sensor.type);
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto put_child;
>  		}
>  
>  		if (IS_ERR(st->sensors[chan])) {
>  			dev_err(dev, "Failed to create sensor %ld",
>  				PTR_ERR(st->sensors[chan]));
> -			return PTR_ERR(st->sensors[chan]);
> +			ret = PTR_ERR(st->sensors[chan]);
> +			goto put_child;
>  		}
>  		/* set generic sensor parameters */
>  		st->sensors[chan]->chan = sensor.chan;
> @@ -1351,6 +1354,9 @@ static int ltc2983_parse_dt(struct ltc2983_data *st)
>  	}
>  
>  	return 0;
> +put_child:
> +	of_node_put(child);
> +	return ret;
>  }
>  
>  static int ltc2983_setup(struct ltc2983_data *st, bool assign_iio)