Re: [PATCH] chardev/baum: unregister the brlapi fd handler on finalize

[Samuel Thibault <samuel.thibault@gnu.org>]

Hongyan Xu, le ven. 04 sept. 2026 11:30:55 +0800, a ecrit:
> baum_chr_open() registers baum->brlapi_fd with the main loop through a
> raw qemu_set_fd_handler() call.  The chardev base class does not know
> about this handler, and char_braille_finalize() only closes the brlapi
> connection and frees the handle; it never removes the fd handler.

It should, indeed, thanks for noticing it.

The other brlapi__closeConnection call at the end of baum_chr_read
should be doing it as well, could you add it there as well?

Thanks,
Samuel

> When the chardev is removed at runtime (QMP chardev-remove /
> object_unparent), BaumChardev is finalized and freed while the main loop
> still holds an fd handler whose opaque points to the freed object.  The
> next time brlapi_fd becomes readable (or the connection drops) the loop
> calls baum_chr_read() with a dangling opaque, dereferencing freed
> memory -> host use-after-free.
> 
> Unregister the handler in char_braille_finalize() before tearing the
> connection down, using the same descriptor that baum_chr_open()
> registered, and NULL the handle afterwards as a belt-and-braces guard.
> Only do so while baum->brlapi is valid: baum_chr_open() sets
> baum->brlapi_fd from brlapi__openConnection(), which is
> BRLAPI_INVALID_FILE_DESCRIPTOR on failure, and that error path already
> g_free()s the handle and returns before any handler is installed.
> 
> Signed-off-by: Hongyan Xu <getshell@seu.edu.cn>
> ---
>  chardev/baum.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/chardev/baum.c b/chardev/baum.c
> index ac1e535ba8..6d9df70c7e 100644
> --- a/chardev/baum.c
> +++ b/chardev/baum.c
> @@ -671,8 +671,16 @@ static void char_braille_finalize(Object *obj)
>  
>      timer_free(baum->cellCount_timer);
>      if (baum->brlapi) {
> +        /*
> +         * baum_chr_open() registered brlapi_fd with the main loop via
> +         * qemu_set_fd_handler(); unregister it before tearing the
> +         * connection down so a later chardev-remove cannot dispatch
> +         * baum_chr_read() with a dangling opaque.
> +         */
> +        qemu_set_fd_handler(baum->brlapi_fd, NULL, NULL, NULL);
>          brlapi__closeConnection(baum->brlapi);
>          g_free(baum->brlapi);
> +        baum->brlapi = NULL;
>      }
>  }
>  
> -- 
> 2.50.1.windows.1
>