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
