chardev/wctable: don't free the instance in wctablet_chr_finalize

[PATCH v1] chardev/wctable: don't free the instance in wctablet_chr_finalize

Posted by Daniil Tatianin 2 years, 5 months ago

Object is supposed to be freed by invoking obj->free, and not
obj->instance_finalize. This would lead to use-after-free followed by
double free in object_unref/object_finalize.

Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
---
 chardev/wctablet.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/chardev/wctablet.c b/chardev/wctablet.c
index e9cb7ca710..fa3c9be04e 100644
--- a/chardev/wctablet.c
+++ b/chardev/wctablet.c
@@ -318,7 +318,6 @@ static void wctablet_chr_finalize(Object *obj)
     TabletChardev *tablet = WCTABLET_CHARDEV(obj);
 
     qemu_input_handler_unregister(tablet->hs);
-    g_free(tablet);
 }
 
 static void wctablet_chr_open(Chardev *chr,
-- 
2.25.1

Re: [PATCH v1] chardev/wctable: don't free the instance in wctablet_chr_finalize

Posted by Marc-André Lureau 2 years, 5 months ago

Hi

On Wed, Nov 17, 2021 at 6:25 PM Daniil Tatianin
<d-tatianin@yandex-team.ru> wrote:
>
> Object is supposed to be freed by invoking obj->free, and not
> obj->instance_finalize. This would lead to use-after-free followed by
> double free in object_unref/object_finalize.
>
> Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>

Fixes: 378af96155d62 ("Add wctablet device")

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  chardev/wctablet.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/chardev/wctablet.c b/chardev/wctablet.c
> index e9cb7ca710..fa3c9be04e 100644
> --- a/chardev/wctablet.c
> +++ b/chardev/wctablet.c
> @@ -318,7 +318,6 @@ static void wctablet_chr_finalize(Object *obj)
>      TabletChardev *tablet = WCTABLET_CHARDEV(obj);
>
>      qemu_input_handler_unregister(tablet->hs);
> -    g_free(tablet);
>  }
>
>  static void wctablet_chr_open(Chardev *chr,
> --
> 2.25.1
>

Re: [PATCH v1] chardev/wctable: don't free the instance in wctablet_chr_finalize

Posted by Paolo Bonzini 2 years, 5 months ago

On 11/17/21 15:51, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Nov 17, 2021 at 6:25 PM Daniil Tatianin
> <d-tatianin@yandex-team.ru> wrote:
>>
>> Object is supposed to be freed by invoking obj->free, and not
>> obj->instance_finalize. This would lead to use-after-free followed by
>> double free in object_unref/object_finalize.
>>
>> Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
> 
> Fixes: 378af96155d62 ("Add wctablet device")
> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
>> ---
>>   chardev/wctablet.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/chardev/wctablet.c b/chardev/wctablet.c
>> index e9cb7ca710..fa3c9be04e 100644
>> --- a/chardev/wctablet.c
>> +++ b/chardev/wctablet.c
>> @@ -318,7 +318,6 @@ static void wctablet_chr_finalize(Object *obj)
>>       TabletChardev *tablet = WCTABLET_CHARDEV(obj);
>>
>>       qemu_input_handler_unregister(tablet->hs);
>> -    g_free(tablet);
>>   }
>>
>>   static void wctablet_chr_open(Chardev *chr,
>> --
>> 2.25.1
>>
> 
> 

Queued, thanks.

Paolo