From: Zijun Hu quic_zijuhu@quicinc.com
dev_pm_get_subsys_data() has below 2 issues under condition (@dev->power.subsys_data != NULL):
- it will do unnecessary kzalloc() and kfree(). - it will return -ENOMEM if the kzalloc() fails, that is wrong since the kzalloc() is not needed.
Fixed by not doing kzalloc() and returning 0 for the condition.
Fixes: ef27bed1870d ("PM: Reference counting of power.subsys_data") Cc: stable@vger.kernel.org Signed-off-by: Zijun Hu quic_zijuhu@quicinc.com --- drivers/base/power/common.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c index 8c34ae1cd8d5..13cb1f2a06e7 100644 --- a/drivers/base/power/common.c +++ b/drivers/base/power/common.c @@ -26,6 +26,14 @@ int dev_pm_get_subsys_data(struct device *dev) { struct pm_subsys_data *psd;
+ spin_lock_irq(&dev->power.lock); + if (dev->power.subsys_data) { + dev->power.subsys_data->refcount++; + spin_unlock_irq(&dev->power.lock); + return 0; + } + spin_unlock_irq(&dev->power.lock); + psd = kzalloc(sizeof(*psd), GFP_KERNEL); if (!psd) return -ENOMEM;
--- base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc change-id: 20241010-fix_dev_pm_get_subsys_data-2478bb200fde
Best regards,
On Mon, Oct 28, 2024 at 08:31:11PM +0800, Zijun Hu wrote:
From: Zijun Hu quic_zijuhu@quicinc.com
dev_pm_get_subsys_data() has below 2 issues under condition (@dev->power.subsys_data != NULL):
- it will do unnecessary kzalloc() and kfree().
But that's ok, everything still works, right?
- it will return -ENOMEM if the kzalloc() fails, that is wrong since the kzalloc() is not needed.
But it's ok to return the proper error if the system is that broken.
Fixed by not doing kzalloc() and returning 0 for the condition.
Fixes: ef27bed1870d ("PM: Reference counting of power.subsys_data") Cc: stable@vger.kernel.org
Why is this relevant for stable kernels?
thanks,
greg k-h
On 2024/11/12 19:46, Greg Kroah-Hartman wrote:
On Mon, Oct 28, 2024 at 08:31:11PM +0800, Zijun Hu wrote:
From: Zijun Hu quic_zijuhu@quicinc.com
dev_pm_get_subsys_data() has below 2 issues under condition (@dev->power.subsys_data != NULL):
- it will do unnecessary kzalloc() and kfree().
But that's ok, everything still works, right?
yes.
- it will return -ENOMEM if the kzalloc() fails, that is wrong since the kzalloc() is not needed.
But it's ok to return the proper error if the system is that broken.
IMO, the API should return 0 (success) instead of -ENOMEM since it does not need to do kzalloc().
Different return value should impact caller's logic.
Fixed by not doing kzalloc() and returning 0 for the condition.
Fixes: ef27bed1870d ("PM: Reference counting of power.subsys_data") Cc: stable@vger.kernel.org
Why is this relevant for stable kernels?
you can remove both Fix and stable tag directly if you like this change.(^^)
thanks,
greg k-h
On 2024/11/12 19:46, Greg Kroah-Hartman wrote:
On Mon, Oct 28, 2024 at 08:31:11PM +0800, Zijun Hu wrote:
From: Zijun Hu quic_zijuhu@quicinc.com
dev_pm_get_subsys_data() has below 2 issues under condition (@dev->power.subsys_data != NULL):
- it will do unnecessary kzalloc() and kfree().
But that's ok, everything still works, right?
i don't think so, as explained below:
under condition (@dev->power.subsys_data != NULL), the API does not need to do kzalloc() and should always return 0 (success).
but it actually does *unnecessary* kzalloc() and have below impact shown:
if (kzalloc() is successfully) it will degrade the API's performance else it changed the API's return value to -ENOMEM from 0, that will impact caller's logic.
- it will return -ENOMEM if the kzalloc() fails, that is wrong since the kzalloc() is not needed.
But it's ok to return the proper error if the system is that broken.
Fixed by not doing kzalloc() and returning 0 for the condition.
Fixes: ef27bed1870d ("PM: Reference counting of power.subsys_data") Cc: stable@vger.kernel.org
Why is this relevant for stable kernels?
it has impact related to performance and logic as explained above.
thanks,
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg Kroah-Hartman -
Zijun Hu