Stuart Auchterlonie <[EMAIL PROTECTED]> wrote: > If you ignore the 'unitialized value' errors in the valgrind log then > you come to the real errors, 'Invalid Write', 'Invalid Read' to/from > memory areas that aren't part of the server or were previously freed.
Ah, OK. That looks like it's a bug that's been there a while. It only happens when TLS is being used inside of PEAP, apparently. Try this patch. If it works, I'll add it into 1.1.2 Alan DeKok. -------------- Index: src/modules/rlm_eap/eap.c =================================================================== RCS file: /source/radiusd/src/modules/rlm_eap/eap.c,v retrieving revision 1.52.4.1 diff -u -r1.52.4.1 eap.c --- src/modules/rlm_eap/eap.c 6 Feb 2006 16:23:49 -0000 1.52.4.1 +++ src/modules/rlm_eap/eap.c 8 May 2006 17:13:30 -0000 @@ -1116,7 +1116,7 @@ if (handler->eap_ds == NULL) { free(*eap_packet_p); *eap_packet_p = NULL; - eap_handler_free(handler); + eaplist_delete(handler); return NULL; } Index: src/modules/rlm_eap/eap.h =================================================================== RCS file: /source/radiusd/src/modules/rlm_eap/eap.h,v retrieving revision 1.27.4.1 diff -u -r1.27.4.1 eap.h --- src/modules/rlm_eap/eap.h 6 Feb 2006 16:23:50 -0000 1.27.4.1 +++ src/modules/rlm_eap/eap.h 8 May 2006 17:13:31 -0000 @@ -121,6 +121,9 @@ int status; int stage; + + int in_list; + void *inst; } EAP_HANDLER; /* Index: src/modules/rlm_eap/mem.c =================================================================== RCS file: /source/radiusd/src/modules/rlm_eap/mem.c,v retrieving revision 1.14.4.1 diff -u -r1.14.4.1 mem.c --- src/modules/rlm_eap/mem.c 6 Feb 2006 16:23:51 -0000 1.14.4.1 +++ src/modules/rlm_eap/mem.c 8 May 2006 17:13:31 -0000 @@ -111,8 +111,8 @@ { EAP_HANDLER *handler; - handler = rad_malloc(sizeof(EAP_HANDLER)); - memset(handler, 0, sizeof(EAP_HANDLER)); + handler = rad_malloc(sizeof(*handler)); + memset(handler, 0, sizeof(*handler)); return handler; } @@ -121,6 +121,12 @@ if (!handler) return; + /* + * This is a hack to work around request_data_add not + * taking an "inst" pointer + */ + if (handler->inst) return eaplist_delete(handler->inst, handler); + if (handler->identity) { free(handler->identity); handler->identity = NULL; @@ -195,6 +201,7 @@ */ handler->timestamp = handler->request->timestamp; handler->status = 1; + handler->inst = inst; memcpy(handler->state, state->strvalue, sizeof(handler->state)); handler->src_ipaddr = handler->request->packet->src_ipaddr; @@ -216,6 +223,8 @@ */ status = rbtree_insert(inst->session_tree, handler); + handler->in_list = 1; + if (status) { EAP_HANDLER *prev; @@ -243,6 +252,40 @@ return 1; } + +static void eaplist_delete_internal(rlm_eap_t *inst, EAP_HANDLER *handler) +{ + + rbtree_deletebydata(inst->session_tree, handler); + + inst->session_head = handler->next; + if (handler->next) handler->next->prev = NULL; + handler->inst = NULL; + eap_handler_free(handler); +} + + +/* + * Delete a handler from the list. + */ +void eaplist_delete(rlm_eap_t *inst, EAP_HANDLER *handler) +{ + if (!handler->in_list || !handler->inst) { + handler->inst = NULL; + eap_handler_free(handler); + return; + } + + /* + * Playing with a data structure shared among threads + * means that we need a lock, to avoid conflict. + */ + pthread_mutex_lock(&(inst->session_mutex)); + eaplist_delete_internal(inst, handler); + pthread_mutex_unlock(&(inst->session_mutex)); +} + + /* * Find a a previous EAP-Request sent by us, which matches * the current EAP-Response. @@ -292,13 +335,7 @@ handler = inst->session_head; if (handler && ((request->timestamp - handler->timestamp) > inst->timer_limit)) { - node = rbtree_find(inst->session_tree, handler); - rad_assert(node != NULL); - rbtree_delete(inst->session_tree, node); - - inst->session_head = handler->next; - if (handler->next) handler->next->prev = NULL; - eap_handler_free(handler); + eaplist_delete(inst, handler); } } Index: src/modules/rlm_eap/rlm_eap.c =================================================================== RCS file: /source/radiusd/src/modules/rlm_eap/rlm_eap.c,v retrieving revision 1.26.2.1.2.1 diff -u -r1.26.2.1.2.1 rlm_eap.c --- src/modules/rlm_eap/rlm_eap.c 6 Feb 2006 16:23:52 -0000 1.26.2.1.2.1 +++ src/modules/rlm_eap/rlm_eap.c 8 May 2006 17:13:31 -0000 @@ -244,7 +244,7 @@ case PW_EAP_PEAP: DEBUG2(" rlm_eap: Unable to tunnel TLS inside of TLS"); eap_fail(handler); - eap_handler_free(handler); + eaplist_delete(inst, handler); return RLM_MODULE_INVALID; break; @@ -265,7 +265,7 @@ */ if (rcode == EAP_INVALID) { eap_fail(handler); - eap_handler_free(handler); + eaplist_delete(inst, handler); DEBUG2(" rlm_eap: Failed in EAP select"); return RLM_MODULE_INVALID; } @@ -367,7 +367,7 @@ } else { DEBUG2(" rlm_eap: Freeing handler"); /* handler is not required any more, free it now */ - eap_handler_free(handler); + eaplist_delete(handler); } /* @@ -496,7 +496,7 @@ REQUEST_DATA_EAP_TUNNEL_CALLBACK); if (!data) { radlog(L_ERR, "rlm_eap: Failed to retrieve callback for tunneled session!"); - eap_handler_free(handler); + eaplist_delete(handler); return RLM_MODULE_FAIL; } @@ -507,7 +507,7 @@ free(data); if (rcode == 0) { eap_fail(handler); - eap_handler_free(handler); + eaplist_delete(handler); return RLM_MODULE_REJECT; } @@ -528,7 +528,7 @@ } else { /* couldn't have been LEAP, there's no tunnel */ DEBUG2(" rlm_eap: Freeing handler"); /* handler is not required any more, free it now */ - eap_handler_free(handler); + eaplist_delete(handler); } /* Index: src/modules/rlm_eap/rlm_eap.h =================================================================== RCS file: /source/radiusd/src/modules/rlm_eap/rlm_eap.h,v retrieving revision 1.13.4.1 diff -u -r1.13.4.1 rlm_eap.h --- src/modules/rlm_eap/rlm_eap.h 6 Feb 2006 16:23:53 -0000 1.13.4.1 +++ src/modules/rlm_eap/rlm_eap.h 8 May 2006 17:13:31 -0000 @@ -101,6 +101,7 @@ EAP_HANDLER *eaplist_find(rlm_eap_t *inst, REQUEST *request, eap_packet_t *eap_packet); void eaplist_free(rlm_eap_t *inst); +void eaplist_delete(rlm_eap_t *inst, EAP_HANDLER *handler); /* State */ void generate_key(void); - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html