> On Wed, Jul 20, 2022 at 08:55:46AM +0000, Wangjing(Hogan) wrote:
> >
> > > On Wed, Jul 20, 2022 at 11:36:07AM +0400, Marc-André Lureau wrote:
> > > > Hi
> > > >
> > > > On Wed, Jul 20, 2022 at 11:13 AM Hogan Wang via
> > > > <qemu-devel@nongnu.org>
> > > > wrote:
> > > >
> > > > > IOWatchPoll object did not hold the @ioc and @src objects
> > > > > reference, then io_watch_poll_prepare execute in IO thread, if
> > > > > IOWatchPoll removed by main thread, then io_watch_poll_prepare
> > > > > access @ioc or @src concurrently lead to coredump.
> > > > >
> > > > > In IO thread monitor scene, the IO thread used to accept client,
> > > > > receive qmp request and handle hung-up event. Main thread used
> > > > > to handle qmp request and send response, it will remove
> > > > > IOWatchPoll and free @ioc when send response fail, then cause
> > > > > use-after-free
> > > > >
> > > >
> > > > I wonder if we are misusing GSources in that case, by removing
> > > > sources from different threads.. Could you be more specific about
> > > > the code path that leads to that?
> > >
> > > It is permitted, but unfortunately every version of glib prior to 2.64
> > > has a race condition that means you'll periodically get a use-after-free
> > > and a crash:
> > >
> > > https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1358
> > >
> > > Libvirt worked around this problem by not calling 'g_source_unref'
> > > directly, but instead have a helper that uses g_idle_add to delay the
> > > unref such that its guaranteed to happen inside the main event loop
> > > thread.
> > >
> > > So I'd like to know what version of glib Hogan is using
> >
> > I am using glib2-2.62.5 in test environment, so it's looks like a glib2
> > known issue.
>
> Hmm, actually the fix should have been backported into the 2.62.5 release
> according to this
>
> https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1361
According to the current source code:
Client connect to the monitor server, chr->gsource(IOWatchPoll) will be
initialized by io_add_watch_poll.
According to io_watch_poll_prepare(Execute in IO Thread) function:
IOWatchPoll->src will be added to chr->gsource as a child source while the
channel readable, but IOWatchPoll not hold the child source's reference because
of g_source_unref(iwp->src), so left the only one reference hold by the child
source list.
If client disconnect,then main thread will destroy the IOWatchPoll and unref
all the child source by g_source_destroy.
Call trace:
tcp_chr_read
tcp_chr_disconnect
tcp_chr_disconnect_locked
tcp_chr_free_connection
remove_fd_in_watch
g_source_destroy(chr->gsource) /* unref all the child
source */
object_unref(OBJECT(s->ioc)); /* QIOChannel freed and
IOWatchPoll->ioc is a wild pointer */
s->ioc = NULL;
At this time, IOWatchPoll->src and IOWatchPoll->ioc freed,
io_watch_poll_prepare called frequently and low-probability use-after-free.
