serial: clear transmit retry callback on unrealize

[PATCH] serial: clear transmit retry callback on unrealize

Posted by Paolo Bonzini 1 month, 1 week ago

The GSource is removed when resetting but remains active (and can
cause use-after-free) on hot-unplug.  Remove it before the character
device is disconnected.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4125
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/char/serial.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/char/serial.c b/hw/char/serial.c
index 0729cd2ce9d..4339562ab00 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -936,6 +936,7 @@ static void serial_unrealize(DeviceState *dev)
 {
     SerialState *s = SERIAL(dev);
 
+    g_clear_handle_id(&s->watch_tag, g_source_remove);
     qemu_chr_fe_deinit(&s->chr, false);
 
     timer_free(s->modem_status_poll);
-- 
2.55.0

Re: [PATCH] serial: clear transmit retry callback on unrealize

Posted by Michael Tokarev 1 month ago

On 8/18/26 17:28, Paolo Bonzini wrote:
> The GSource is removed when resetting but remains active (and can
> cause use-after-free) on hot-unplug.  Remove it before the character
> device is disconnected.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4125
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

I'm picking this up for qemu-stable.
Please let me know if I shouldn't.

Thanks,

/mjt

> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index 0729cd2ce9d..4339562ab00 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -936,6 +936,7 @@ static void serial_unrealize(DeviceState *dev)
>   {
>       SerialState *s = SERIAL(dev);
>   
> +    g_clear_handle_id(&s->watch_tag, g_source_remove);
>       qemu_chr_fe_deinit(&s->chr, false);
>   
>       timer_free(s->modem_status_poll);

Re: [PATCH] serial: clear transmit retry callback on unrealize

Posted by Marc-André Lureau 1 month, 1 week ago

Hi

On Tue, Aug 18, 2026 at 6:29 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The GSource is removed when resetting but remains active (and can
> cause use-after-free) on hot-unplug.  Remove it before the character
> device is disconnected.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4125
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

it fixes a use-after-free, so
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

But should we care about flushing pending data?

> ---
>  hw/char/serial.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index 0729cd2ce9d..4339562ab00 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -936,6 +936,7 @@ static void serial_unrealize(DeviceState *dev)
>  {
>      SerialState *s = SERIAL(dev);
>
> +    g_clear_handle_id(&s->watch_tag, g_source_remove);
>      qemu_chr_fe_deinit(&s->chr, false);
>
>      timer_free(s->modem_status_poll);
> --
> 2.55.0
>
>


-- 
Marc-André Lureau

Re: [PATCH] serial: clear transmit retry callback on unrealize

Posted by Paolo Bonzini 1 month, 1 week ago

Il mar 18 ago 2026, 16:42 Marc-André Lureau <marcandre.lureau@gmail.com> ha
scritto:

> Hi
>
> On Tue, Aug 18, 2026 at 6:29 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > The GSource is removed when resetting but remains active (and can
> > cause use-after-free) on hot-unplug.  Remove it before the character
> > device is disconnected.
> >
> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4125
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>
> it fixes a use-after-free, so
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> But should we care about flushing pending data?
>

To where, :) since the transmit channel was not ready?

Paolo


> > ---
> >  hw/char/serial.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/hw/char/serial.c b/hw/char/serial.c
> > index 0729cd2ce9d..4339562ab00 100644
> > --- a/hw/char/serial.c
> > +++ b/hw/char/serial.c
> > @@ -936,6 +936,7 @@ static void serial_unrealize(DeviceState *dev)
> >  {
> >      SerialState *s = SERIAL(dev);
> >
> > +    g_clear_handle_id(&s->watch_tag, g_source_remove);
> >      qemu_chr_fe_deinit(&s->chr, false);
> >
> >      timer_free(s->modem_status_poll);
> > --
> > 2.55.0
> >
> >
>
>
> --
> Marc-André Lureau
>
>