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

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

[Hongyan Xu]

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.

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

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

[Samuel Thibault]

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
> 

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

[Samuel Thibault]

From: Hongyan Xu <getshell@seu.edu.cn>

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.

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>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Message-ID: <20260904033055.413-1-getshell@seu.edu.cn>
---
 chardev/baum.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/chardev/baum.c b/chardev/baum.c
index ac1e535ba8..166d6f2f09 100644
--- a/chardev/baum.c
+++ b/chardev/baum.c
@@ -659,6 +659,7 @@ static void baum_chr_read(void *opaque)
     }
     if (ret == -1 && (brlapi_errno != BRLAPI_ERROR_LIBCERR || errno != EINTR)) {
         brlapi_perror("baum: brlapi_readKey");
+        qemu_set_fd_handler(baum->brlapi_fd, NULL, NULL, NULL);
         brlapi__closeConnection(baum->brlapi);
         g_free(baum->brlapi);
         baum->brlapi = NULL;
@@ -671,8 +672,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.53.0