From 13d1d3084e93467c5a0e26909fc10de8ae0258f3 Mon Sep 17 00:00:00 2001 From: gardners Date: Thu, 22 Mar 2012 16:33:25 +1030 Subject: [PATCH] Various fixes to track down memory handling bugs. Fixed one free-before-time bug with queuing MDP frames. Some heap corruption bug seems to remain. --- overlay_advertise.c | 2 +- overlay_buffer.c | 26 +++++++++++++++++++++++++- overlay_interface.c | 5 +++++ overlay_mdp.c | 12 ++++++------ overlay_payload.c | 28 ++++++++++++++++++++++++++++ rhizome_packetformats.c | 17 +++++++++++------ serval.h | 2 ++ 7 files changed, 78 insertions(+), 14 deletions(-) diff --git a/overlay_advertise.c b/overlay_advertise.c index 6b759d3f..e3e228bb 100644 --- a/overlay_advertise.c +++ b/overlay_advertise.c @@ -159,7 +159,7 @@ int overlay_route_add_advertisements(int interface,overlay_buffer *e) if (oad_bin==bin&&oad_slot==slot) break; } - e->bytes[rfs_offset]=1+8+1+1+8*slots_used; + ob_setbyte(e,rfs_offset,1+8+1+1+8*slots_used); return 0; } diff --git a/overlay_buffer.c b/overlay_buffer.c index 26efefe8..d9a51069 100644 --- a/overlay_buffer.c +++ b/overlay_buffer.c @@ -95,8 +95,10 @@ int ob_makespace(overlay_buffer *b,int bytes) if (newSize>65536) { if (newSize&65535) newSize+=65536-(newSize&65535); } - if (0) printf(" realloc(b->bytes=%p,newSize=%d)\n", + if (1) printf(" realloc(b->bytes=%p,newSize=%d)\n", b->bytes,newSize); +#warning useless malloc() call to make sure that heap corruption check runs before we do any real work + void *p=malloc(1); unsigned char *r=realloc(b->bytes,newSize); if (!r) return WHY("realloc() failed"); b->bytes=r; @@ -107,6 +109,28 @@ int ob_makespace(overlay_buffer *b,int bytes) return 0; } +int ob_setbyte(overlay_buffer *b,int ofs,unsigned char value) +{ + if (ofs<0||ofs>b->allocSize) { + fprintf(stderr,"ERROR: Asked to set byte %d in overlay buffer %p, which has only %d allocated bytes.\n", + ofs,b,b->allocSize); + exit(-1); + } + b->bytes[ofs]=value; + return 0; +} + +int ob_bcopy(overlay_buffer *b,int from, int to, int len) +{ + if (from<0||to<0||len<0||(from+len)>=b->allocSize||(to+len)>=b->allocSize) + { + fprintf(stderr,"call to ob_bcopy would corrupt memory. Aborting.\n"); + exit(-1); + } + bcopy(&b->bytes[from],&b->bytes[to],len); + return 0; +} + int ob_append_byte(overlay_buffer *b,unsigned char byte) { if (ob_makespace(b,1)) return WHY("ob_makespace() failed"); diff --git a/overlay_interface.c b/overlay_interface.c index 47af67bd..9b24bd9f 100644 --- a/overlay_interface.c +++ b/overlay_interface.c @@ -576,12 +576,17 @@ int overlay_stuff_packet_from_queue(int i,overlay_buffer *e,int q,long long now, overlay_frame **p=&overlay_tx[q].first; while(p&&*p) { + printf("p=%p, *p=%p, queue=%d\n",p,*p,q); + /* Throw away any stale frames */ overlay_frame *pp=*p; if (!pp) break; /* XXX Uses hardcoded freshness threshold, when it should get it from the queue */ + printf("now=%lld, *p=%p, q=%d, overlay_tx[q]=%p\n", + now,*p,q,&overlay_tx[q]); + overlay_queue_dump(&overlay_tx[q]); if (now>((*p)->enqueued_at+overlay_tx[q].latencyTarget)) { /* Stale, so remove from queue. */ diff --git a/overlay_mdp.c b/overlay_mdp.c index cfe89ec3..cc0ab8c9 100644 --- a/overlay_mdp.c +++ b/overlay_mdp.c @@ -280,6 +280,8 @@ int overlay_mdp_poll() frame=calloc(sizeof(overlay_frame),1); if (!frame) return WHY("calloc() failed to allocate overlay frame"); frame->type=OF_TYPE_DATA; + frame->prev=NULL; + frame->next=NULL; /* Work out the disposition of the frame. For now we are only worried about the crypto matters, and not compression that may be applied @@ -350,12 +352,10 @@ int overlay_mdp_poll() if (frame) op_free(frame); return WHY("Error enqueuing frame"); } - else WHY("queued frame"); - - WHY("Not implemented"); - overlay_mdp_reply_error(mdp_named_socket,recvaddr_un,recvaddrlen, - 1,"Sending MDP packets not implemented"); - op_free(frame); + else { + WHY("queued frame"); + return 0; + } } break; case MDP_BIND: /* Bind to port */ diff --git a/overlay_payload.c b/overlay_payload.c index a7383629..1208697e 100644 --- a/overlay_payload.c +++ b/overlay_payload.c @@ -166,6 +166,30 @@ overlay_buffer *overlay_payload_unpackage(overlay_frame *b) { return NULL; } +int dump_queue(char *msg,int q) +{ + overlay_txqueue *qq=&overlay_tx[q]; + printf("Contents of TX queue #%d (%s):\n",q,msg); + printf(" length=%d, maxLength=%d\n",qq->length,qq->maxLength); + struct overlay_frame *f=qq->first,*l=qq->last; + + printf(" head of queue = %p, tail of queue = %p\n", + f,l); + + struct overlay_frame *n=f; + int count=0; + + while(n) { + printf(" queue entry #%d : prev=%p, next=%p\n", + count,n->prev,n->next); + if (n==n->next) { + printf(" ERROR: loop in queue\n"); + return -1; + } + n=n->next; + } + return 0; +} int overlay_payload_enqueue(int q,overlay_frame *p) { /* Add payload p to queue q. @@ -180,6 +204,8 @@ int overlay_payload_enqueue(int q,overlay_frame *p) if (!p) return WHY("Cannot queue NULL"); if (overlay_tx[q].length>=overlay_tx[q].maxLength) return WHY("Queue congested"); + + dump_queue("before",q); overlay_frame *l=overlay_tx[q].last; if (l) l->next=p; @@ -191,6 +217,8 @@ int overlay_payload_enqueue(int q,overlay_frame *p) if (!overlay_tx[q].first) overlay_tx[q].first=p; overlay_tx[q].length++; + dump_queue("after",q); + return 0; } diff --git a/rhizome_packetformats.c b/rhizome_packetformats.c index f7ba7736..92191539 100644 --- a/rhizome_packetformats.c +++ b/rhizome_packetformats.c @@ -73,6 +73,9 @@ int bundles_available=-1; int bundle_offset[2]={0,0}; int overlay_rhizome_add_advertisements(int interface_number,overlay_buffer *e) { +#warning Mac-specific debug thing here + setenv("MallocScribble","1",1); + int pass; int bytes=e->sizeLimit-e->length; int overhead=1+8+1+3+1+1+1; /* maximum overhead */ @@ -192,6 +195,8 @@ int overlay_rhizome_add_advertisements(int interface_number,overlay_buffer *e) int overhead=0; int frameFull=0; if (!pass) overhead=2; + printf("e=%p, e->bytes=%p,e->length=%d\n",e,e->bytes,e->length); + if (ob_makespace(e,overhead+blob_bytes)) { if (debug&DEBUG_RHIZOME) fprintf(stderr,"Stopped cramming %s into Rhizome advertisement frame.\n", @@ -200,8 +205,8 @@ int overlay_rhizome_add_advertisements(int interface_number,overlay_buffer *e) } if (!pass) { /* put manifest length field and manifest ID */ - e->bytes[e->length]=(blob_bytes>>8)&0xff; - e->bytes[e->length+1]=(blob_bytes>>0)&0xff; + ob_setbyte(e,e->length,(blob_bytes>>8)&0xff); + ob_setbyte(e,e->length+1,(blob_bytes>>0)&0xff); if (debug&DEBUG_RHIZOME) fprintf(stderr,"length bytes written at offset 0x%x\n",e->length); } @@ -247,14 +252,14 @@ int overlay_rhizome_add_advertisements(int interface_number,overlay_buffer *e) if (debug&DEBUG_RHIZOME) printf("Appended %d rhizome advertisements to packet using %d bytes.\n",bundles_advertised,bytes_used); int rfs_value=1+8+1+1+1+bytes_used; if (rfs_value<0xfa) - e->bytes[rfs_offset]=rfs_value; + ob_setbyte(e,rfs_offset,rfs_value); else { ob_makespace(e,1); - bcopy(&e->bytes[rfs_offset],&e->bytes[rfs_offset+1], + ob_bcopy(e,rfs_offset,rfs_offset+1, e->length-rfs_offset); - e->bytes[rfs_offset]=0xfa+(rfs_value-250)/256; - e->bytes[rfs_offset+1]=(rfs_value-250)&0xff; + ob_setbyte(e,rfs_offset,0xfa+(rfs_value-250)/256); + ob_setbyte(e,rfs_offset+1,(rfs_value-250)&0xff); e->length++; } printf("Final packet size = %d\n",e->length); diff --git a/serval.h b/serval.h index 02b53bc1..e921c54b 100644 --- a/serval.h +++ b/serval.h @@ -1050,3 +1050,5 @@ int overlay_mdp_client_poll(long long timeout_ms); int overlay_mdp_recv(overlay_mdp_frame *mdp,int *ttl); extern int mdp_client_socket; +int ob_bcopy(overlay_buffer *b,int from, int to, int len); +int ob_setbyte(overlay_buffer *b,int ofs,unsigned char value);