Hi
On Thu, Sep 16, 2021 at 11:29 PM Volker Rümelin <vr_q...@t-online.de> wrote:
> Make chr in the QemuConsole object a strong reference to the
> referenced chardev device. This prevents a use after free error
> if the chardev device goes away unexpectedly.
>
> To reproduce the error start qemu-system built with address
> sanitizer with the the following command line options.
>
> -display sdl -chardev vc,id=test0,cols=132,rows=50.
>
> Open the monitor console with CTRL-ALT-3 and remove the
> unconnected chardev device test0.
>
> (qemu) chardev-remove test0
>
> Open the text console test0 with CTRL-ALT-2 and type a character.
> QEMU immediately exits with this error message.
>
Hmm, why is it not busy like the other chardevs?
> ==28148==ERROR: AddressSanitizer: heap-use-after-free
> on address 0x60e000043778 at pc 0x558712ba7125
> bp 0x7fff270980b0 sp 0x7fff270980a8
> READ of size 8 at 0x60e000043778 thread T0
> #0 0x558712ba7124 in qemu_chr_be_can_write
> ../qemu-master/chardev/char.c:188
> #1 0x558711624770 in kbd_send_chars
> ../qemu-master/ui/console.c:1113
> #2 0x558711634e91 in kbd_put_keysym_console
> ../qemu-master/ui/console.c:1175
> #3 0x55871163532a in kbd_put_string_console
> ../qemu-master/ui/console.c:1221
> #4 0x5587120a21e4 in handle_textinput
> ../qemu-master/ui/sdl2.c:464
> #5 0x5587120a21e4 in sdl2_poll_events
> ../qemu-master/ui/sdl2.c:650
> #6 0x5587116269c3 in dpy_refresh
> ../qemu-master/ui/console.c:1673
> #7 0x5587116269c3 in gui_update
> ../qemu-master/ui/console.c:158
> #8 0x558712d3a919 in timerlist_run_timers
> ../qemu-master/util/qemu-timer.c:573
> #9 0x558712d3b183 in qemu_clock_run_timers
> ../qemu-master/util/qemu-timer.c:587
> #10 0x558712d3b183 in qemu_clock_run_all_timers
> ../qemu-master/util/qemu-timer.c:669
> #11 0x558712d286d9 in main_loop_wait
> ../qemu-master/util/main-loop.c:542
> #12 0x5587123d313b in qemu_main_loop
> ../qemu-master/softmmu/runstate.c:726
> #13 0x5587115f989d in main
> ../qemu-master/softmmu/main.c:50
> #14 0x7f832ee0934c in __libc_start_main
> (/lib64/libc.so.6+0x2534c)
> #15 0x55871160b6e9 in _start
> (/home/ruemelin/rpmbuild/BUILD/qemu-6.1.50-build/
> qemu-system-x86_64+0x1f4f6e9)
>
> Signed-off-by: Volker Rümelin <vr_q...@t-online.de>
> ---
> ui/console.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/ui/console.c b/ui/console.c
> index 29a3e3f0f5..1ef5a96295 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -2264,6 +2264,7 @@ static void vc_chr_open(Chardev *chr,
> }
>
> s->chr = chr;
> + object_ref(chr);
> drv->console = s;
>
> if (display_state) {
>
Isn't this effectively preventing removing the chardev? Hence, it should
either be made busy, or keeping a weak pointer invalidated on
instance_finalize imho.
thanks
--
Marc-André Lureau
