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