From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Mon, 24 Feb 2020 21:36:40 +0000 (-0500)
Subject: Use 'noop' callback for unset function pointers.  Fixes SEGV when filters are swapped... 
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=80ea68d03703fb5628debdb4c53bdc2b92ae225b;p=thirdparty%2Ffreeradius-server.git

Use 'noop' callback for unset function pointers.  Fixes SEGV when filters are swapped out between corral and service.
---

diff --git a/src/lib/util/event.c b/src/lib/util/event.c
index 6f0970de21a..b4dd24db168 100644
--- a/src/lib/util/event.c
+++ b/src/lib/util/event.c
@@ -415,6 +415,16 @@ fr_time_t fr_event_list_time(fr_event_list_t *el)
 	}
 }
 
+/** Placeholder callback to avoid branches in service loop
+ *
+ * This is set in place of any NULL function pointers, so that the event loop doesn't
+ * SEGV if a filter callback function is unset between corral and service.
+ */
+static void fr_event_fd_noop(UNUSED fr_event_list_t *el, UNUSED int fd, UNUSED int flags, UNUSED void *uctx)
+{
+	return;
+}
+
 /** Build a new evset based on function pointers present
  *
  * @note The contents of active functions may be inconsistent if this function errors.  But the
@@ -452,16 +462,25 @@ static ssize_t fr_event_build_evset(struct kevent out_kev[], size_t outlen, fr_e
 		uint32_t	prev_fflags = 0;
 
 		do {
-			if (*(uintptr_t const *)((uint8_t const *)prev + map->offset)) {
-				EVENT_DEBUG("\t%s prev set", map->name);
+			fr_event_fd_cb_t prev_func;
+			fr_event_fd_cb_t new_func;
+
+			/*
+			 *	If the previous value was the 'noop'
+			 *	callback, it's identical to being unset.
+			 */
+			prev_func = *(fr_event_fd_cb_t const *)((uint8_t const *)prev + map->offset);
+			if (prev_func && (prev_func != fr_event_fd_noop)) {
+				EVENT_DEBUG("\t%s prev set (%p)", map->name, prev_func);
 				prev_fflags |= map->fflags;
 				has_prev_func = true;
 			} else {
 				EVENT_DEBUG("\t%s prev unset", map->name);
 			}
 
-			if (*(uintptr_t const *)((uint8_t const *)new + map->offset)) {
-				EVENT_DEBUG("\t%s curr set", map->name);
+			new_func = *(fr_event_fd_cb_t const *)((uint8_t const *)new + map->offset);
+			if (new_func) {
+				EVENT_DEBUG("\t%s curr set (%p)", map->name, new_func);
 				current_fflags |= map->fflags;
 				has_current_func = true;
 
@@ -485,10 +504,12 @@ static ssize_t fr_event_build_evset(struct kevent out_kev[], size_t outlen, fr_e
 				       sizeof(fr_event_fd_cb_t));
 			} else {
 				EVENT_DEBUG("\t%s curr unset", map->name);
+
 				/*
 				 *	Mark this filter function as inactive
+				 *	by setting it to the 'noop' callback.
 				 */
-				memset((uint8_t *)active + map->offset, 0, sizeof(fr_event_fd_cb_t));
+				*((fr_event_fd_cb_t *)((uint8_t *)active + map->offset)) = fr_event_fd_noop;
 			}
 
 			if (!(map + 1)->coalesce) break;
@@ -1527,6 +1548,8 @@ int fr_event_corral(fr_event_list_t *el, fr_time_t now, bool wait)
 
 	el->num_fd_events = num_fd_events;
 
+	EVENT_DEBUG("%s - kevent returned %u FD events", __FUNCTION__, el->num_fd_events);
+
 	/*
 	 *	If there are no FD events, we must have woken up from a timer
 	 */
@@ -1558,6 +1581,8 @@ void fr_event_service(fr_event_list_t *el)
 
 	if (unlikely(el->exit)) return;
 
+	EVENT_DEBUG("%s - Servicing %u FD events", __FUNCTION__, el->num_fd_events);
+
 	/*
 	 *	Run all of the file descriptor events.
 	 */