Re: [Qemu-devel] Crashing in tcp_close

[Samuel Thibault]

Hello,

Stefan Hajnoczi, on Fri 04 Nov 2016 11:14:19 +0000, wrote:
> CCing slirp maintainers to get attention on this bug

Thanks!

> > Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
> > 0x00007ffff6a1bb5b in _int_free (av=0x7ffff6d5fb20 <main_arena>,
> > p=<optimised out>, have_lock=0) at malloc.c:4006
> > 4006    malloc.c: No such file or directory.
> > (gdb) bt
> > #0  0x00007ffff6a1bb5b in _int_free (av=0x7ffff6d5fb20 <main_arena>,
> > p=<optimised out>, have_lock=0)
> >     at malloc.c:4006
> > #1  0x00007ffff6a1fabc in __GI___libc_free (mem=<optimised out>) at
> > malloc.c:2969
> > #2  0x00005555559a6c0f in tcp_close (tp=tp@entry=0x555556621ed0) at
> > slirp/tcp_subr.c:334
> > #3  0x00005555559a6c8f in tcp_drop (tp=tp@entry=0x555556621ed0,
> > err=<optimised out>) at slirp/tcp_subr.c:298
> > #4  0x00005555559a816b in tcp_timers (timer=<optimised out>,
> > tp=0x555556621ed0) at slirp/tcp_timer.c:179
> > #3  0x00005555559a6c8f in tcp_drop (tp=tp@entry=0x555556621ed0,
> > err=<optimised out>) at slirp/tcp_subr.c:298
> > #4  0x00005555559a816b in tcp_timers (timer=<optimised out>,
> > tp=0x555556621ed0) at slirp/tcp_timer.c:179
> > #5  tcp_slowtimo (slirp=slirp@entry=0x55555658ecf0) at slirp/tcp_timer.c:89

> > * If so, what additional gdb output would you like me to provide?
> 
> I wonder if this connection has already been closed/freed before and the
> timer fires shortly afterward.  That's just a guess based on the
> backtrace.

That's very unlikely: soclose removes the socket from the list, so
tcp_slowtimo wouldn't be able to find it. That'd rather be a buffer
overflow. But it's hard to believe it could come from the socket
structure since it doesn't contain any buffer.

Brian, could you run it with

export MALLOC_CHECK_=2

and also this could be useful:

export MALLOC_PERTURB_=1234

Also, to rule out the double-free scenario, and try to catch a buffer
overflow coming from the socket structure itself, I have attached a
patch which adds some debugging.

> > * If developers want to reproduce this, let me know and I can probably send
> > the VM qcow2 file and/or packer source privately off-list [I need to check
> > permission for that]

That could be useful.

Samuel

diff --git a/slirp/socket.c b/slirp/socket.c
index 280050a..e603164 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -51,10 +51,12 @@ socreate(Slirp *slirp)
   so = (struct socket *)malloc(sizeof(struct socket));
   if(so) {
     memset(so, 0, sizeof(struct socket));
+    so->canary1 = 0xdeadbeef;
     so->so_state = SS_NOFDREF;
     so->s = -1;
     so->slirp = slirp;
     so->pollfds_idx = -1;
+    so->canary2 = 0xbe3fd3ad;
   }
   return(so);
 }
@@ -67,6 +69,14 @@ sofree(struct socket *so)
 {
   Slirp *slirp = so->slirp;
 
+  if (so->s == -1234)
+    fprintf(stderr,"oops, re-freeing a freed socket!\n");
+  if (so->canary1 != 0xdeadbeef)
+    fprintf(stderr,"oops, canary1 bogus!\n");
+  if (so->canary2 != 0xbe3fd3ad)
+    fprintf(stderr,"oops, canary2 bogus!\n");
+  so->s = -1234;
+
   if (so->so_emu==EMU_RSH && so->extra) {
        sofree(so->extra);
        so->extra=NULL;
diff --git a/slirp/socket.h b/slirp/socket.h
index 8feed2a..14fac1c 100644
--- a/slirp/socket.h
+++ b/slirp/socket.h
@@ -17,6 +17,7 @@
 
 struct socket {
   struct socket *so_next,*so_prev;      /* For a linked list of sockets */
+  int canary1;
 
   int s;                           /* The actual socket */
 
@@ -70,6 +71,7 @@ struct socket {
   struct sbuf so_rcv;          /* Receive buffer */
   struct sbuf so_snd;          /* Send buffer */
   void * extra;                        /* Extra pointer */
+  int canary2;
 };