aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJordan Augé <jordan.auge+fdio@cisco.com>2021-01-20 16:32:34 +0100
committerJacques SAMAIN <jsamain+fdio@cisco.com>2021-01-27 13:43:39 +0000
commit9b0e459e9d3300fba34c87e8afcb847d83971173 (patch)
treedec73f98c3e3801d3ba4206cfb0e05a12d55065c
parent2d33dfd939488b81cc4a23f78f949e72391ef236 (diff)
[HICN-668] Fix leaks + double free
Change-Id: I88795e5dc2a55df7ffee5cf66a9bd4fa5652e353 Signed-off-by: Jordan Augé <jordan.auge+fdio@cisco.com>
-rw-r--r--hicn-light/src/hicn/command_line/daemon/hicnLightDaemon_main.c5
-rw-r--r--hicn-light/src/hicn/core/mapme.c181
-rw-r--r--hicn-light/src/hicn/processor/fibEntry.c11
-rw-r--r--hicn-light/src/hicn/processor/fibEntry.h4
-rw-r--r--hicn-light/src/hicn/strategies/loadBalancer.c5
5 files changed, 96 insertions, 110 deletions
diff --git a/hicn-light/src/hicn/command_line/daemon/hicnLightDaemon_main.c b/hicn-light/src/hicn/command_line/daemon/hicnLightDaemon_main.c
index 89a80d0b1..ab7039f5e 100644
--- a/hicn-light/src/hicn/command_line/daemon/hicnLightDaemon_main.c
+++ b/hicn-light/src/hicn/command_line/daemon/hicnLightDaemon_main.c
@@ -374,6 +374,7 @@ int main(int argc, const char *argv[]) {
logger_Log(logger, LoggerFacility_Core, PARCLogLevel_Error, "daemon",
"Forwarder initialization failed. Are you running it with sudo "
"privileges?");
+ logger_Release(&logger);
return -1;
}
@@ -402,9 +403,9 @@ int main(int argc, const char *argv[]) {
forwarder_Destroy(&forwarder);
#ifndef _WIN32
- sleep(2);
+ //sleep(2);
#else
- Sleep(2000);
+ //Sleep(2000);
WSACleanup();
#endif
diff --git a/hicn-light/src/hicn/core/mapme.c b/hicn-light/src/hicn/core/mapme.c
index 4ee3d5bff..2a64833a8 100644
--- a/hicn-light/src/hicn/core/mapme.c
+++ b/hicn-light/src/hicn/core/mapme.c
@@ -144,21 +144,31 @@ ERR_MALLOC:
return NULL;
}
-void mapmeTFIB_Release(MapMeTFIB **tfibPtr) {
- MapMeTFIB *tfib = *tfibPtr;
- /* TODO; Release all timers: see mapmeTFIB_Remove */
- parcHashMap_Release(&tfib->nexthops);
- free(tfib);
- *tfibPtr = NULL;
+static PARCIterator *mapmeTFIB_CreateKeyIterator(const MapMeTFIB *tfib) {
+ /*
+ * Creating an iterator on an empty HashMap seems to raise an exception
+ * due to :
+ * parcTrapOutOfMemoryIf(state->listIterator == NULL,
+ * "Cannot create parcLinkedList_CreateIterator");
+ * in : _parcHashMap_Init
+ *
+ * All buckets are empty, as they are after creation, and as they should be,
+ * but the error is triggered.
+ */
+ if (parcHashMap_Size(tfib->nexthops) == 0)
+ return NULL;
+ return parcHashMap_CreateKeyIterator(tfib->nexthops);
}
+void mapmeTFIB_Release(const MapMe * mapme, MapMeTFIB **tfibPtr);
+
/**
* @function mapme_CreateTFIB
* @abstract Associate a new TFIB entry to a FIB entry.
* @param [in] - Pointer to the FIB entry.
* @return Boolean indicating the success of the operation.
*/
-static void mapme_CreateTFIB(FibEntry *fibEntry) {
+static void mapme_CreateTFIB(const MapMe * mapme, FibEntry *fibEntry) {
MapMeTFIB *tfib;
/* Make sure we don't already have an associated TFIB entry */
@@ -166,7 +176,7 @@ static void mapme_CreateTFIB(FibEntry *fibEntry) {
// assertNull(tfib);
tfib = mapmeTFIB_Create();
- fibEntry_setUserData(fibEntry, tfib, (void (*)(void **))mapmeTFIB_Release);
+ fibEntry_setUserData(fibEntry, mapme, tfib, (void (*)(const void*, void**))mapmeTFIB_Release);
}
#define TFIB(fibEntry) ((MapMeTFIB *)fibEntry_getUserData(fibEntry))
@@ -197,9 +207,9 @@ static void mapmeTFIB_Put(MapMeTFIB *tfib, unsigned conn_id,
* syntax...
*/
PARCUnsigned *cid = parcUnsigned_Create(conn_id);
- PARCBuffer *buffer =
- parcBuffer_CreateFromArray(&timer, sizeof(PARCEventTimer *));
+ PARCBuffer *buffer = parcBuffer_CreateFromArray(&timer, sizeof(PARCEventTimer *));
parcHashMap_Put(tfib->nexthops, cid, buffer);
+
parcUnsigned_Release(&cid);
parcBuffer_Release(&buffer);
}
@@ -213,6 +223,7 @@ static void mapmeTFIB_Remove(const MapMe * mapme, MapMeTFIB *tfib, unsigned conn
PARCByteArray *array = parcBuffer_Array(buffer);
PARCEventTimer * timer = *((PARCEventTimer **)parcByteArray_Array(array));
if (timer) {
+ parcEventTimer_Stop(timer);
Dispatcher *dispatcher = forwarder_GetDispatcher(mapme->forwarder);
dispatcher_DestroyTimerEvent(dispatcher, &timer);
}
@@ -222,22 +233,27 @@ static void mapmeTFIB_Remove(const MapMe * mapme, MapMeTFIB *tfib, unsigned conn
parcUnsigned_Release(&cid);
}
-static PARCIterator *mapmeTFIB_CreateKeyIterator(const MapMeTFIB *tfib) {
- /*
- * Creating an iterator on an empty HashMap seems to raise an exception
- * due to :
- * parcTrapOutOfMemoryIf(state->listIterator == NULL,
- * "Cannot create parcLinkedList_CreateIterator");
- * in : _parcHashMap_Init
- *
- * All buckets are empty, as they are after creation, and as they should be,
- * but the error is triggered.
- */
- if (parcHashMap_Size(tfib->nexthops) == 0)
- return NULL;
- return parcHashMap_CreateKeyIterator(tfib->nexthops);
+void mapmeTFIB_Release(const MapMe * mapme, MapMeTFIB **tfibPtr) {
+ MapMeTFIB *tfib = *tfibPtr;
+
+ /* Release all TFIB entries, incl. timers */
+ PARCIterator *iterator = mapmeTFIB_CreateKeyIterator(tfib);
+ if (iterator) {
+ /* No iterator is created if the TFIB is empty */
+ while (parcIterator_HasNext(iterator)) {
+ PARCUnsigned *cid = parcIterator_Next(iterator);
+ unsigned conn_id = parcUnsigned_GetUnsigned(cid);
+ mapmeTFIB_Remove(mapme, tfib, conn_id);
+ }
+ parcIterator_Release(&iterator);
+ }
+
+ parcHashMap_Release(&tfib->nexthops);
+ free(tfib);
+ *tfibPtr = NULL;
}
+
int hicn_prefix_from_name(const Name *name, hicn_prefix_t *prefix) {
NameBitvector *bv = name_GetContentName(name);
ip_prefix_t ip_prefix;
@@ -250,15 +266,11 @@ int hicn_prefix_from_name(const Name *name, hicn_prefix_t *prefix) {
static Message *mapme_createMessage(const MapMe *mapme, const Name *name,
mapme_params_t *params) {
Ticks now = forwarder_GetTicks(mapme->forwarder);
- Logger *logger = logger_Acquire(forwarder_GetLogger(mapme->forwarder));
+ Logger *logger = forwarder_GetLogger(mapme->forwarder);
INFO(mapme, "[MAP-Me] CreateMessage type=%d seq=%d", params->type,
params->seq);
- size_t size = (params->protocol == IPPROTO_IPV6) ? HICN_MAPME_V6_HDRLEN
- : HICN_MAPME_V4_HDRLEN;
- uint8_t *icmp_pkt = parcMemory_AllocateAndClear(size);
-
hicn_prefix_t prefix;
int rc = hicn_prefix_from_name(name, &prefix);
if (rc < 0) {
@@ -266,6 +278,11 @@ static Message *mapme_createMessage(const MapMe *mapme, const Name *name,
goto ERR_NAME;
}
+
+ size_t size = (params->protocol == IPPROTO_IPV6) ? HICN_MAPME_V6_HDRLEN
+ : HICN_MAPME_V4_HDRLEN;
+ uint8_t *icmp_pkt = parcMemory_AllocateAndClear(size);
+
INFO(mapme, "[MAP-Me] Creating MAP-Me packet");
size_t len = hicn_mapme_create_packet(icmp_pkt, &prefix, params);
if (len == 0) {
@@ -279,6 +296,7 @@ static Message *mapme_createMessage(const MapMe *mapme, const Name *name,
MessagePacketType_Interest, now, logger);
ERR_CREATE:
+ parcMemory_Deallocate(&icmp_pkt);
ERR_NAME:
return NULL;
}
@@ -287,7 +305,7 @@ static Message *mapme_createAckMessage(const MapMe *mapme,
const uint8_t *msgBuffer,
const mapme_params_t *params) {
Ticks now = forwarder_GetTicks(mapme->forwarder);
- Logger *logger = logger_Acquire(forwarder_GetLogger(mapme->forwarder));
+ Logger *logger = forwarder_GetLogger(mapme->forwarder);
size_t size = (params->protocol == IPPROTO_IPV6) ? HICN_MAPME_V6_HDRLEN
: HICN_MAPME_V4_HDRLEN;
@@ -297,11 +315,15 @@ static Message *mapme_createAckMessage(const MapMe *mapme,
size_t len = hicn_mapme_create_ack(icmp_pkt, params);
if (len != size) {
ERR(mapme, "[MAP-Me] Failed to create mapme ack packet through lib");
- return NULL;
+ goto ERR;
}
return message_CreateFromByteArray(
NO_INGRESS, icmp_pkt, MessagePacketType_ContentObject, now, logger);
+
+ERR:
+ parcMemory_Deallocate(&icmp_pkt);
+ return NULL;
}
struct setFacePendingArgs {
@@ -329,6 +351,7 @@ static void mapme_setFacePendingCallback(int fd, PARCEventType which_event,
INFO(args->mapme, "Timeout during retransmission. Re-sending");
mapme_setFacePending(args->mapme, args->name, args->fibEntry, args->conn_id,
args->send, args->is_producer, false, args->num_retx);
+ free(args);
}
/**
@@ -392,9 +415,6 @@ static bool mapme_setFacePending(const MapMe *mapme, const Name *name,
for (size_t i = 0; i < numberSet_Length(conns); i++) {
unsigned conn_id = numberSet_GetItem(conns, i);
- PARCEventTimer *oldTimer = (PARCEventTimer *)mapmeTFIB_Get(TFIB(fibEntry), conn_id);
- if (oldTimer)
- parcEventTimer_Stop(oldTimer);
mapmeTFIB_Remove(mapme, TFIB(fibEntry), conn_id);
}
@@ -429,17 +449,18 @@ static bool mapme_setFacePending(const MapMe *mapme, const Name *name,
name_str, params.seq, conn_id);
free(name_str);
connection_ReSend(conn, special_interest, NOT_A_NOTIFICATION);
- parcMemory_Deallocate((void**)&special_interest);
} else {
INFO(mapme, "[MAP-Me] Stopped retransmissions as face went down");
}
+ message_Release(&special_interest);
if (num_retx < MAX_RETX) {
INFO(mapme, "[MAP-Me] - Scheduling retransmission\n");
/* Schedule retransmission */
struct setFacePendingArgs *args =
malloc(sizeof(struct setFacePendingArgs));
- if (!args) goto ERR_MALLOC;
+ if (!args)
+ goto ERR_MALLOC;
args->mapme = mapme;
args->name = name;
args->fibEntry = fibEntry;
@@ -453,7 +474,10 @@ static bool mapme_setFacePending(const MapMe *mapme, const Name *name,
struct timeval timeout = {mapme->retx / 1000,
(mapme->retx % 1000) * 1000};
rc = parcEventTimer_Start(timer, &timeout);
- if (rc < 0) goto ERR_TIMER;
+ if (rc < 0) {
+ free(args);
+ goto ERR_TIMER;
+ }
} else {
INFO(mapme, "[MAP-Me] Last retransmission.");
timer = NULL;
@@ -463,19 +487,14 @@ static bool mapme_setFacePending(const MapMe *mapme, const Name *name,
timer = NULL;
}
- PARCEventTimer *oldTimer =
- (PARCEventTimer *)mapmeTFIB_Get(TFIB(fibEntry), conn_id);
- if (oldTimer) {
- INFO(mapme, "[MAP-Me] - Found old timer, would need to cancel !");
- // parcEventTimer_Stop(oldTimer);
- }
- INFO(mapme, "[MAP-Me] - Putting new timer in TFIB");
- if (timer) mapmeTFIB_Put(TFIB(fibEntry), conn_id, timer);
+ mapmeTFIB_Remove(mapme, TFIB(fibEntry), conn_id);
+ mapmeTFIB_Put(TFIB(fibEntry), conn_id, timer);
return true;
-ERR_MALLOC:
ERR_TIMER:
+ dispatcher_DestroyTimerEvent(dispatcher, &timer);
+ERR_MALLOC:
return false;
}
@@ -511,7 +530,7 @@ void
mapme_send_updates(const MapMe * mapme, FibEntry * fibEntry, const NumberSet * nexthops)
{
if (!TFIB(fibEntry)) /* Create TFIB associated to FIB entry */
- mapme_CreateTFIB(fibEntry);
+ mapme_CreateTFIB(mapme, fibEntry);
TFIB(fibEntry)->seq++;
const Name *name = fibEntry_GetPrefix(fibEntry);
@@ -617,39 +636,6 @@ mapme_onConnectionEvent(const MapMe *mapme, const Connection *conn_added, connec
INFO(mapme, "[MAP-Me] Done");
}
-#if 0
-#ifdef WITH_POLICY
-void mapme_onPolicyUpdate(const MapMe *mapme, const Connection *conn_selected, FibEntry * fibEntry)
-{
- /* Ignore local connections corresponding to applications for now */
- if (connection_IsLocal(conn_selected))
- return;
-
- unsigned conn_selected_id = connection_GetConnectionId(conn_selected);
- INFO(mapme, "[MAP-Me] New connection %d", conn_selected_id);
-
- const Name *name = fibEntry_GetPrefix(fibEntry);
-
- /* Skip entries that have no local connection as next hop */
- if (!mapme_hasLocalNextHops(mapme, fibEntry))
- return;
-
- /* This entry corresponds to a locally served prefix, set
- * Special Interest */
- if (!TFIB(fibEntry)) /* Create TFIB associated to FIB entry */
- mapme_CreateTFIB(fibEntry);
- TFIB(fibEntry)->seq++;
-
- char *name_str = name_ToString(name);
- INFO(mapme, "[MAP-Me] sending IU/IN for name %s on connection %d", name_str,
- conn_selected_id);
- free(name_str);
-
- mapme_setFacePending(mapme, name, fibEntry, conn_selected_id, true, true, true, 0);
-}
-#endif /* WITH_POLICY */
-#endif
-
/*------------------------------------------------------------------------------
* Special Interest handling
*----------------------------------------------------------------------------*/
@@ -684,11 +670,18 @@ static bool mapme_onSpecialInterest(const MapMe *mapme,
* We always ack, even duplicates.
*/
Message *ack = mapme_createAckMessage(mapme, msgBuffer, params);
- if (!ack) goto ERR_ACK_CREATE;
+ if (!ack) {
+ name_Release(&name);
+ return false;
+ }
rv = connection_ReSend(conn_in, ack, NOT_A_NOTIFICATION);
- if (!rv) goto ERR_ACK_SEND;
message_Release(&ack);
+ if (!rv) {
+ name_Release(&name);
+ return false;
+ }
+
/* EPM on FIB */
/* only the processor has access to the FIB */
FIB *fib = forwarder_getFib(mapme->forwarder);
@@ -722,7 +715,7 @@ static bool mapme_onSpecialInterest(const MapMe *mapme,
fibEntry = fibEntry_Create(name, fwdStrategy);
#endif /* WITH_POLICY */
FibEntry *lpm = fib_MatchName(fib, name);
- mapme_CreateTFIB(fibEntry);
+ mapme_CreateTFIB(mapme, fibEntry);
fib_Add(fib, fibEntry);
if (!lpm) {
TFIB(fibEntry)->seq = seq;
@@ -745,7 +738,7 @@ static bool mapme_onSpecialInterest(const MapMe *mapme,
/* Create TFIB associated to FIB entry */
INFO(mapme,
"[MAP-Me] - Creating TFIB entry with default sequence number");
- mapme_CreateTFIB(fibEntry);
+ mapme_CreateTFIB(mapme, fibEntry);
}
/*
@@ -816,15 +809,6 @@ static bool mapme_onSpecialInterest(const MapMe *mapme,
*/
INFO(mapme, "[MAP-Me] - (3/3) next hops ~~> prev hops");
- PARCEventTimer *oldTimer =
- (PARCEventTimer *)mapmeTFIB_Get(TFIB(fibEntry), conn_in_id);
- if (oldTimer) {
- /* This happens if we receive an IU while we are still sending
- * one in the other direction
- */
- INFO(mapme, "[MAP-Me] - Canceled pending timer");
- parcEventTimer_Stop(oldTimer);
- }
mapmeTFIB_Remove(mapme, TFIB(fibEntry), conn_in_id);
/* Remove all next hops */
@@ -898,11 +882,6 @@ static bool mapme_onSpecialInterest(const MapMe *mapme,
}
return true;
-
-ERR_ACK_SEND:
- message_Release(&ack);
-ERR_ACK_CREATE:
- return false;
}
void mapme_onSpecialInterestAck(const MapMe *mapme, const uint8_t *msgBuffer,
@@ -910,7 +889,7 @@ void mapme_onSpecialInterestAck(const MapMe *mapme, const uint8_t *msgBuffer,
mapme_params_t *params) {
INFO(mapme, "[MAP-Me] Receive IU/IN Ack on connection %d", conn_in_id);
- const Name * name =
+ Name * name =
name_CreateFromPacket(msgBuffer, MessagePacketType_ContentObject);
name_setLen((Name*) name, prefix->len);
char * name_str = name_ToString(name);
@@ -920,6 +899,9 @@ void mapme_onSpecialInterestAck(const MapMe *mapme, const uint8_t *msgBuffer,
FIB *fib = forwarder_getFib(mapme->forwarder);
FibEntry *fibEntry = fib_Contains(fib, name);
+
+ name_Release(&name);
+
if (!fibEntry) {
return;
}
@@ -967,7 +949,6 @@ void mapme_onSpecialInterestAck(const MapMe *mapme, const uint8_t *msgBuffer,
}
/* Stop timer and remove entry from TFIB */
- parcEventTimer_Stop(timer);
mapmeTFIB_Remove(mapme, TFIB(fibEntry), conn_in_id);
INFO(mapme, "[MAP-Me] - Removing TFIB entry for ack on connection %d",
diff --git a/hicn-light/src/hicn/processor/fibEntry.c b/hicn-light/src/hicn/processor/fibEntry.c
index 7412b4ccf..30bcc1f9a 100644
--- a/hicn-light/src/hicn/processor/fibEntry.c
+++ b/hicn-light/src/hicn/processor/fibEntry.c
@@ -64,8 +64,9 @@ struct fib_entry {
#endif /* WITH_POLICY */
#ifdef WITH_MAPME
NumberSet * previous_nexthops;
+ const void *userDataOwner;
void *userData;
- void (*userDataRelease)(void **userData);
+ void (*userDataRelease)(const void *owner, void **userData);
#endif /* WITH_MAPME */
};
@@ -101,6 +102,7 @@ FibEntry *fibEntry_Create(Name *name, strategy_type fwdStrategy) {
fibEntry->refcount = 1;
#ifdef WITH_MAPME
+ fibEntry->userDataOwner = NULL;
fibEntry->userData = NULL;
fibEntry->userDataRelease = NULL;
#endif /* WITH_MAPME */
@@ -136,7 +138,7 @@ void fibEntry_Release(FibEntry **fibEntryPtr) {
fibEntry->fwdStrategy->destroy(&(fibEntry->fwdStrategy));
#ifdef WITH_MAPME
if (fibEntry->userData) {
- fibEntry->userDataRelease(&fibEntry->userData);
+ fibEntry->userDataRelease(fibEntry->userDataOwner, &fibEntry->userData);
}
#endif /* WITH_MAPME */
#ifdef WITH_POLICY
@@ -881,9 +883,10 @@ void *fibEntry_getUserData(const FibEntry *fibEntry) {
return fibEntry->userData;
}
-void fibEntry_setUserData(FibEntry *fibEntry, const void *userData,
- void (*userDataRelease)(void **)) {
+void fibEntry_setUserData(FibEntry *fibEntry, const void *userDataOwner, const
+ void *userData, void (*userDataRelease)(const void *, void **)) {
parcAssertNotNull(fibEntry, "Parameter fibEntry must be non-null");
+ fibEntry->userDataOwner = userDataOwner;
fibEntry->userData = (void *)userData;
fibEntry->userDataRelease = userDataRelease;
}
diff --git a/hicn-light/src/hicn/processor/fibEntry.h b/hicn-light/src/hicn/processor/fibEntry.h
index 3f78f47dd..cf267d6a6 100644
--- a/hicn-light/src/hicn/processor/fibEntry.h
+++ b/hicn-light/src/hicn/processor/fibEntry.h
@@ -158,8 +158,8 @@ void *fibEntry_getUserData(const FibEntry *fibEntry);
* @param [in@ userDataRelease - Callback used to release user data upon change
* of FIB entry removal.
*/
-void fibEntry_setUserData(FibEntry *fibEntry, const void *userData,
- void (*userDataRelease)(void **));
+void fibEntry_setUserData(FibEntry *fibEntry, const void *userDataOwner,
+ const void *userData, void (*userDataRelease)(const void *, void **));
#endif /* WITH_MAPME */
diff --git a/hicn-light/src/hicn/strategies/loadBalancer.c b/hicn-light/src/hicn/strategies/loadBalancer.c
index bba398b02..82b5a6103 100644
--- a/hicn-light/src/hicn/strategies/loadBalancer.c
+++ b/hicn-light/src/hicn/strategies/loadBalancer.c
@@ -216,8 +216,8 @@ static NumberSet *_strategyLoadBalancer_LookupNexthop(
PARCUnsigned *cid = parcUnsigned_Create(numberSet_GetItem(nexthops, i));
const StrategyNexthopState *state =
parcHashMap_Get(lb->strategy_state, cid);
- if (!state){
- parcUnsigned_Release(&cid);
+ if (!state) {
+ parcUnsigned_Release(&cid);
continue;
}
distance -= strategyNexthopState_GetWeight(state);
@@ -227,6 +227,7 @@ static NumberSet *_strategyLoadBalancer_LookupNexthop(
parcUnsigned_Release(&cid);
break;
}
+ parcUnsigned_Release(&cid);
}
#else
unsigned in_connection = message_GetIngressConnectionId(interestMessage);