| Message ID | 20240522-fwnode_for_each_available_child_node_scoped-v1-2-1188b0da12dc@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | device property: introduce fwnode_for_each_available_child_node_scoped() | expand |
On 5/22/24 12:46, Javier Carrasco wrote: > On 22/05/2024 21:08, Guenter Roeck wrote: >> On 5/22/24 02:18, Javier Carrasco wrote: >>> The error path from a zero value of the "shunt-resistor-micro-ohms" >>> property does not decrement the refcount of the child node. >>> >>> Instead of adding the missing fwnode_handle_put(), a safer fix for >>> future modifications is using the _scoped version of the macro, >>> which removes the need for fwnode_handle_put() in all error paths. >>> >>> The macro defines the child node internally, which removes the need for >>> the current child node declaration as well. >>> >>> Fixes: 10b029020487 ("hwmon: (ltc2992) Avoid division by zero") >>> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> >> >> I really don't like fixes which depend on an API change. >> >> Guenter >> > > We could split the fix into two steps as suggested by Andy, where the > fix would simply add the missing fwnode_handle_put(), adding the > corresponding tag to include it in the stable kernels (the bug was > introduced with v6.7). By doing that, the new macro would not need to be > backported, which is probably a safer approach. > > The patch with the new macro would be left as it is, and another patch > would convert the the loop to use the new macro to avoid that a similar > bug arises again. That would be this 2/2 + dropping the additional > fwnode_handle_put(). > Please do. Thanks, Guenter > Best regards, > Javier Carrasco >
diff --git a/drivers/hwmon/ltc2992.c b/drivers/hwmon/ltc2992.c index 229aed15d5ca..3feee400ecf8 100644 --- a/drivers/hwmon/ltc2992.c +++ b/drivers/hwmon/ltc2992.c @@ -855,24 +855,19 @@ static const struct regmap_config ltc2992_regmap_config = { static int ltc2992_parse_dt(struct ltc2992_state *st) { struct fwnode_handle *fwnode; - struct fwnode_handle *child; u32 addr; u32 val; int ret; fwnode = dev_fwnode(&st->client->dev); - fwnode_for_each_available_child_node(fwnode, child) { + fwnode_for_each_available_child_node_scoped(fwnode, child) { ret = fwnode_property_read_u32(child, "reg", &addr); - if (ret < 0) { - fwnode_handle_put(child); + if (ret < 0) return ret; - } - if (addr > 1) { - fwnode_handle_put(child); + if (addr > 1) return -EINVAL; - } ret = fwnode_property_read_u32(child, "shunt-resistor-micro-ohms", &val); if (!ret) {
The error path from a zero value of the "shunt-resistor-micro-ohms" property does not decrement the refcount of the child node. Instead of adding the missing fwnode_handle_put(), a safer fix for future modifications is using the _scoped version of the macro, which removes the need for fwnode_handle_put() in all error paths. The macro defines the child node internally, which removes the need for the current child node declaration as well. Fixes: 10b029020487 ("hwmon: (ltc2992) Avoid division by zero") Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> --- drivers/hwmon/ltc2992.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)