This series addresses two issues in spm_cpuidle_register: a missing device_node release in an error path, and releasing a reference to a device after its usage.
These issues were found while analyzing the code, and the patches have been successfully compiled and statically analyzed, but not tested on real hardware as I don't have access to it. Any volunteering for testing is always more than welcome.
Signed-off-by: Javier Carrasco javier.carrasco.cruz@gmail.com --- Javier Carrasco (2): cpuidle: qcom-spm: fix device node release in spm_cpuidle_register cpuidle: qcom-spm: fix platform device release in spm_cpuidle_register
drivers/cpuidle/cpuidle-qcom-spm.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) --- base-commit: 6fb2fa9805c501d9ade047fc511961f3273cdcb5 change-id: 20241029-cpuidle-qcom-spm-cleanup-6f03669bcd70
Best regards,
If of_find_device_by_node() fails, its error path does not include a call to of_node_put(cpu_node), which has been successfully acquired at this point.
Move the existing of_node_put(cpu_node) to the point where cpu_node is no longer required, covering all code paths and avoiding leaking the resource in any case.
Cc: stable@vger.kernel.org Fixes: 60f3692b5f0b ("cpuidle: qcom_spm: Detach state machine from main SPM handling") Signed-off-by: Javier Carrasco javier.carrasco.cruz@gmail.com --- drivers/cpuidle/cpuidle-qcom-spm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/cpuidle/cpuidle-qcom-spm.c b/drivers/cpuidle/cpuidle-qcom-spm.c index 3ab240e0e122..c9ab49b310fd 100644 --- a/drivers/cpuidle/cpuidle-qcom-spm.c +++ b/drivers/cpuidle/cpuidle-qcom-spm.c @@ -96,12 +96,12 @@ static int spm_cpuidle_register(struct device *cpuidle_dev, int cpu) return -ENODEV;
saw_node = of_parse_phandle(cpu_node, "qcom,saw", 0); + of_node_put(cpu_node); if (!saw_node) return -ENODEV;
pdev = of_find_device_by_node(saw_node); of_node_put(saw_node); - of_node_put(cpu_node); if (!pdev) return -ENODEV;
On Wed, Oct 30, 2024 at 07:38:32AM +0100, Javier Carrasco wrote:
If of_find_device_by_node() fails, its error path does not include a call to of_node_put(cpu_node), which has been successfully acquired at this point.
Move the existing of_node_put(cpu_node) to the point where cpu_node is no longer required, covering all code paths and avoiding leaking the resource in any case.
Cc: stable@vger.kernel.org Fixes: 60f3692b5f0b ("cpuidle: qcom_spm: Detach state machine from main SPM handling") Signed-off-by: Javier Carrasco javier.carrasco.cruz@gmail.com
drivers/cpuidle/cpuidle-qcom-spm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org
A reference to a device obtained via of_find_device_by_node() requires explicit calls to put_device() when it is no longer required to avoid leaking the resource.
Add the missing calls to put_device(&pdev->dev) in the success path as well as in the only error path before the device is no longer required.
Note that the acquired device is neither assigned nor used to manage additional resources, and it can be released right after using it.
Cc: stable@vger.kernel.org Fixes: 60f3692b5f0b ("cpuidle: qcom_spm: Detach state machine from main SPM handling") Signed-off-by: Javier Carrasco javier.carrasco.cruz@gmail.com --- drivers/cpuidle/cpuidle-qcom-spm.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/cpuidle/cpuidle-qcom-spm.c b/drivers/cpuidle/cpuidle-qcom-spm.c index c9ab49b310fd..601aa81ffff3 100644 --- a/drivers/cpuidle/cpuidle-qcom-spm.c +++ b/drivers/cpuidle/cpuidle-qcom-spm.c @@ -106,10 +106,13 @@ static int spm_cpuidle_register(struct device *cpuidle_dev, int cpu) return -ENODEV;
data = devm_kzalloc(cpuidle_dev, sizeof(*data), GFP_KERNEL); - if (!data) + if (!data) { + put_device(&pdev->dev); return -ENOMEM; + }
data->spm = dev_get_drvdata(&pdev->dev); + put_device(&pdev->dev); if (!data->spm) return -EINVAL;
On Wed, Oct 30, 2024 at 07:38:33AM +0100, Javier Carrasco wrote:
A reference to a device obtained via of_find_device_by_node() requires explicit calls to put_device() when it is no longer required to avoid leaking the resource.
Add the missing calls to put_device(&pdev->dev) in the success path as well as in the only error path before the device is no longer required.
Note that the acquired device is neither assigned nor used to manage additional resources, and it can be released right after using it.
Well... This raises one question: if the device is put, then its drvdata can go away. But at the same time the drvdata can also go away if the SPM device is just unbound from the driver. Granted the age of those platforms it's probably not worth refactoring the drivers too much.
Reviewed-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org
Cc: stable@vger.kernel.org Fixes: 60f3692b5f0b ("cpuidle: qcom_spm: Detach state machine from main SPM handling") Signed-off-by: Javier Carrasco javier.carrasco.cruz@gmail.com
drivers/cpuidle/cpuidle-qcom-spm.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/cpuidle/cpuidle-qcom-spm.c b/drivers/cpuidle/cpuidle-qcom-spm.c index c9ab49b310fd..601aa81ffff3 100644 --- a/drivers/cpuidle/cpuidle-qcom-spm.c +++ b/drivers/cpuidle/cpuidle-qcom-spm.c @@ -106,10 +106,13 @@ static int spm_cpuidle_register(struct device *cpuidle_dev, int cpu) return -ENODEV; data = devm_kzalloc(cpuidle_dev, sizeof(*data), GFP_KERNEL);
- if (!data)
- if (!data) {
put_device(&pdev->dev);- }
data->spm = dev_get_drvdata(&pdev->dev);
- put_device(&pdev->dev); if (!data->spm) return -EINVAL;
-- 2.43.0
linux-stable-mirror@lists.linaro.org
-
Dmitry Baryshkov -
Javier Carrasco