[PATCH] firmware: stratix10-svc: Use proper atomic to fix possible double free

[PATCH] firmware: stratix10-svc: Use proper atomic to fix possible double free

[Krzysztof Kozlowski]

Doing atomic_dec() and then separate atomic_read() leaves small race
window leading to double free in stratix10_svc_remove_async_client():

CPU 0                               CPU 1
=====                               =====
refcount == 2

stratix10_svc_remove_async_client() stratix10_svc_remove_async_client()
atomic_dec(&refcount);
                                    atomic_dec(&refcount);
if (atomic_read(&refcount) == 0)
  kfree(achan);
                                    if (atomic_read(&refcount) == 0)
                                      kfree(achan); // double free

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-506b1474-e503-4085-b62a-929cde036317
Fixes: bcb9f4f07061 ("firmware: stratix10-svc: Add support for async communication")
Cc: <stable@vger.kernel.org>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
 drivers/firmware/stratix10-svc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
index 88391ea161ed..aca4d354bdd7 100644
--- a/drivers/firmware/stratix10-svc.c
+++ b/drivers/firmware/stratix10-svc.c
@@ -1444,8 +1444,7 @@ int stratix10_svc_remove_async_client(struct stratix10_svc_chan *chan)
 	}
 
 	if (achan == actrl->common_async_chan) {
-		atomic_dec(&actrl->common_achan_refcount);
-		if (atomic_read(&actrl->common_achan_refcount) == 0) {
+		if (atomic_dec_and_test(&actrl->common_achan_refcount)) {
 			ida_destroy(&achan->job_id_pool);
 			ida_free(&actrl->async_id_pool,
 				 achan->async_client_id);
-- 
2.53.0

Re: [PATCH] firmware: stratix10-svc: Use proper atomic to fix possible double free

[Dinh Nguyen]

On 11/1/26 06:29, Krzysztof Kozlowski wrote:
> Doing atomic_dec() and then separate atomic_read() leaves small race
> window leading to double free in stratix10_svc_remove_async_client():
> 
> CPU 0                               CPU 1
> =====                               =====
> refcount == 2
> 
> stratix10_svc_remove_async_client() stratix10_svc_remove_async_client()
> atomic_dec(&refcount);
>                                      atomic_dec(&refcount);
> if (atomic_read(&refcount) == 0)
>    kfree(achan);
>                                      if (atomic_read(&refcount) == 0)
>                                        kfree(achan); // double free
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/bug/linux-506b1474-e503-4085-b62a-929cde036317
> Fixes: bcb9f4f07061 ("firmware: stratix10-svc: Add support for async communication")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> ---
>   drivers/firmware/stratix10-svc.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
> index 88391ea161ed..aca4d354bdd7 100644
> --- a/drivers/firmware/stratix10-svc.c
> +++ b/drivers/firmware/stratix10-svc.c
> @@ -1444,8 +1444,7 @@ int stratix10_svc_remove_async_client(struct stratix10_svc_chan *chan)
>   	}
>   
>   	if (achan == actrl->common_async_chan) {
> -		atomic_dec(&actrl->common_achan_refcount);
> -		if (atomic_read(&actrl->common_achan_refcount) == 0) {
> +		if (atomic_dec_and_test(&actrl->common_achan_refcount)) {
>   			ida_destroy(&achan->job_id_pool);
>   			ida_free(&actrl->async_id_pool,
>   				 achan->async_client_id);

Applied!

Thanks,
Dinh