Re: FreeRADIUS 1.1.1 Segmentation fault on Fedora 4

[Alan DeKok]

"Alan DeKok" <[EMAIL PROTECTED]> wrote:
>   Sorry, sent the wrong patch.

  With a lock bug.  Dang.  I'll get it right one of these days.

  OK, This should work.

  Alan DeKok.
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 21:53:07 -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 21:53:08 -0000
@@ -25,6 +25,10 @@
 
 static const char rcsid[] = "$Id: mem.c,v 1.14.4.1 2006/02/06 16:23:51 nbk Exp 
$";
 
+static void eaplist_delete_locked(rlm_eap_t *inst, EAP_HANDLER *handler);
+static void eaplist_delete(rlm_eap_t *inst, EAP_HANDLER *handler);
+
+
 /*
  * Allocate a new EAP_PACKET
  */
@@ -111,8 +115,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 +125,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 +205,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 +227,8 @@
         */
        status = rbtree_insert(inst->session_tree, handler);
 
+       handler->in_list = 1;
+
        if (status) {
                EAP_HANDLER *prev;
 
@@ -244,6 +257,56 @@
 }
 
 /*
+ *     Delete, assuming that the mutex is locked.
+ */
+static void eaplist_delete_locked(rlm_eap_t *inst, EAP_HANDLER *handler)
+{
+       rbtree_deletebydata(inst->session_tree, handler);
+
+       /*
+        *      And unsplice it from the linked list.
+        */
+       if (handler->prev) {
+               handler->prev->next = handler->next;
+       } else {
+               inst->session_head = NULL;
+       }
+       if (handler->next) {
+               handler->next->prev = handler->prev;
+       } else {
+               inst->session_tail = NULL;
+       }
+       handler->prev = handler->next = NULL;
+
+       handler->inst = NULL;
+       handler->in_list = 0;
+       eap_handler_free(handler);
+}
+
+
+/*
+ *     Delete a handler from the list.
+ */
+static void eaplist_delete(rlm_eap_t *inst, EAP_HANDLER *handler)
+{
+       if (!handler->in_list || !handler->inst) {
+               handler->inst = NULL;
+               handler->in_list = 0;
+               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_locked(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 +355,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_locked(inst, handler);
                }
        }
 
@@ -320,25 +377,7 @@
                if (verify_state(state, handler->timestamp) != 0) {
                        handler = NULL;
                } else {
-                       /*
-                        *      It's OK, delete it from the tree.
-                        */
-                       rbtree_delete(inst->session_tree, node);
-
-                       /*
-                        *      And unsplice it from the linked list.
-                        */
-                       if (handler->prev) {
-                               handler->prev->next = handler->next;
-                       } else {
-                               inst->session_head = NULL;
-                       }
-                       if (handler->next) {
-                               handler->next->prev = handler->prev;
-                       } else {
-                               inst->session_tail = NULL;
-                       }
-                       handler->prev = handler->next = NULL;
+                       eaplist_delete_locked(inst, handler);
                }
        }
 
- 
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html