[5.10,105/299] crypto: stm32/cryp - Fix PM reference leak on stm32-cryp.c

From: Shixin Liu <liushixin2@huawei.com>

[ Upstream commit 747bf30fd944f02f341b5f3bc7d97a13f2ae2fbe ]

pm_runtime_get_sync will increment pm usage counter even it failed.
Forgetting to putting operation will result in reference leak here.
Fix it by replacing it with pm_runtime_resume_and_get to keep usage
counter balanced.

Signed-off-by: Shixin Liu <liushixin2@huawei.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/crypto/stm32/stm32-cryp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

On Mon 2021-05-10 12:18:22, Greg Kroah-Hartman wrote:
> From: Shixin Liu <liushixin2@huawei.com>
> 
> [ Upstream commit 747bf30fd944f02f341b5f3bc7d97a13f2ae2fbe ]
> 
> pm_runtime_get_sync will increment pm usage counter even it failed.
> Forgetting to putting operation will result in reference leak here.
> Fix it by replacing it with pm_runtime_resume_and_get to keep usage
> counter balanced.

Yes, but that only works when code checks the return value.

> +++ b/drivers/crypto/stm32/stm32-cryp.c
> @@ -542,7 +542,7 @@ static int stm32_cryp_hw_init(struct stm32_cryp *cryp)
>  	int ret;
>  	u32 cfg, hw_mode;
>  
> -	pm_runtime_get_sync(cryp->dev);
> +	pm_runtime_resume_and_get(cryp->dev);
>  
>  	/* Disable interrupt */
>  	stm32_cryp_write(cryp, CRYP_IMSCR, 0);

Again, this is wrong.

> @@ -2043,7 +2043,7 @@ static int stm32_cryp_remove(struct platform_device *pdev)
>  	if (!cryp)
>  		return -ENODEV;
>  
> -	ret = pm_runtime_get_sync(cryp->dev);
> +	ret = pm_runtime_resume_and_get(cryp->dev);
>  	if (ret < 0)
>  		return ret;
>  

But this may be right.

Best regards,
								Pavel

On 2021/5/10 20:07, Pavel Machek wrote:
> On Mon 2021-05-10 12:18:22, Greg Kroah-Hartman wrote:

>> From: Shixin Liu <liushixin2@huawei.com>

>>

>> [ Upstream commit 747bf30fd944f02f341b5f3bc7d97a13f2ae2fbe ]

>>

>> pm_runtime_get_sync will increment pm usage counter even it failed.

>> Forgetting to putting operation will result in reference leak here.

>> Fix it by replacing it with pm_runtime_resume_and_get to keep usage

>> counter balanced.

> Yes, but that only works when code checks the return value.

Thank you for the correction. Yes, You are right that if we carry out runtime resume
failed on the path where the return value is not checked, the pm usage counter will
be put in later path.

But I have another question. Why don't we check the return values in these path?
Does that mean these resumes will never fail?
>> +++ b/drivers/crypto/stm32/stm32-cryp.c

>> @@ -542,7 +542,7 @@ static int stm32_cryp_hw_init(struct stm32_cryp *cryp)

>>  	int ret;

>>  	u32 cfg, hw_mode;

>>  

>> -	pm_runtime_get_sync(cryp->dev);

>> +	pm_runtime_resume_and_get(cryp->dev);

>>  

>>  	/* Disable interrupt */

>>  	stm32_cryp_write(cryp, CRYP_IMSCR, 0);

> Again, this is wrong.

>

>> @@ -2043,7 +2043,7 @@ static int stm32_cryp_remove(struct platform_device *pdev)

>>  	if (!cryp)

>>  		return -ENODEV;

>>  

>> -	ret = pm_runtime_get_sync(cryp->dev);

>> +	ret = pm_runtime_resume_and_get(cryp->dev);

>>  	if (ret < 0)

>>  		return ret;

>>  

> But this may be right.

>

> Best regards,

> 								Pavel