[Qemu-devel] qemu-char issues (was Re: [PULL 00/15] vhost, pc: fixes for

[Qemu-devel] qemu-char issues (was Re: [PULL 00/15] vhost, pc: fixes for

[Michael S. Tsirkin]

2.5)
Message-ID: <20151126190029-mutt-send-email-mst@redhat.com>
Reply-To: 
In-Reply-To: <20151126182218-mutt-send-email-mst@redhat.com>

On Thu, Nov 26, 2015 at 06:24:12PM +0200, Michael S. Tsirkin wrote:
> On Thu, Nov 26, 2015 at 06:19:46PM +0200, Michael S. Tsirkin wrote:
> > On Thu, Nov 26, 2015 at 11:26:10AM +0000, Peter Maydell wrote:
> > > On 19 November 2015 at 13:35, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > The following changes since commit 8337c6cbc37c6b2184f41bab3eaff47d5e68012a:
> > > >
> > > >   Update version for v2.5.0-rc0 release (2015-11-13 17:10:36 +0000)
> > > >
> > > > are available in the git repository at:
> > > >
> > > >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> > > >
> > > > for you to fetch changes up to 1c7ba94a184df1eddd589d5400d879568d3e5d08:
> > > >
> > > >   exec: silence hugetlbfs warning under qtest (2015-11-19 15:26:05 +0200)
> > > >
> > > > ----------------------------------------------------------------
> > > > vhost, pc: fixes for 2.5
> > > >
> > > > Fixes all over the place.
> > > >
> > > > This also re-enables a test we disabled in 2.5 cycle
> > > > now that there's a way not to get a warning from it.
> > > >
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > 
> > > Hi; I've just noticed that since this pull was applied the Travis
> > > builds have been failing:
> > > https://travis-ci.org/qemu/qemu/builds
> > > 
> > > The log messages are rather odd but suggest a virtio-user problem:
> > 
> > So far, it looks like I found a bunch of qemu-char (or possibly glib?)
> > problems.
> > This is on Fedora 23.
> > How to reproduce:
> > 
> > First, apply this patch:
> > 
> > 	vhost-user-test: fix migration overlap test
> > 
> > Now
> > 
> > [mst@robin qemu]$ make -j 16
> >   CC    qemu-char.o
> >   LINK  x86_64-softmmu/qemu-system-x86_64
> >   LINK  i386-softmmu/qemu-system-i386
> > [mst@robin qemu]$ make tests/vhost-user-test
> >   CC    tests/vhost-user-test.o
> >   LINK  tests/vhost-user-test
> > 
> > 
> > Run under valgrind:
> > 	QTEST_QEMU_BINARY=./x86_64-softmmu/qemu-system-x86_64 valgrind tests/vhost-user-test
> > 
> > What seems to happen is that after remove_fd_in_watch, read callback
> > is still invoked. read fails so it calls close, and close
> > causes use after free.
> > 
> > Help would be appreciated.
> 
> Here's the log:
> http://paste.fedoraproject.org/294863/55491614
> 
> As you see tcp_chr_close freed a bunch of
> stuff, and now tcp_chr_read attempts to use it.

Last note:
https://developer.gnome.org/glib/stable/glib-The-Main-Event-Loop.html#mainloop-memory-management
suggests two ways to manage memory for sources.
We seem to do neither, opting for a different approach.

> > -- 
> > MST

Re: [Qemu-devel] qemu-char issues (was Re: [PULL 00/15] vhost, pc: fixes for

[Peter Maydell]

On 26 November 2015 at 17:01, Michael S. Tsirkin <mst@redhat.com> wrote:
> Last note:
> https://developer.gnome.org/glib/stable/glib-The-Main-Event-Loop.html#mainloop-memory-management
> suggests two ways to manage memory for sources.
> We seem to do neither, opting for a different approach.

We seem to pretty much be using what that doc calls the "preferred
approach": we record the tag we get back from g_source_attach
(this happens in io_add_watch_poll()).  Then on object destruction
we call remove_fd_in_watch() with that tag, which calls
io_remove_watch_poll(), which does a g_source_destroy(). And
it looks like g_source_destroy() is the "works on any GMainContext"
version of g_source_remove().

So I guess the question is: when this bug happens, have we
called g_source_destroy() before we got the callback from
glib, or is the problem that we didn't actually call
g_source_destroy() on everything we should have as part
of the destructor?

It's getting a bit late here now but I can have a look at
this tomorrow if nobody else gets there first.

thanks
-- PMM