1. Patchew
  2. QEMU
  3. View series

[PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()

Markus Armbruster posted 1 patch 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20251013112102.2396012-1-armbru@redhat.com
Maintainers: "Marc-André Lureau" <marcandre.lureau@redhat.com>
ui/qemu-pixman.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
[PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()
Posted by Markus Armbruster 1 month ago 
Reported-by: Bernhard Beschow <shentey@gmail.com>
Fixes: b296b29d3414 (ui/pixman: Consistent error handling in qemu_pixman_shareable_free())
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 ui/qemu-pixman.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c
index e46c6232cf..aea09755b9 100644
--- a/ui/qemu-pixman.c
+++ b/ui/qemu-pixman.c
@@ -291,7 +291,9 @@ qemu_pixman_shareable_free(qemu_pixman_shareable handle,
     Error *err = NULL;
 
     qemu_win32_map_free(ptr, handle, &err);
-    error_report_err(err);
+    if (err) {
+        error_report_err(err);
+    }
 #else
     qemu_memfd_free(ptr, size, handle);
 #endif
-- 
2.49.0
Re: [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()
Posted by Bernhard Beschow 3 weeks, 4 days ago 

Am 13. Oktober 2025 11:21:02 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>Reported-by: Bernhard Beschow <shentey@gmail.com>
>Fixes: b296b29d3414 (ui/pixman: Consistent error handling in qemu_pixman_shareable_free())
>Signed-off-by: Markus Armbruster <armbru@redhat.com>
>---
> ui/qemu-pixman.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c
>index e46c6232cf..aea09755b9 100644
>--- a/ui/qemu-pixman.c
>+++ b/ui/qemu-pixman.c
>@@ -291,7 +291,9 @@ qemu_pixman_shareable_free(qemu_pixman_shareable handle,
>     Error *err = NULL;
> 
>     qemu_win32_map_free(ptr, handle, &err);
>-    error_report_err(err);
>+    if (err) {
>+        error_report_err(err);
>+    }
> #else
>     qemu_memfd_free(ptr, size, handle);
> #endif

Thanks for the quick fix, Markus! The patch is reviewed and I've been testing it for a week. Who is going to merge it?

Best regards,
Bernhard
Re: [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()
Posted by Markus Armbruster 3 weeks, 4 days ago 
Bernhard Beschow <shentey@gmail.com> writes:

> Am 13. Oktober 2025 11:21:02 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>>Reported-by: Bernhard Beschow <shentey@gmail.com>
>>Fixes: b296b29d3414 (ui/pixman: Consistent error handling in qemu_pixman_shareable_free())
>>Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>---
>> ui/qemu-pixman.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>>diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c
>>index e46c6232cf..aea09755b9 100644
>>--- a/ui/qemu-pixman.c
>>+++ b/ui/qemu-pixman.c
>>@@ -291,7 +291,9 @@ qemu_pixman_shareable_free(qemu_pixman_shareable handle,
>>     Error *err = NULL;
>> 
>>     qemu_win32_map_free(ptr, handle, &err);
>>-    error_report_err(err);
>>+    if (err) {
>>+        error_report_err(err);
>>+    }
>> #else
>>     qemu_memfd_free(ptr, size, handle);
>> #endif
>
> Thanks for the quick fix, Markus! The patch is reviewed and I've been testing it for a week. Who is going to merge it?

I can do it.

You provided

    Tested-by: Bernhard Beschow
    Reviewed-by: Bernhard Beschow

without an e-mail address.  Intentional?  If not, should I use
<shentey@gmail.com>?
Re: [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()
Posted by Bernhard Beschow 3 weeks, 3 days ago 

Am 20. Oktober 2025 12:32:33 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>Bernhard Beschow <shentey@gmail.com> writes:
>
>> Am 13. Oktober 2025 11:21:02 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>>>Reported-by: Bernhard Beschow <shentey@gmail.com>
>>>Fixes: b296b29d3414 (ui/pixman: Consistent error handling in qemu_pixman_shareable_free())
>>>Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>---
>>> ui/qemu-pixman.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>>diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c
>>>index e46c6232cf..aea09755b9 100644
>>>--- a/ui/qemu-pixman.c
>>>+++ b/ui/qemu-pixman.c
>>>@@ -291,7 +291,9 @@ qemu_pixman_shareable_free(qemu_pixman_shareable handle,
>>>     Error *err = NULL;
>>> 
>>>     qemu_win32_map_free(ptr, handle, &err);
>>>-    error_report_err(err);
>>>+    if (err) {
>>>+        error_report_err(err);
>>>+    }
>>> #else
>>>     qemu_memfd_free(ptr, size, handle);
>>> #endif
>>
>> Thanks for the quick fix, Markus! The patch is reviewed and I've been testing it for a week. Who is going to merge it?
>
>I can do it.
>
>You provided
>
>    Tested-by: Bernhard Beschow
>    Reviewed-by: Bernhard Beschow
>
>without an e-mail address.  Intentional?

Oh, that wasn't intentional.

> If not, should I use <shentey@gmail.com>?

Yes, please.

Best regards,
Bernhard
Re: [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()
Posted by Markus Armbruster 3 weeks, 3 days ago 
Bernhard Beschow <shentey@gmail.com> writes:

> Am 20. Oktober 2025 12:32:33 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>>Bernhard Beschow <shentey@gmail.com> writes:
>>
>>> Am 13. Oktober 2025 11:21:02 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>>>>Reported-by: Bernhard Beschow <shentey@gmail.com>
>>>>Fixes: b296b29d3414 (ui/pixman: Consistent error handling in qemu_pixman_shareable_free())
>>>>Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>>---
>>>> ui/qemu-pixman.c | 4 +++-
>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>>diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c
>>>>index e46c6232cf..aea09755b9 100644
>>>>--- a/ui/qemu-pixman.c
>>>>+++ b/ui/qemu-pixman.c
>>>>@@ -291,7 +291,9 @@ qemu_pixman_shareable_free(qemu_pixman_shareable handle,
>>>>     Error *err = NULL;
>>>> 
>>>>     qemu_win32_map_free(ptr, handle, &err);
>>>>-    error_report_err(err);
>>>>+    if (err) {
>>>>+        error_report_err(err);
>>>>+    }
>>>> #else
>>>>     qemu_memfd_free(ptr, size, handle);
>>>> #endif
>>>
>>> Thanks for the quick fix, Markus! The patch is reviewed and I've been testing it for a week. Who is going to merge it?
>>
>>I can do it.
>>
>>You provided
>>
>>    Tested-by: Bernhard Beschow
>>    Reviewed-by: Bernhard Beschow
>>
>>without an e-mail address.  Intentional?
>
> Oh, that wasn't intentional.
>
>> If not, should I use <shentey@gmail.com>?
>
> Yes, please.

Done, PR sent.  Thank you!
Re: [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()
Posted by Bernhard Beschow 3 weeks, 2 days ago 

Am 21. Oktober 2025 09:50:32 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>Bernhard Beschow <shentey@gmail.com> writes:
>
>> Am 20. Oktober 2025 12:32:33 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>>>Bernhard Beschow <shentey@gmail.com> writes:
>>>
>>>> Am 13. Oktober 2025 11:21:02 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>>>>>Reported-by: Bernhard Beschow <shentey@gmail.com>
>>>>>Fixes: b296b29d3414 (ui/pixman: Consistent error handling in qemu_pixman_shareable_free())
>>>>>Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>>>---
>>>>> ui/qemu-pixman.c | 4 +++-
>>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>>diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c
>>>>>index e46c6232cf..aea09755b9 100644
>>>>>--- a/ui/qemu-pixman.c
>>>>>+++ b/ui/qemu-pixman.c
>>>>>@@ -291,7 +291,9 @@ qemu_pixman_shareable_free(qemu_pixman_shareable handle,
>>>>>     Error *err = NULL;
>>>>> 
>>>>>     qemu_win32_map_free(ptr, handle, &err);
>>>>>-    error_report_err(err);
>>>>>+    if (err) {
>>>>>+        error_report_err(err);
>>>>>+    }
>>>>> #else
>>>>>     qemu_memfd_free(ptr, size, handle);
>>>>> #endif
>>>>
>>>> Thanks for the quick fix, Markus! The patch is reviewed and I've been testing it for a week. Who is going to merge it?
>>>
>>>I can do it.
>>>
>>>You provided
>>>
>>>    Tested-by: Bernhard Beschow
>>>    Reviewed-by: Bernhard Beschow
>>>
>>>without an e-mail address.  Intentional?
>>
>> Oh, that wasn't intentional.
>>
>>> If not, should I use <shentey@gmail.com>?
>>
>> Yes, please.
>
>Done, PR sent.  Thank you!

Nice. Thanks!
Re: [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()
Posted by Philippe Mathieu-Daudé 1 month ago 
On 13/10/25 13:21, Markus Armbruster wrote:
> Reported-by: Bernhard Beschow <shentey@gmail.com>
> Fixes: b296b29d3414 (ui/pixman: Consistent error handling in qemu_pixman_shareable_free())
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   ui/qemu-pixman.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c
> index e46c6232cf..aea09755b9 100644
> --- a/ui/qemu-pixman.c
> +++ b/ui/qemu-pixman.c
> @@ -291,7 +291,9 @@ qemu_pixman_shareable_free(qemu_pixman_shareable handle,
>       Error *err = NULL;
>   
>       qemu_win32_map_free(ptr, handle, &err);

Slightly better if qemu_win32_map_free(), as per commit e3fe3988d78
("error: Document Error API usage rules") recommended style:

+ *  We recommend
+ *   • bool-valued functions return true on success / false on failure,
+ *   • pointer-valued functions return non-null / null pointer, and
+ *   • integer-valued functions return non-negative / negative.

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

> -    error_report_err(err);
> +    if (err) {
> +        error_report_err(err);
> +    }
>   #else
>       qemu_memfd_free(ptr, size, handle);
>   #endif
Re: [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()
Posted by Markus Armbruster 1 month ago 
Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 13/10/25 13:21, Markus Armbruster wrote:
>> Reported-by: Bernhard Beschow <shentey@gmail.com>
>> Fixes: b296b29d3414 (ui/pixman: Consistent error handling in qemu_pixman_shareable_free())
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   ui/qemu-pixman.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c
>> index e46c6232cf..aea09755b9 100644
>> --- a/ui/qemu-pixman.c
>> +++ b/ui/qemu-pixman.c
>> @@ -291,7 +291,9 @@ qemu_pixman_shareable_free(qemu_pixman_shareable handle,
>>       Error *err = NULL;
>>   
>>       qemu_win32_map_free(ptr, handle, &err);
>
> Slightly better if qemu_win32_map_free(), as per commit e3fe3988d78
> ("error: Document Error API usage rules") recommended style:
>
> + *  We recommend
> + *   • bool-valued functions return true on success / false on failure,
> + *   • pointer-valued functions return non-null / null pointer, and
> + *   • integer-valued functions return non-negative / negative.

I went fo a minimal fix.

I now see that this is the only use of qemu_win32_map_free().  Changing
it to return bool would've been okay.

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

Thanks!

>> -    error_report_err(err);
>> +    if (err) {
>> +        error_report_err(err);
>> +    }
>>   #else
>>       qemu_memfd_free(ptr, size, handle);
>>   #endif
Re: [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()
Posted by Bernhard Beschow 1 month ago 

Am 13. Oktober 2025 14:11:32 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 13/10/25 13:21, Markus Armbruster wrote:
>> Reported-by: Bernhard Beschow <shentey@gmail.com>
>> Fixes: b296b29d3414 (ui/pixman: Consistent error handling in qemu_pixman_shareable_free())
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   ui/qemu-pixman.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c
>> index e46c6232cf..aea09755b9 100644
>> --- a/ui/qemu-pixman.c
>> +++ b/ui/qemu-pixman.c
>> @@ -291,7 +291,9 @@ qemu_pixman_shareable_free(qemu_pixman_shareable handle,
>>       Error *err = NULL;
>>         qemu_win32_map_free(ptr, handle, &err);
>
>Slightly better if qemu_win32_map_free(), as per commit e3fe3988d78
>("error: Document Error API usage rules") recommended style:
>
>+ *  We recommend
>+ *   • bool-valued functions return true on success / false on failure,
>+ *   • pointer-valued functions return non-null / null pointer, and
>+ *   • integer-valued functions return non-negative / negative.

Now I see: Since qemu_win32_map_free() returns void, the pattern is to check for the error pointer. Could be mentioned in the commit message for educational porposes.

Anyhow,
Tested-by: Bernhard Beschow
Reviewed-by: Bernhard Beschow

>
>Anyhow,
>Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
>> -    error_report_err(err);
>> +    if (err) {
>> +        error_report_err(err);
>> +    }
>>   #else
>>       qemu_memfd_free(ptr, size, handle);
>>   #endif
>
Re: [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()
Posted by Daniel P. Berrangé 1 month ago 
On Mon, Oct 13, 2025 at 01:21:02PM +0200, Markus Armbruster wrote:
> Reported-by: Bernhard Beschow <shentey@gmail.com>
> Fixes: b296b29d3414 (ui/pixman: Consistent error handling in qemu_pixman_shareable_free())
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  ui/qemu-pixman.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.