[PATCH v2] soundwire: mipi_disco: Put fwnode on property array read errors

[PATCH v2] soundwire: mipi_disco: Put fwnode on property array read errors

[Xiangwu Lai]

sdw_master_read_prop() and sdw_slave_read_dpn() take a fwnode
reference with device_get_named_child_node() and already release it
on the -ENOMEM and success paths.  However, the error paths added
for the property array reads jump straight out without dropping the
reference, leaking it on every failed read.  The lane-list support
in sdw_slave_read_dpn() is even missing the put on its -ENOMEM path,
unlike the other array allocations in the same function.

Release the reference on those error paths as well, matching the
existing -ENOMEM handling in the same functions.

Fixes: a489afc105ed ("soundwire: mipi-disco: add error handling for property array read")
Fixes: 71b405b18444 ("soundwire: mipi-disco: add support for DP0/DPn 'lane-list' property")
Signed-off-by: Xiangwu Lai <laixiangwu@cmss.chinamobile.com>
---
Changes in v2:

- Fix the subject of the first Fixes tag to match the commit title
  ("mipi-disco" instead of "mipi_disco") and add a second Fixes tag
  for the lane-list error paths, which were introduced by a different
  commit and also cover the missing put on the lane-list -ENOMEM
  path (suggested by Sashiko AI review).
- Note: an earlier attempt to fix these leaks using __free() was
  posted in 2024 but never merged:
  https://lore.kernel.org/all/20241019-mipi_disco-of_node_put-v1-1-1de27e6852c4@gmail.com/

Link: https://lore.kernel.org/all/20261112021827.1835-1-laixiangwu@cmss.chinamobile.com
 drivers/soundwire/mipi_disco.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/soundwire/mipi_disco.c b/drivers/soundwire/mipi_disco.c
index 259ad98b5..65f8a41e6 100644
--- a/drivers/soundwire/mipi_disco.c
+++ b/drivers/soundwire/mipi_disco.c
@@ -96,8 +96,10 @@ int sdw_master_read_prop(struct sdw_bus *bus)
 		ret = fwnode_property_read_u32_array(link,
 				"mipi-sdw-clock-frequencies-supported",
 				prop->clk_freq, prop->num_clk_freq);
-		if (ret < 0)
+		if (ret < 0) {
+			fwnode_handle_put(link);
 			return ret;
+		}
 	}
 
 	/*
@@ -132,8 +134,10 @@ int sdw_master_read_prop(struct sdw_bus *bus)
 					       scales_prop,
 					       prop->clk_gears,
 					       prop->num_clk_gears);
-		if (ret < 0)
+		if (ret < 0) {
+			fwnode_handle_put(link);
 			return ret;
+		}
 	}
 
 	fwnode_property_read_u32(link, "mipi-sdw-default-frame-rate",
@@ -262,8 +266,10 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave,
 			ret = fwnode_property_read_u32_array(node,
 					"mipi-sdw-port-wordlength-configs",
 					dpn[i].words, dpn[i].num_words);
-			if (ret < 0)
+			if (ret < 0) {
+				fwnode_handle_put(node);
 				return ret;
+			}
 		}
 
 		fwnode_property_read_u32(node, "mipi-sdw-data-port-type",
@@ -305,8 +311,10 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave,
 			ret = fwnode_property_read_u32_array(node,
 					"mipi-sdw-channel-number-list",
 					dpn[i].channels, dpn[i].num_channels);
-			if (ret < 0)
+			if (ret < 0) {
+				fwnode_handle_put(node);
 				return ret;
+			}
 		}
 
 		nval = fwnode_property_count_u32(node, "mipi-sdw-channel-combination-list");
@@ -325,8 +333,10 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave,
 					"mipi-sdw-channel-combination-list",
 					dpn[i].ch_combinations,
 					dpn[i].num_ch_combinations);
-			if (ret < 0)
+			if (ret < 0) {
+				fwnode_handle_put(node);
 				return ret;
+			}
 		}
 
 		fwnode_property_read_u32(node,
@@ -347,14 +357,18 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave,
 			dpn[i].lane_list = devm_kcalloc(&slave->dev,
 							dpn[i].num_lanes, sizeof(*dpn[i].lane_list),
 							GFP_KERNEL);
-			if (!dpn[i].lane_list)
+			if (!dpn[i].lane_list) {
+				fwnode_handle_put(node);
 				return -ENOMEM;
+			}
 
 			ret = fwnode_property_read_u32_array(node,
 						       "mipi-sdw-lane-list",
 						       dpn[i].lane_list, dpn[i].num_lanes);
-			if (ret < 0)
+			if (ret < 0) {
+				fwnode_handle_put(node);
 				return ret;
+			}
 		}
 
 		fwnode_handle_put(node);
-- 
2.50.1.windows.1

Re: [PATCH v2] soundwire: mipi_disco: Put fwnode on property array read errors

[Pierre-Louis Bossart]

On 11/12/26 07:09, Xiangwu Lai wrote:
> sdw_master_read_prop() and sdw_slave_read_dpn() take a fwnode
> reference with device_get_named_child_node() and already release it
> on the -ENOMEM and success paths.  However, the error paths added
> for the property array reads jump straight out without dropping the
> reference, leaking it on every failed read.  The lane-list support
> in sdw_slave_read_dpn() is even missing the put on its -ENOMEM path,
> unlike the other array allocations in the same function.
> 
> Release the reference on those error paths as well, matching the
> existing -ENOMEM handling in the same functions.
> 
> Fixes: a489afc105ed ("soundwire: mipi-disco: add error handling for property array read")
> Fixes: 71b405b18444 ("soundwire: mipi-disco: add support for DP0/DPn 'lane-list' property")
> Signed-off-by: Xiangwu Lai <laixiangwu@cmss.chinamobile.com>
> ---
> Changes in v2:
> 
> - Fix the subject of the first Fixes tag to match the commit title
>   ("mipi-disco" instead of "mipi_disco") and add a second Fixes tag
>   for the lane-list error paths, which were introduced by a different
>   commit and also cover the missing put on the lane-list -ENOMEM
>   path (suggested by Sashiko AI review).
> - Note: an earlier attempt to fix these leaks using __free() was
>   posted in 2024 but never merged:
>   https://lore.kernel.org/all/20241019-mipi_disco-of_node_put-v1-1-1de27e6852c4@gmail.com/

Thanks for the patch, the existing code is indeed inconsistent. LGTM.

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>