All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: "Marc-André Lureau" <marcandre.lureau@gmail.com>
Cc: Hogan Wang <hogan.wang@huawei.com>,
	Markus Armbruster <armbru@redhat.com>,
	QEMU <qemu-devel@nongnu.org>, Paolo Bonzini <pbonzini@redhat.com>,
	wangxinxin.wang@huawei.com
Subject: Re: [PATCH v2] chardev: avoid use-after-free when client disconnect
Date: Wed, 20 Jul 2022 09:19:55 +0100	[thread overview]
Message-ID: <Yte6qzbrZk6jwXpw@redhat.com> (raw)
In-Reply-To: <CAJ+F1CL6ppZ_J_HK4-hHQG21=cerzBmArL7tkUcy1eYpMLcYUA@mail.gmail.com>

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 mian thread, then io_watch_poll_prepare access @ioc or
> >
> 
> mian->main
> 
> 
> > @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 

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 :|



  reply	other threads:[~2022-07-20  8:21 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-20  7:10 [PATCH v2] chardev: avoid use-after-free when client disconnect Hogan Wang via
2022-07-20  7:36 ` Marc-André Lureau
2022-07-20  8:19   ` Daniel P. Berrangé [this message]
  -- strict thread matches above, loose matches on Subject: below --
2022-07-20  8:55 Wangjing(Hogan) via
2022-07-20  9:00 ` Daniel P. Berrangé
2022-07-20 10:07 Wangjing(Hogan) via

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Yte6qzbrZk6jwXpw@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=hogan.wang@huawei.com \
    --cc=marcandre.lureau@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=wangxinxin.wang@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.