From: Pierre-Louis Bossart <[email protected]>
The documentation for device_get_named_child_node() mentions this
important point:
"
The caller is responsible for calling fwnode_handle_put() on the
returned fwnode pointer.
"
Add fwnode_handle_put() to avoid leaked references.
Signed-off-by: Pierre-Louis Bossart <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/amd_manager.c | 3 +++
drivers/soundwire/intel_auxdevice.c | 6 +++++-
drivers/soundwire/mipi_disco.c | 30 +++++++++++++++++++++++------
3 files changed, 32 insertions(+), 7 deletions(-)
diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
index 20d94bcfc9b4..795e223f7e5c 100644
--- a/drivers/soundwire/amd_manager.c
+++ b/drivers/soundwire/amd_manager.c
@@ -571,6 +571,9 @@ static int sdw_master_read_amd_prop(struct sdw_bus *bus)
amd_manager->wake_en_mask = wake_en_mask;
fwnode_property_read_u32(link, "amd-sdw-power-mode", &power_mode_mask);
amd_manager->power_mode_mask = power_mode_mask;
+
+ fwnode_handle_put(link);
+
return 0;
}
diff --git a/drivers/soundwire/intel_auxdevice.c b/drivers/soundwire/intel_auxdevice.c
index 17cf27e6ea73..18517121cc89 100644
--- a/drivers/soundwire/intel_auxdevice.c
+++ b/drivers/soundwire/intel_auxdevice.c
@@ -155,8 +155,10 @@ static int sdw_master_read_intel_prop(struct sdw_bus *bus)
SDW_MASTER_QUIRKS_CLEAR_INITIAL_PARITY;
intel_prop = devm_kzalloc(bus->dev, sizeof(*intel_prop), GFP_KERNEL);
- if (!intel_prop)
+ if (!intel_prop) {
+ fwnode_handle_put(link);
return -ENOMEM;
+ }
/* initialize with hardware defaults, in case the properties are not found */
intel_prop->doaise = 0x1;
@@ -184,6 +186,8 @@ static int sdw_master_read_intel_prop(struct sdw_bus *bus)
intel_prop->dodse,
intel_prop->dods);
+ fwnode_handle_put(link);
+
return 0;
}
diff --git a/drivers/soundwire/mipi_disco.c b/drivers/soundwire/mipi_disco.c
index 55a9c51c84c1..e5d9df26d4dc 100644
--- a/drivers/soundwire/mipi_disco.c
+++ b/drivers/soundwire/mipi_disco.c
@@ -66,8 +66,10 @@ int sdw_master_read_prop(struct sdw_bus *bus)
prop->clk_freq = devm_kcalloc(bus->dev, prop->num_clk_freq,
sizeof(*prop->clk_freq),
GFP_KERNEL);
- if (!prop->clk_freq)
+ if (!prop->clk_freq) {
+ fwnode_handle_put(link);
return -ENOMEM;
+ }
fwnode_property_read_u32_array(link,
"mipi-sdw-clock-frequencies-supported",
@@ -92,8 +94,10 @@ int sdw_master_read_prop(struct sdw_bus *bus)
prop->clk_gears = devm_kcalloc(bus->dev, prop->num_clk_gears,
sizeof(*prop->clk_gears),
GFP_KERNEL);
- if (!prop->clk_gears)
+ if (!prop->clk_gears) {
+ fwnode_handle_put(link);
return -ENOMEM;
+ }
fwnode_property_read_u32_array(link,
"mipi-sdw-supported-clock-gears",
@@ -116,6 +120,8 @@ int sdw_master_read_prop(struct sdw_bus *bus)
fwnode_property_read_u32(link, "mipi-sdw-command-error-threshold",
&prop->err_threshold);
+ fwnode_handle_put(link);
+
return 0;
}
EXPORT_SYMBOL(sdw_master_read_prop);
@@ -197,8 +203,10 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave,
dpn[i].num_words,
sizeof(*dpn[i].words),
GFP_KERNEL);
- if (!dpn[i].words)
+ if (!dpn[i].words) {
+ fwnode_handle_put(node);
return -ENOMEM;
+ }
fwnode_property_read_u32_array(node,
"mipi-sdw-port-wordlength-configs",
@@ -236,8 +244,10 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave,
dpn[i].num_channels,
sizeof(*dpn[i].channels),
GFP_KERNEL);
- if (!dpn[i].channels)
+ if (!dpn[i].channels) {
+ fwnode_handle_put(node);
return -ENOMEM;
+ }
fwnode_property_read_u32_array(node,
"mipi-sdw-channel-number-list",
@@ -251,8 +261,10 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave,
dpn[i].num_ch_combinations,
sizeof(*dpn[i].ch_combinations),
GFP_KERNEL);
- if (!dpn[i].ch_combinations)
+ if (!dpn[i].ch_combinations) {
+ fwnode_handle_put(node);
return -ENOMEM;
+ }
fwnode_property_read_u32_array(node,
"mipi-sdw-channel-combination-list",
@@ -274,6 +286,8 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave,
/* TODO: Read audio mode */
+ fwnode_handle_put(node);
+
i++;
}
@@ -348,10 +362,14 @@ int sdw_slave_read_prop(struct sdw_slave *slave)
prop->dp0_prop = devm_kzalloc(&slave->dev,
sizeof(*prop->dp0_prop),
GFP_KERNEL);
- if (!prop->dp0_prop)
+ if (!prop->dp0_prop) {
+ fwnode_handle_put(port);
return -ENOMEM;
+ }
sdw_slave_read_dp0(slave, port, prop->dp0_prop);
+
+ fwnode_handle_put(port);
}
/*
--
2.40.1
> Add fwnode_handle_put() to avoid leaked references.
Are you going to respond also to my previous patch review
in more constructive ways?
https://lore.kernel.org/lkml/[email protected]/
https://lkml.org/lkml/2024/4/29/493
Regards,
Markus
On 5/28/24 10:19, Markus Elfring wrote:
>> Add fwnode_handle_put() to avoid leaked references.
>
> Are you going to respond also to my previous patch review
> in more constructive ways?
> https://lore.kernel.org/lkml/[email protected]/
> https://lkml.org/lkml/2024/4/29/493
Sorry about that, both Bard and I missed your comments.
On the Fixes tag: I made a deliberate choice to add all the fixes in one
patch, to show that the usage was copy-pasted and done 'wrong' in
multiple places. That makes it really hard to add a Fixes tag since the
different uses were added in a time interval of about 5 years.
We could split and have multiple patches if that was desired, but I
would still not include a Fixes tag since the leaked references are not
that bad, we read the Manager properties on probe, and the peripheral
properties are generally not used by codec drivers, so it's unlikely
that any user will ever see a problem that requires a backport in
linux-stable. This problem was found by reading the documentation while
adding new things, not by any user report or test failure.
The error flow was revisited and hardened in a follow-up patchset which
also adds new properties for MIPI DisCo 2.1 spec [1], we'll share the
patches in this kernel cycle.
[1] https://github.com/thesofproject/linux/pull/4857