[svn-commits] rmudgett: branch 1.4 r2202 - in /branches/1.4: pri_internal.h prisched.c

SVN commits to the Digium repositories svn-commits at lists.digium.com
Wed Feb 16 13:23:06 CST 2011


Author: rmudgett
Date: Wed Feb 16 13:23:02 2011
New Revision: 2202

URL: http://svnview.digium.com/svn/libpri?view=rev&rev=2202
Log:
Crash if NFAS swaps D channels on a call with an active timer.

If a Q.931 call record related timer is started on one NFAS D channel
expires after NFAS swaps to another D channel, then libpri could crash.

For example:
1) Hangup a call.
1a) Send a DISCONNECT.
1b) Start the T305 retransmit timer on the current D channel.
2) The RELEASE comes in on another D channel.
2a) The found call record switches its assignment to the new D channel.
2b) Attempt to stop T305.  Unfortunately, the timer was started on another
    D channel so the attempt does not find the timer to stop.
3) The hangup sequence continues normally and the call record is freed
   since there is only one call record pool.
4) T305 expires on the original D channel and crashes the system when it
   uses the stale call record pointer it has saved.

Made each D channel timer pool have a unique range of valid timer
identifiers.  If a given timer identifier is not in the range for the
current NFAS D channel, then search the D channel group for the original D
channel.

JIRA LIBPRI-58
JIRA SWP-2721

Modified:
    branches/1.4/pri_internal.h
    branches/1.4/prisched.c

Modified: branches/1.4/pri_internal.h
URL: http://svnview.digium.com/svn/libpri/branches/1.4/pri_internal.h?view=diff&rev=2202&r1=2201&r2=2202
==============================================================================
--- branches/1.4/pri_internal.h (original)
+++ branches/1.4/pri_internal.h Wed Feb 16 13:23:02 2011
@@ -90,6 +90,8 @@
 		unsigned num_slots;
 		/*! Maximum timer slots currently needed. */
 		unsigned max_used;
+		/*! First timer id in this timer pool. */
+		unsigned first_id;
 	} sched;
 	int debug;			/* Debug stuff */
 	int state;			/* State of D-channel */
@@ -906,12 +908,12 @@
 int q931_is_call_valid(struct pri *ctrl, struct q931_call *call);
 int q931_is_call_valid_gripe(struct pri *ctrl, struct q931_call *call, const char *func_name, unsigned long func_line);
 
-extern int pri_schedule_event(struct pri *pri, int ms, void (*function)(void *data), void *data);
+unsigned pri_schedule_event(struct pri *ctrl, int ms, void (*function)(void *data), void *data);
 
 extern pri_event *pri_schedule_run(struct pri *pri);
 
-extern void pri_schedule_del(struct pri *pri, int ev);
-int pri_schedule_check(struct pri *ctrl, int id, void (*function)(void *data), void *data);
+void pri_schedule_del(struct pri *ctrl, unsigned id);
+int pri_schedule_check(struct pri *ctrl, unsigned id, void (*function)(void *data), void *data);
 
 extern pri_event *pri_mkerror(struct pri *pri, char *errstr);
 
