On Thu, May 15, 2025 at 07:20:12PM -0300, Fabiano Rosas wrote:
> tcp_chr_free_connection() can be called multiple times in succession,
> in which case the yank function will get as argument a NULL s->sioc
> that has been cleared by the previous tcp_chr_free_connection() call.
>
> This leads to an abort() at yank_unregister_function().
>
> #0 __GI_raise (sig=6) at ../sysdeps/unix/sysv/linux/raise.c:51
> #1 __GI_abort () at abort.c:79
> #2 qtest_check_status (s=0x513000005600) at ../tests/qtest/libqtest.c:209
> #3 qtest_wait_qemu (s=0x513000005600) at ../tests/qtest/libqtest.c:273
> #4 qtest_kill_qemu (s=0x513000005600) at ../tests/qtest/libqtest.c:285
> #5 kill_qemu_hook_func (s=0x513000005600) at ../tests/qtest/libqtest.c:294
> #6 g_hook_list_invoke (hook_list=0x55ea9cc750c0 <abrt_hooks>,
> may_recurse=0) at ../glib/ghook.c:534
> #7 sigabrt_handler (signo=6) at ../tests/qtest/libqtest.c:299
> #8 <signal handler called>
> #9 __GI_raise (sig=6) at ../sysdeps/unix/sysv/linux/raise.c:51
> #10 __GI_abort () at abort.c:79
> #11 yank_unregister_function (instance=0x7fb26f2ea9a0,
> func=0x55ea9bcc0a10 <char_socket_yank_iochannel>, opaque=0x0) at
> ../util/yank.c:151
> #12 tcp_chr_free_connection (chr=0x51300000ffc0) at
> ../chardev/char-socket.c:385
> #13 tcp_chr_disconnect_locked (chr=0x51300000ffc0) at
> ../chardev/char-socket.c:477
> #14 tcp_chr_disconnect (chr=0x51300000ffc0) at ../chardev/char-socket.c:495
> #15 tcp_chr_hup (channel=0x514000000040, cond=G_IO_HUP,
> opaque=0x51300000ffc0) at ../chardev/char-socket.c:536
> #16 qio_channel_fd_source_dispatch (source=0x50c0000b5fc0,
> callback=0x55ea9bcd6770 <tcp_chr_hup>,
> user_data=0x51300000ffc0) at ../io/channel-watch.c:84
> #17 g_main_dispatch (context=0x50f000000040) at ../glib/gmain.c:3381
> #18 g_main_context_dispatch (context=context@entry=0x50f000000040) at
> ../glib/gmain.c:4099
> #19 g_main_context_iterate (context=0x50f000000040, block=block@entry=1,
> dispatch=dispatch@entry=1,
> self=<optimized out>) at ../glib/gmain.c:4175
> #20 g_main_loop_run (loop=0x502000055690) at ../glib/gmain.c:4373
>
> Commit ebae6477dc ("chardev: check if the chardev is registered for
> yanking") seems to have encountered a similar issue, but checking
> s->registered_yank is not a complete solution because that flag
> pertains to the yank instance, not to each individual function.
>
> Skip the yank_unregister_function() in case s->sioc is already NULL,
> which indicates the last yank function was already removed.
>
> Signed-off-by: Fabiano Rosas <faro...@suse.de>
> ---
> Can tcp_chr_free_connection() race with itself? I'm assuming no.
>
> Could we just make yank_unregister_instance() remove all yank
> functions at once? Those asserts/abort in the yank code are a bit
> masochistic.
> ---
> chardev/char-socket.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berra...@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
