From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:54528)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mlureau@redhat.com>) id 1bkY8R-0003KZ-Ru
	for qemu-devel@nongnu.org; Thu, 15 Sep 2016 10:59:33 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <mlureau@redhat.com>) id 1bkY8N-00064v-Sv
	for qemu-devel@nongnu.org; Thu, 15 Sep 2016 10:59:31 -0400
Received: from mx4-phx2.redhat.com ([209.132.183.25]:58351)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mlureau@redhat.com>) id 1bkY8N-00064Y-JM
	for qemu-devel@nongnu.org; Thu, 15 Sep 2016 10:59:27 -0400
Date: Thu, 15 Sep 2016 10:59:25 -0400 (EDT)
From: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau <mlureau@redhat.com>
Message-ID: <683170734.858859.1473951565736.JavaMail.zimbra@redhat.com>
In-Reply-To: <20160915143158.4796-1-lma@suse.com>
References: <20160915143158.4796-1-lma@suse.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH] msmouse: Fix segfault caused by free the
 chr before chardev cleanup.
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Lin Ma <lma@suse.com>
Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, marcandre lureau <marcandre.lureau@redhat.com>

Hi

----- Original Message -----
> Segfault happens when leaving qemu with msmouse backend:
>=20
>  #0  0x00007fa8526ac975 in raise () at /lib64/libc.so.6
>  #1  0x00007fa8526add8a in abort () at /lib64/libc.so.6
>  #2  0x0000558be78846ab in error_exit (err=3D16, msg=3D0x558be799da10 ...
>  #3  0x0000558be7884717 in qemu_mutex_destroy (mutex=3D0x558be93be750) at=
 ...
>  #4  0x0000558be7549951 in qemu_chr_free_common (chr=3D0x558be93be750) at=
 ...
>  #5  0x0000558be754999c in qemu_chr_free (chr=3D0x558be93be750) at ...
>  #6  0x0000558be7549a20 in qemu_chr_delete (chr=3D0x558be93be750) at ...
>  #7  0x0000558be754a8ef in qemu_chr_cleanup () at qemu-char.c:4643
>  #8  0x0000558be755843e in main (argc=3D5, argv=3D0x7ffe925d7118, ...
>=20
> The chr was freed by msmouse close callback before chardev cleanup,
> Then qemu_mutex_destroy triggered raise().
>=20
> Because freeing chr is handled by qemu_chr_free_common, Remove the free f=
rom
> msmouse_chr_close to avoid double free.
>=20
> Signed-off-by: Lin Ma <lma@suse.com>

Reviewed-by: Marc-Andr=C3=A9 Lureau <marcandre.lureau@redhat.com>

> ---
>  backends/msmouse.c | 1 -
>  1 file changed, 1 deletion(-)
>=20
> diff --git a/backends/msmouse.c b/backends/msmouse.c
> index aeb9055..7690c42 100644
> --- a/backends/msmouse.c
> +++ b/backends/msmouse.c
> @@ -139,7 +139,6 @@ static void msmouse_chr_close (struct CharDriverState
> *chr)
> =20
>      qemu_input_handler_unregister(mouse->hs);
>      g_free(mouse);
> -    g_free(chr);
>  }

Ooch, chr_close is not supposed to free chr! There might be other cases whe=
re memory corruption could occur.
=20
> =20
>  static QemuInputHandler msmouse_handler =3D {
> --
> 2.9.2
>=20
>=20