inn 2.4.3 segfaults in certain incoming.conf situations

Bill Davidsen davidsen at tmr.com
Wed Jun 27 16:38:16 UTC 2007


Julien ÉLIE wrote:
> En réponse à Marc Haber :
>   
>> I have tcpdumps of inn reloading incoming.conf yesterday and today.
>> Yesterday, it continued running, today, it crashed. In both cases, it
>> sent out the same set of DNS queries and received the same set of
>> replies (tcpdumps available on request). Everything happened in like
>> two seconds, so I do not suspect any DNS timing issues or timeouts.
>>     
>
> I too believe this is not a DNS issue but a INN issue.
>
> Indeed, I have successfully reproduced the problem with the last
> CURRENT INN 2.5.
>
> I used the « incoming.conf » file of Marc and reloaded the file
> several times.  The problem does not occur every time (I would say
> it occurs every 3 or 5 times).  That is very strange.
>
>   
That was my point, the INN code doesn't change (it might be wrong, but 
it should stay wrong), so some external factor is changing the behavior.

> Besides, innd does not crash every time (I do not know why).
>
>
> For instance:
>
> Jun 27 09:15:55 news innd: ctlinnd command o:incoming.conf:
> Jun 27 09:16:00 news innd: SERVER reload incoming.conf
> Jun 27 09:16:12 news innd: ctlinnd command o:incoming.conf:
> Jun 27 09:16:14 news innd: SERVER reload incoming.conf
> Jun 27 09:16:21 news innd: ctlinnd command o:incoming.conf:
> Jun 27 09:16:23 news innd: SERVER '}' unexpected line 236 in /home/news/etc/incoming.conf
>
>
> I also had another error, from time to time:
>
> Jun 27 09:27:29 news innd: SERVER incomplete group (geiz-XXXX) in /home/news/etc/incoming.conf line 243
>
> (where « geiz-XXXX » is the exact peer name beginning with « geiz- »)
> I do not put it because you told you do not want it to be archived.
> Note that it was always the same peer which was logged.
>
>
>
> I think the correct recognization of « group » pattern
> is bad.  Indeed, the following « incoming.conf » makes
> Jun 27 09:50:26 news innd: SERVER '}' unexpected line 15 in /home/news/etc/incoming.conf
>
>
>
> group peers {
>         patterns:       "*"
>
>         group tnib-peers {
>                 patterns:       "tnib.*"
>                 peer PEER1 {
>                 }
>         }
>
>         group ka-peers {
>                 patterns:       "ka.*"
>                 peer PEER2 {
>                 }
>         }
> }
>
>
>
> There is no problem when there is *only* « tnib-peers »
> or « ka-peers ».
>
>
> I tried to find something in innd/rc.c but I do not
> understand well everything in this file.
>
> [line 887, when a group is found]
>
> 	groupcount++;
> 	if (groupcount == 1) {
>           group_params = groups = xmalloc(sizeof(REMOTEHOST));
> 	}
> 	else if (groupcount >= maxgroup) {
>           /* alloc 5 groups */
>           groups = xrealloc(groups, (groupcount + 4) * sizeof(REMOTEHOST));
> 	  maxgroup += 5;
> 	  group_params = groups + groupcount - 1;
> 	}
>
> Especially, what is the use of « maxgroup »? (whose value is
> initially 0)
>   

I believe that is just used to determine when the size of group_params 
needs to be increased. With the initial case an exception. Whoever wrote 
it should learn how to use /*comments*/ to make it more maintainable. 
Not increasing maxgroup the first time is clever. There's a possible bug 
in the code below, after "free(groups)" I would set maxgroups back to 
zero, although the logic may prevent ever passing through this code again.

> And the last line « group_params = groups + groupcount - 1 »
> where group_params = groups + 2 - 1 = groups + 1 when reaching
> the second group?
>   


> There is a problem:
>
> group peers -> groupcount=1 so group_params = groups
> group tnib-peers -> groupcount=2 so group_params = groups + 1 and maxgroup = 5
> group ka-peers -> groupcount=2 so group_params = groups
>                   It decreases at line 1069 :
>
> 	else if (groupcount > 0 && group_params->Label != NULL) {
> 	  RCadddata(data, &infocount, K_END_GROUP, T_STRING, NULL);
> 	  group_params->Label = NULL;
> 	  groupcount--;
> 	  if (groupcount == 0)
> 	    free(groups);
> 	  else
> ---->	    group_params--;
> 	}
> 	else {
> 	  syslog(L_ERROR, RIGHT_BRACE, LogName, linecount, filename);
> 	}
>
> and when the peers group is closed, there is an error since
> groupcount=1 *but* group_params = groups - 1 (?)
> Well, it is if I am not mistaken by my reading the code (and it would
> perhaps explain why the error is not systematic since « groups - 1 »
> depends on what there is in memory).
> [I might be wrong.  Do not hesitate to correct me.]
>
>   
 

-- 
bill davidsen <davidsen at tmr.com>
  CTO TMR Associates, Inc
  Doing interesting things with small computers since 1979




More information about the inn-workers mailing list