From: Victor Julien <victor@inliniac.net>
Date: Thu, 27 Feb 2014 12:23:45 +0000 (+0100)
Subject: pool: on Init() error, properly clean up
X-Git-Tag: suricata-2.0rc2~49
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=82ae41d320fac6739695ab7cc31cbc3f08dc1d25;p=thirdparty%2Fsuricata.git

pool: on Init() error, properly clean up

In the stream engine, Init() can fail if the memcap is reached. In this
case the segment was not freed by PoolGet:

==8600== Thread 1:
==8600== 70,480 bytes in 1,762 blocks are definitely lost in loss record 611 of 612
==8600==    at 0x4C2A2DB: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8600==    by 0x914CC8: TcpSegmentPoolAlloc (stream-tcp-reassemble.c:166)
==8600==    by 0xA0D315: PoolGet (util-pool.c:297)
==8600==    by 0x9302CD: StreamTcpGetSegment (stream-tcp-reassemble.c:3768)
==8600==    by 0x921FE8: StreamTcpReassembleHandleSegmentHandleData (stream-tcp-reassemble.c:1873)
==8600==    by 0x92EEDA: StreamTcpReassembleHandleSegment (stream-tcp-reassemble.c:3584)
==8600==    by 0x8D3BB1: HandleEstablishedPacketToServer (stream-tcp.c:1969)
==8600==    by 0x8D7F98: StreamTcpPacketStateEstablished (stream-tcp.c:2323)
==8600==    by 0x8F13B8: StreamTcpPacket (stream-tcp.c:4243)
==8600==    by 0x8F2537: StreamTcp (stream-tcp.c:4485)
==8600==    by 0x95DFBB: TmThreadsSlotVarRun (tm-threads.c:559)
==8600==    by 0x8BE60D: TmThreadsSlotProcessPkt (tm-threads.h:142)

tcp.segment_memcap_drop   | PcapFile                  | 1762

This patch fixes PoolGet to both Cleanup and Free the Alloc'd data in
case Init fails.
---

diff --git a/src/util-pool.c b/src/util-pool.c
index 41ab1fd708..2db077fa29 100644
--- a/src/util-pool.c
+++ b/src/util-pool.c
@@ -300,8 +300,15 @@ void *PoolGet(Pool *p) {
             }
 
             if (pitem != NULL) {
-                if (p->Init(pitem, p->InitData) != 1)
+                if (p->Init(pitem, p->InitData) != 1) {
+                    if (p->Cleanup)
+                        p->Cleanup(pitem);
+                    if (p->Free != NULL)
+                        p->Free(pitem);
+                    else
+                        SCFree(pitem);
                     SCReturnPtr(NULL, "void");
+                }
 
                 p->allocated++;