iio: dac: ad5770r: Put fwnode in error case during ->probe()
Subsystems (2)
PATCH 1/1 iio: dac: ad5770r: Put fwnode in error case during ->probe()
device_for_each_child_node() bumps a reference counting of a returned variable. We have to balance it whenever we return to the caller.
--- drivers/iio/dac/ad5770r.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
drivers/iio/dac/ad5770r.c +11 -5
@@ -524,23 +524,29 @@ static int ad5770r_channel_config(struct ad5770r_state *st)device_for_each_child_node (&st->spi->dev, child) { ret =fwnode_property_read_u32 (child,"num" , &num);if (ret) -return ret; -if (num >=AD5770R_MAX_CHANNELS ) -return -EINVAL ; + goto err_child_out; +if (num >=AD5770R_MAX_CHANNELS ) { + ret = -EINVAL ; + goto err_child_out; + } ret =fwnode_property_read_u32_array (child,"adi,range-microamp" , tmp,2 );if (ret) -return ret; + goto err_child_out; min = tmp[0 ] /1000 ; max = tmp[1 ] /1000 ; ret =ad5770r_store_output_range (st, min, max, num);if (ret) -return ret; + goto err_child_out; } +return 0 ; + +err_child_out: +fwnode_handle_put (child);return ret; }
Signature · 1 line
2.31.1
Quoted 3 lines · Andy Shevchenko
device_for_each_child_node()bumps a reference counting of a returned variable. We have to balance it whenever we return to the caller.
This feels like the device_for_each_child_node() [and the of_node variant] iterator[s] need[s] some re-design to account for how easy it is to slip stuff by.
But that's something quite out of this scope.
Quoted 50 lines
Fixes cbbb819837f6 ("iio: dac: ad5770r: Add AD5770R support")Cc Alexandru Tachici <[email protected]>Signed-off-by Andy Shevchenko <[email protected]>--- drivers/iio/dac/ad5770r.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)drivers/iio/dac/ad5770r.c +11
-6@@ -524,23 +524,29 @@ static int ad5770r_channel_config(struct ad5770r_state *st)device_for_each_child_node (&st->spi->dev, child) { ret =fwnode_property_read_u32 (child,"num" , &num);if (ret) -return ret; -if (num >=AD5770R_MAX_CHANNELS ) -return -EINVAL ; + goto err_child_out; +if (num >=AD5770R_MAX_CHANNELS ) { + ret = -EINVAL ; + goto err_child_out; + } ret =fwnode_property_read_u32_array (child,"adi,range-microamp" , tmp,2 );if (ret) -return ret; + goto err_child_out; min = tmp[0 ] /1000 ; max = tmp[1 ] /1000 ; ret =ad5770r_store_output_range (st, min, max, num);if (ret) -return ret; + goto err_child_out; } +return 0 ; + +err_child_out: +fwnode_handle_put (child);return ret; } --2.31.1
A message that is not in the archive (sent off-list or not yet received): CAHp75VfFTnS6+6KwqixTXNsMvC_LWGnY0rv3n9g_-i7HK2eq0w@mail.gmail.com
Quoted 23 lines · On Thu, 13 May 2021 01:00:22 +0300 Andy Shevchenko <[email protected]>…
Quoted 10 lines · Alexandru Ardelean
Quoted 2 lines · Andy Shevchenko
device_for_each_child_node()bumps a reference counting of a returnedvariable.
Quoted 1 line
We have to balance it whenever we return to the caller.
This feels like the
device_for_each_child_node()[and theof_nodevariant] iterator[s] need[s] some re-design to account for how easy it is to slip stuff by.I don’t know how you can do it differently if you wish to give a guarantee that object doesn’t suddenly disappear while you are iterating over it.
Look at the
bus_find_device()and similar APIs, they basically do the similar.
Quoted 7 lines
Quoted 5 lines
But that's something quite out of this scope.
Reviewed-by Alexandru Ardelean <[email protected]>Thanks.
Applied to the fixes-togreg branch of iio.git and marked for stable.
Thanks,
Joathan
Quoted 56 lines
Quoted 54 lines
drivers/iio/dac/ad5770r.c static int ad5770r_channel_config(struct
Quoted 14 lines
Fixes cbbb819837f6 ("iio: dac: ad5770r: Add AD5770R support")Cc Alexandru Tachici <[email protected]>Signed-off-by Andy Shevchenko <[email protected]>--- drivers/iio/dac/ad5770r.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)drivers/iio/dac/ad5770r.c +0
-0@@ -524,23 +524,29 @@ static int ad5770r_channel_config(struct
ad5770r_state*st)Quoted 4 lines
drivers/iio/dac/ad5770r.c +0
-1device_for_each_child_node (&st->spi->dev, child) { ret =fwnode_property_read_u32 (child,"num" , &num);if (ret) -return ret;drivers/iio/dac/ad5770r.c static int ad5770r_channel_config(struct ad5770r_state *st) · line 538
Quoted 10 lines
drivers/iio/dac/ad5770r.c +5
-2-if (num >=AD5770R_MAX_CHANNELS ) -return -EINVAL ; + goto err_child_out; +if (num >=AD5770R_MAX_CHANNELS ) { + ret = -EINVAL ; + goto err_child_out; + } ret = fwnode_property_read_u32_array(child,"adi,range-microamp",
Quoted 23 lines
drivers/iio/dac/ad5770r.c +6
-3tmp, 2);if (ret) -return ret; + goto err_child_out; min = tmp[0 ] /1000 ; max = tmp[1 ] /1000 ; ret =ad5770r_store_output_range (st, min, max, num);if (ret) -return ret; + goto err_child_out; } +return 0 ; + +err_child_out: +fwnode_handle_put (child);return ret; } --2.31.1