FreeRADIUS 1.1.1 Segmentation fault on Fedora 4

Alan DeKok aland at nitros9.org
Mon May 8 19:09:40 CEST 2006


Stuart Auchterlonie <stuart.auchterlonie at homechoice.net> 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);




More information about the Freeradius-Users mailing list