@@ -1008,7 +1010,7 @@
  */
 static inline struct pri *PRI_NFAS_MASTER(struct pri *ctrl)
 {
-	while (ctrl->master) {
+	if (ctrl->master) {
 		ctrl = ctrl->master;
 	}
 	return ctrl;

Modified: branches/1.4/prisched.c
URL: http://svnview.digium.com/svn/libpri/branches/1.4/prisched.c?view=diff&rev=2202&r1=2201&r2=2202
==============================================================================
--- branches/1.4/prisched.c (original)
+++ branches/1.4/prisched.c Wed Feb 16 13:23:02 2011
@@ -38,13 +38,15 @@
 /*! Initial number of scheduled timer slots. */
 #define SCHED_EVENTS_INITIAL	128
 /*!
- * Maximum number of scheduled timer slots.
- * Should be a power of 2 multiple of SCHED_EVENTS_INITIAL.
+ * \brief Maximum number of scheduled timer slots.
+ * \note Should be a power of 2 and at least SCHED_EVENTS_INITIAL.
  */
 #define SCHED_EVENTS_MAX		8192
 
 /*! \brief The maximum number of timers that were active at once. */
 static unsigned maxsched = 0;
+/*! Last pool id */
+static unsigned pool_id = 0;
 
 /* Scheduler routines */
 
@@ -87,6 +89,23 @@
 		memcpy(timers, ctrl->sched.timer,
 			ctrl->sched.num_slots * sizeof(struct pri_sched));
 		free(ctrl->sched.timer);
+	} else {
+		/* Creating the timer pool. */
+		pool_id += SCHED_EVENTS_MAX;
+		if (pool_id < SCHED_EVENTS_MAX
+			|| pool_id + (SCHED_EVENTS_MAX - 1) < SCHED_EVENTS_MAX) {
+			/*
+			 * Not likely to happen.
+			 *
+			 * Timer id's may be aliased if this D channel is used in an
+			 * NFAS group with redundant D channels.  Another D channel in
+			 * the group may have the same pool_id.
+			 */
+			pri_error(ctrl,
+				"Pool_id wrapped.  Please ignore if you are not using NFAS with backup D channels.\n");
+			pool_id = SCHED_EVENTS_MAX;
+		}
+		ctrl->sched.first_id = pool_id;
 	}
 
 	/* Put the new timer table in place. */
@@ -106,7 +125,7 @@
  * \retval 0 if scheduler table is full and could not schedule the event.
  * \retval id Scheduled event id.
  */
-int pri_schedule_event(struct pri *ctrl, int ms, void (*function)(void *data), void *data)
+unsigned pri_schedule_event(struct pri *ctrl, int ms, void (*function)(void *data), void *data)
 {
 	unsigned max_used;
 	unsigned x;
@@ -138,7 +157,7 @@
 	ctrl->sched.timer[x].when = tv;
 	ctrl->sched.timer[x].callback = function;
 	ctrl->sched.timer[x].data = data;
-	return x + 1;
+	return ctrl->sched.first_id + x;
 }
 
 /*!
@@ -231,18 +250,35 @@
  * \param ctrl D channel controller.
  * \param id Scheduled event id to delete.
  * 0 is a disabled/unscheduled event id that is ignored.
- * 1 - MAX_SCHED is a valid event id.
  *
  * \return Nothing
  */
-void pri_schedule_del(struct pri *ctrl, int id)
-{
-	if (0 < id && id <= ctrl->sched.num_slots) {
-		ctrl->sched.timer[id - 1].callback = NULL;
-	} else if (id) {
-		pri_error(ctrl, "Asked to delete sched id %d??? num_slots=%d\n", id,
-			ctrl->sched.num_slots);
-	}
+void pri_schedule_del(struct pri *ctrl, unsigned id)
+{
+	struct pri *nfas;
+
+	if (!id) {
+		/* Disabled/unscheduled event id. */
+		return;
+	}
+	if (ctrl->sched.first_id <= id
+		&& id <= ctrl->sched.first_id + (SCHED_EVENTS_MAX - 1)) {
+		ctrl->sched.timer[id - ctrl->sched.first_id].callback = NULL;
+		return;
+	}
+	if (ctrl->nfas) {
+		/* Try to find the timer on another D channel. */
+		for (nfas = PRI_NFAS_MASTER(ctrl); nfas; nfas = nfas->slave) {
+			if (nfas->sched.first_id <= id
+				&& id <= nfas->sched.first_id + (SCHED_EVENTS_MAX - 1)) {
+				nfas->sched.timer[id - nfas->sched.first_id].callback = NULL;
+				return;
+			}
+		}
+	}
+	pri_error(ctrl,
+		"Asked to delete sched id 0x%08x??? first_id=0x%08x, num_slots=0x%08x\n", id,
+		ctrl->sched.first_id, ctrl->sched.num_slots);
 }
 
 /*!
@@ -251,22 +287,36 @@
  * \param ctrl D channel controller.
  * \param id Scheduled event id to check.
  * 0 is a disabled/unscheduled event id.
- * 1 - MAX_SCHED is a valid event id.
  * \param function Callback function to call when timeout.
  * \param data Value to give callback function when timeout.
  *
  * \return TRUE if scheduled event has the callback.
  */
-int pri_schedule_check(struct pri *ctrl, int id, void (*function)(void *data), void *data)
-{
-	if (0 < id && id <= ctrl->sched.num_slots) {
-		if (ctrl->sched.timer[id - 1].callback == function
-			&& ctrl->sched.timer[id - 1].data == data) {
-			return 1;
-		}
-	} else if (id) {
-		pri_error(ctrl, "Asked to check sched id %d??? num_slots=%d\n", id,
-			ctrl->sched.num_slots);
-	}
+int pri_schedule_check(struct pri *ctrl, unsigned id, void (*function)(void *data), void *data)
+{
+	struct pri *nfas;
+
+	if (!id) {
+		/* Disabled/unscheduled event id. */
+		return 0;
+	}
+	if (ctrl->sched.first_id <= id
+		&& id <= ctrl->sched.first_id + (SCHED_EVENTS_MAX - 1)) {
+		return ctrl->sched.timer[id - ctrl->sched.first_id].callback == function
+			&& ctrl->sched.timer[id - ctrl->sched.first_id].data == data;
+	}
+	if (ctrl->nfas) {
+		/* Try to find the timer on another D channel. */
+		for (nfas = PRI_NFAS_MASTER(ctrl); nfas; nfas = nfas->slave) {
+			if (nfas->sched.first_id <= id
+				&& id <= nfas->sched.first_id + (SCHED_EVENTS_MAX - 1)) {
+				return nfas->sched.timer[id - nfas->sched.first_id].callback == function
+					&& nfas->sched.timer[id - nfas->sched.first_id].data == data;
+			}
+		}
+	}
+	pri_error(ctrl,
+		"Asked to check sched id 0x%08x??? first_id=0x%08x, num_slots=0x%08x\n", id,
+		ctrl->sched.first_id, ctrl->sched.num_slots);
 	return 0;
 }




More information about the svn-commits mailing list