Skip to contentSkip to conversation

msmouse: Fix segfault caused by free the chr before chardev cleanup.

PATCH Applied Lin Ma 3 participants 3 messages Sep 15, 2016

lore
Share & tools
lore

Lin Ma

author
#0 lore raw

PATCH 1/1 msmouse: Fix segfault caused by free the chr before chardev cleanup.

Segfault happens when leaving qemu with msmouse backend:

 #0  0x00007fa8526ac975 in raise () at /lib64/libc.so.6
 #1  0x00007fa8526add8a in abort () at /lib64/libc.so.6
 #2  0x0000558be78846ab in error_exit (err=16, msg=0x558be799da10 ...
 #3  0x0000558be7884717 in qemu_mutex_destroy (mutex=0x558be93be750) at ...
 #4  0x0000558be7549951 in qemu_chr_free_common (chr=0x558be93be750) at ...
 #5  0x0000558be754999c in qemu_chr_free (chr=0x558be93be750) at ...
 #6  0x0000558be7549a20 in qemu_chr_delete (chr=0x558be93be750) at ...
 #7  0x0000558be754a8ef in qemu_chr_cleanup () at qemu-char.c:4643
 #8  0x0000558be755843e in main (argc=5, argv=0x7ffe925d7118, ...

The chr was freed by msmouse close callback before chardev cleanup, Then qemu_mutex_destroy triggered raise().

Because freeing chr is handled by qemu_chr_free_common, Remove the free from msmouse_chr_close to avoid double free.

Signed-off-by Lin Ma <[email protected]>
---
 backends/msmouse.c | 1 -
 1 file changed, 1 deletion(-)
backends/msmouse.c +0 -1
index aeb9055..7690c42 100644
@@ -139,7 +139,6 @@ static void msmouse_chr_close (struct CharDriverState *chr)
 
     qemu_input_handler_unregister(mouse->hs);
     g_free(mouse);
-    g_free(chr);
 }
 
 static QemuInputHandler msmouse_handler = {
Signature · 1 line
2.9.2
Reviewed-by on v1 1/1
#1 lore raw

Hi

----- Original Message -----

Quoted 12 lines

Segfault happens when leaving qemu with msmouse backend:

 #0  0x00007fa8526ac975 in raise () at /lib64/libc.so.6
 #1  0x00007fa8526add8a in abort () at /lib64/libc.so.6
 #2  0x0000558be78846ab in error_exit (err=16, msg=0x558be799da10 ...
 #3  0x0000558be7884717 in qemu_mutex_destroy (mutex=0x558be93be750) at ...
 #4  0x0000558be7549951 in qemu_chr_free_common (chr=0x558be93be750) at ...
 #5  0x0000558be754999c in qemu_chr_free (chr=0x558be93be750) at ...
 #6  0x0000558be7549a20 in qemu_chr_delete (chr=0x558be93be750) at ...
 #7  0x0000558be754a8ef in qemu_chr_cleanup () at qemu-char.c:4643
 #8  0x0000558be755843e in main (argc=5, argv=0x7ffe925d7118, ...
Quoted 7 lines

The chr was freed by msmouse close callback before chardev cleanup, Then qemu_mutex_destroy triggered raise().

Because freeing chr is handled by qemu_chr_free_common, Remove the free from msmouse_chr_close to avoid double free.

Signed-off-by Lin Ma <[email protected]>
Reviewed-by Marc-André Lureau <[email protected]>
Quoted 9 lines
---
 backends/msmouse.c | 1 -
 1 file changed, 1 deletion(-)
backends/msmouse.c +0 -0
index aeb9055..7690c42 100644

backends/msmouse.c static void msmouse_chr_close (struct CharDriverState · line 143

Quoted 6 lines
backends/msmouse.c +0 -1
@@ -139,7 +139,6 @@ static void msmouse_chr_close (struct CharDriverState
 *chr)
 
     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 where memory corruption could occur.

Quoted 6 lines
backends/msmouse.c +0 -1
 static QemuInputHandler msmouse_handler = {
--

2.9.2

Applied
#2 lore raw
In reply to Lin Ma
Quoted 29 lines · Lin Ma

Segfault happens when leaving qemu with msmouse backend:

 #0  0x00007fa8526ac975 in raise () at /lib64/libc.so.6
 #1  0x00007fa8526add8a in abort () at /lib64/libc.so.6
 #2  0x0000558be78846ab in error_exit (err=16, msg=0x558be799da10 ...
 #3  0x0000558be7884717 in qemu_mutex_destroy (mutex=0x558be93be750) at ...
 #4  0x0000558be7549951 in qemu_chr_free_common (chr=0x558be93be750) at ...
 #5  0x0000558be754999c in qemu_chr_free (chr=0x558be93be750) at ...
 #6  0x0000558be7549a20 in qemu_chr_delete (chr=0x558be93be750) at ...
 #7  0x0000558be754a8ef in qemu_chr_cleanup () at qemu-char.c:4643
 #8  0x0000558be755843e in main (argc=5, argv=0x7ffe925d7118, ...

The chr was freed by msmouse close callback before chardev cleanup, Then qemu_mutex_destroy triggered raise().

Because freeing chr is handled by qemu_chr_free_common, Remove the free from msmouse_chr_close to avoid double free.

Signed-off-by Lin Ma <[email protected]>
---
 backends/msmouse.c | 1 -
 1 file changed, 1 deletion(-)
backends/msmouse.c +0 -0
index aeb9055..7690c42 100644

backends/msmouse.c static void msmouse_chr_close (struct CharDriverState *chr) · line 144

Quoted 7 lines
backends/msmouse.c +0 -1
@@ -139,7 +139,6 @@ static void msmouse_chr_close (struct CharDriverState *chr)
 
     qemu_input_handler_unregister(mouse->hs);
     g_free(mouse);
-    g_free(chr);
 }
 
 static QemuInputHandler msmouse_handler = {
Fixes c1111a24a3358ecd2f17be7c8b117cfe8bc5e5f8
Cc [email protected]

Queued for 2.8, thanks.

Paolo