[PATCH] plugins/hotblocks: Fix potential deadlock in plugin_exit() function

Cong Liu posted 1 patch 7 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230921061231.60948-1-liucong2@kylinos.cn
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, Alexandre Iooss <erdnaxe@crans.org>, Mahmoud Mandour <ma.mandourr@gmail.com>
There is a newer version of this series
contrib/plugins/hotblocks.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH] plugins/hotblocks: Fix potential deadlock in plugin_exit() function
Posted by Cong Liu 7 months, 1 week ago
This patch fixes a potential deadlock in the plugin_exit() function of QEMU.
The original code does not release the lock mutex if it is NULL. This patch
adds a check for it being NULL and releases the mutex in that case.

Signed-off-by: Cong Liu <liucong2@kylinos.cn>
---
 contrib/plugins/hotblocks.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c
index 6b74d25fead6..1f713f1904f3 100644
--- a/contrib/plugins/hotblocks.c
+++ b/contrib/plugins/hotblocks.c
@@ -70,6 +70,8 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
 
         g_list_free(it);
         g_mutex_unlock(&lock);
+    } else {
+        g_mutex_unlock(&lock);
     }
 
     qemu_plugin_outs(report->str);
-- 
2.34.1
Re: [PATCH] plugins/hotblocks: Fix potential deadlock in plugin_exit() function
Posted by Philippe Mathieu-Daudé 7 months, 1 week ago
Hi Cong,

On 21/9/23 08:12, Cong Liu wrote:
> This patch fixes a potential deadlock in the plugin_exit() function of QEMU.
> The original code does not release the lock mutex if it is NULL. This patch
> adds a check for it being NULL and releases the mutex in that case.

You are correct.

> Signed-off-by: Cong Liu <liucong2@kylinos.cn>
> ---
>   contrib/plugins/hotblocks.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c
> index 6b74d25fead6..1f713f1904f3 100644
> --- a/contrib/plugins/hotblocks.c
> +++ b/contrib/plugins/hotblocks.c
> @@ -70,6 +70,8 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
>   
>           g_list_free(it);
>           g_mutex_unlock(&lock);
> +    } else {
> +        g_mutex_unlock(&lock);
>       }

The code stays simpler if you simply move the unlock call out
of the if() statement, here.

>   
>       qemu_plugin_outs(report->str);

Regards,

Phil.
[PATCH v2] plugins/hotblocks: Fix potential deadlock in plugin_exit() function
Posted by Cong Liu 7 months, 1 week ago
This patch fixes a potential deadlock in the plugin_exit() function of QEMU.
The original code does not release the lock mutex if it is NULL. This patch
adds a check for it being NULL and releases the mutex in that case.

Signed-off-by: Cong Liu <liucong2@kylinos.cn>
Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 contrib/plugins/hotblocks.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c
index 6b74d25fead6..b99b93ad8dc7 100644
--- a/contrib/plugins/hotblocks.c
+++ b/contrib/plugins/hotblocks.c
@@ -69,9 +69,9 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
         }
 
         g_list_free(it);
-        g_mutex_unlock(&lock);
     }
 
+    g_mutex_unlock(&lock);
     qemu_plugin_outs(report->str);
 }
 
-- 
2.34.1


Re: [PATCH v2] plugins/hotblocks: Fix potential deadlock in plugin_exit() function
Posted by Philippe Mathieu-Daudé 7 months, 1 week ago
On 21/9/23 11:23, Cong Liu wrote:
> This patch fixes a potential deadlock in the plugin_exit() function of QEMU.
> The original code does not release the lock mutex if it is NULL. This patch
> adds a check for it being NULL and releases the mutex in that case.
> 
> Signed-off-by: Cong Liu <liucong2@kylinos.cn>
> Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Not really suggested, just reviewed ;)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> ---
>   contrib/plugins/hotblocks.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c
> index 6b74d25fead6..b99b93ad8dc7 100644
> --- a/contrib/plugins/hotblocks.c
> +++ b/contrib/plugins/hotblocks.c
> @@ -69,9 +69,9 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
>           }
>   
>           g_list_free(it);
> -        g_mutex_unlock(&lock);
>       }
>   
> +    g_mutex_unlock(&lock);
>       qemu_plugin_outs(report->str);
>   }
>