[radvd-devel-l] radvd crash with multiple interfaces when 1 interface comes back up

[Pekka Savola]

Hello,

During today's testing I found a similar problem that Teemu fixes last 
year.

When you have multiple interfaces (IgnoreIfMissing=on, and one 
interface is down, when you bring it back up, radvd crashes with a 
segfault in timer.c:152 ("tm->prev->next = tm->next;").

Interestingly enough, I don't see this happening when running radvd as 
root.

I played around with this a bit, but could not figure out the real 
root cause.  I'm tempted to work around this by applying the following 
work-around fix.  Anyone else interested in looking into this a bit 
more?

On Thu, 27 Aug 2009, Teemu Torma wrote:
> On Thursday 27 August 2009 15:20:23 Pekka Savola wrote:
>> Anyone interested and have time to look at this?  I'll try to figure
>> out more when I have a bit more time.
>
> I did that yesterday and I have been running the radvd with attached patch
> which seems to cure the problem.
>
> The issue is that when interface comes back up, send_ra will call
> reload_config which will reset timers for all interfaces.  However, the
> current timer event will continue processing as usual and at the end will
> reset the current timer.  Since reload_config already did that, timer list
> pointers will be screwed and at least in my case cause an infinite loop.
>
> I haven't tried this, but it might suffice to ignore already existing timer
> entry in set_timer (i.e, tm->next && tm->prev).
>
> I don't know the code well enough to say if this is a good way to deal with
> the issue.
>
> Teemu
-------------- next part --------------
Index: timer.c
===================================================================
RCS file: /work/cvsroot/radvd/timer.c,v
retrieving revision 1.9
diff -u -r1.9 timer.c
--- timer.c	18 Oct 2005 19:17:29 -0000	1.9
+++ timer.c	4 May 2010 09:14:52 -0000
@@ -147,7 +147,7 @@
 	 */
 
 	/* unused timers are initialized to LONG_MAX so we skip them */
-	while (tm->expires.tv_sec != LONG_MAX && check_time_diff(tm, tv))
+	while (tm->next && tm->prev && tm->expires.tv_sec != LONG_MAX && check_time_diff(tm, tv))
 	{		
 		tm->prev->next = tm->next;
 		tm->next->prev = tm->prev;
Index: CHANGES
===================================================================
RCS file: /work/cvsroot/radvd/CHANGES,v
retrieving revision 1.98
diff -u -r1.98 CHANGES
--- CHANGES	10 Mar 2010 07:57:36 -0000	1.98
+++ CHANGES	4 May 2010 09:14:52 -0000
@@ -1,5 +1,8 @@
 $Id: CHANGES,v 1.98 2010/03/10 07:57:36 psavola Exp $
 
+04/05/2010	Fix a segmentation fault on reload_config() timer list
+		corruption that only occurs with multiple interfaces.
+
 10/03/2010	Fix radvd skipping multiple interfaces when
                 UnicastOnly is on or AdvSendAdvert is off.
 		This got broken in radvd 1.3. Patch from Cedric BAIL.