On Fri, 2003-04-04 at 14:47, Gianni Tedesco wrote: > On Fri, 2003-04-04 at 14:33, rmkml wrote: > > Hi All, > > > > I found a pb with firestorm, It's a genuine bug, thanks for spotting. Attached patch should fix it. PS. The syslog thing is a bug inherited from the log plugin which is now fixed in CVS. -- // Gianni Tedesco (gianni at scaramanga dot co dot uk) lynx --source www.scaramanga.co.uk/gianni-at-ecsc.asc | gpg --import 8646BE7D: 6D9F 2287 870E A2C9 8F60 3A3C 91B5 7669 8646 BE7D
Index: firestorm/ChangeLog
diff -u firestorm/ChangeLog:1.241 firestorm/ChangeLog:1.242
--- firestorm/ChangeLog:1.241 Fri Apr 4 14:55:55 2003
+++ firestorm/ChangeLog Fri Apr 4 16:57:59 2003
@@ -20,6 +20,7 @@
* Fix dsize bug, fix off-by-one offset bug
* Add syslog target
* Fix log ip address bug for ipfrag
+ * Don't double free fragments when timed out
Version 0.5.2
* Use list head for LRU in tcpstream
Index: firestorm/HACKING
diff -u firestorm/HACKING:1.200 firestorm/HACKING:1.201
--- firestorm/HACKING:1.200 Wed Mar 26 12:49:33 2003
+++ firestorm/HACKING Fri Apr 4 16:57:59 2003
@@ -5,11 +5,6 @@
firestorm but aren't sure how, this is the place to start.
-BUGS
-====
- o There is at least one crash bug...
-
-
API CLEANUPS
============
o Matchers should be registered on to protocols.
Index: firestorm/NEWS
diff -u firestorm/NEWS:1.128 firestorm/NEWS:1.129
--- firestorm/NEWS:1.128 Fri Apr 4 14:55:55 2003
+++ firestorm/NEWS Fri Apr 4 16:57:59 2003
@@ -7,6 +7,7 @@
* Fixed longstanding bugs in dsize matcher
* Snort compatible 'offset' modifier
* Fix bug in log plugin displaying IP addresses in ipfrag alerts
+ * Fix crash bug in ipfrag
NEW FEATURES
* Balance alerts between alert spools
* Firecat can create indexes on elog files
Index: firestorm/decode_plugins/ipfrag.c
diff -u firestorm/decode_plugins/ipfrag.c:1.29 firestorm/decode_plugins/ipfrag.c:1.30
--- firestorm/decode_plugins/ipfrag.c:1.29 Sat Jan 25 16:47:05 2003
+++ firestorm/decode_plugins/ipfrag.c Fri Apr 4 16:57:59 2003
@@ -77,6 +77,23 @@
#define IPHASH 128 /* Must be a power of two */
struct ipq *ipq_hash[IPHASH]; /* IP fragment hash table */
+__INLINE__ struct ipfrag *fragstruct_alloc(void)
+{
+ struct ipfrag *ret;
+
+ ret=malloc(sizeof(*ret));
+ if ( ret )
+ ipfrag_mem += sizeof(*ret);
+
+ return ret;
+}
+
+__INLINE__ void fragstruct_free(struct ipfrag *x)
+{
+ free(x);
+ ipfrag_mem -= sizeof(*x);
+}
+
/* Hash function for ipq_hash lookup */
__INLINE__ unsigned int ipq_hashfn(u_int16_t id,
u_int32_t saddr,
@@ -190,8 +207,7 @@
free(bar->fdata);
ipfrag_mem-=bar->flen;
}
- free(bar);
- ipfrag_mem-=sizeof(struct ipfrag);
+ fragstruct_free(bar);
}
/* Remove from LRU queue */
@@ -395,11 +411,12 @@
/* Trim down to low memory watermark */
__INLINE__ void ip_evictor(struct packet *pkt, struct ipq *cq)
{
- dmesg(M_DEBUG,"Running the ipfrag evictor!");
+ dmesg(M_DEBUG,"Running the ipfrag evictor! %u(%i) %i",
+ ipfrag_mem, ipfrag_mem, sizeof(struct ipfrag));
ipfrag_oom(pkt);
+ ipfrag_err_mem++;
while ( (ipfrag_mem > ipfrag_mem_lo) ) {
if ( !ipq_oldest || ipq_oldest==cq) return;
- ipfrag_err_mem++;
ipq_kill(ipq_oldest);
}
}
@@ -433,7 +450,7 @@
* is suspicious (read: evasive) */
ipfrag_timedout(pkt);
ipq_kill(qp);
- return 0;
+ return 1;
}
/* Check other timeouts */
@@ -524,8 +541,9 @@
}
/* Insert data into fragment chain */
- if ( !(me=calloc(1, sizeof(struct ipfrag))) ) return 1;
- ipfrag_mem+=sizeof(struct ipfrag);
+ me = fragstruct_alloc();
+ if ( !me )
+ return 1;
/* Find out where to insert this fragment in the list */
prev=NULL;
@@ -575,41 +593,46 @@
}
qp->meat -= free_it->len;
- free(free_it);
- ipfrag_mem-=sizeof(struct ipfrag);
+ fragstruct_free(free_it);
}
}
/* Make the fragment */
- me->len=len-(chop);
- me->offset=offset;
- me->len-=ihl;
+ me->len = len - chop;
+ me->offset = offset;
+ me->len -= ihl;
if ( offset ) {
chop=ihl;
+ } else {
+ chop=0;
}
/* IP defragmentation can be zerocopy */
if ( pkt->flags & FP_CLONE ) {
unsigned int alen=me->len;
- if ( !offset ) alen+=ihl;
+ if ( !offset )
+ alen+=ihl;
if ( !(me->fdata=malloc(alen)) ) {
- free(me);
+ fragstruct_free(me);
return 1;
}
ipfrag_mem+=alen;
- memcpy(me->fdata, ((void *)iph)+chop, alen);
+ memcpy(me->fdata, ((char *)iph)+chop, alen);
me->free=1;
me->flen=alen;
+ mesg(M_DEBUG, "alloc %u", alen);
}else {
me->fdata=((void *)iph)+chop;
me->free=0;
}
me->data=me->fdata;
- if ( !offset ) me->data+=ihl;
+ /* FIXME: looks wrong */
+ if ( !offset )
+ me->data+=ihl;
/* Insert the fragment */
me->next=next;
Index: firestorm/decode_plugins/tcpstream.c
diff -u firestorm/decode_plugins/tcpstream.c:1.119 firestorm/decode_plugins/tcpstream.c:1.120
--- firestorm/decode_plugins/tcpstream.c:1.119 Sat Jan 25 15:19:38 2003
+++ firestorm/decode_plugins/tcpstream.c Fri Apr 4 16:57:59 2003
@@ -1115,6 +1115,7 @@
if ( !(tcp_hash=calloc(tcp_hashsz,
sizeof(*tcp_hash))) ) {
free(all_sessions);
+ all_sessions=NULL;
goto oom;
}
Attachment:
signature.asc
Description: This is a digitally signed message part