inn 2.4.3 segfaults in certain incoming.conf situations

Bill Davidsen davidsen at tmr.com
Fri Jul 20 13:44:11 UTC 2007


Julien ÉLIE wrote:
> En réponse à Bill Davidsen :
>
> [I have a patch below.  Marc, your problem is fixed -- well, I believe it is.]
>
>   
>>> 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.
>>     
>
> The external factor is what there is in memory.
>
> Indeed, as I have already written, we have:
>
> group peers {
>         patterns:       "*"
>
>         group tnib-peers {
>                 patterns:       "tnib.*"
>                 peer PEER1 {
>                 }
>         }
>
>         group ka-peers {
>                 patterns:       "ka.*"
>                 peer PEER2 {
>                 }
>         }
> }
>
> 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 should be "groups + 1" too, so when the last group is closed, group_params
> is not well defined and something bad *may* happen.
>
>
> I put:
> notice("%s closed", group_params->Label);
> when a group is closed.
> And it gives (several attempts):
>
> # One which works:
>
> Jul 20 13:52:34 news innd: ctlinnd command o:incoming.conf:
> Jul 20 13:52:35 news innd: tnib-peers closed
> Jul 20 13:52:35 news innd: ka-peers closed
> Jul 20 13:52:35 news innd:  closed
> Jul 20 13:52:35 news innd: SERVER reload incoming.conf
>
> # Another which works:
>
> Jul 20 13:44:50 news innd: ctlinnd command o:incoming.conf:
> Jul 20 13:44:51 news innd: tnib-peers closed
> Jul 20 13:44:51 news innd: U~IåWVS~Cì<~KE^HèÔžùÿ~AÃ~G^Q  closed
> Jul 20 13:44:51 news innd: SERVER reload incoming.conf
>
> # but well, I do not think global params in "peers", if any, will be passed;
> # since group_params->Label is wrong, group_params->XXX are also wrong.
>
>
> # One which fails:
>
> Jul 20 13:52:49 news innd: ctlinnd command o:incoming.conf:
> Jul 20 13:52:49 news innd: tnib-peers closed
> Jul 20 13:52:49 news innd: ka-peers closed
> Jul 20 13:52:49 news innd: SERVER '}' unexpected line 28 in /home/news/etc/incoming.conf
> Jul 20 13:52:49 news innd: SERVER reload incoming.conf
>
>
> # And one which makes innd crash:
>
> Jul 20 13:46:08 news innd: ctlinnd command o:incoming.conf:
> Jul 20 13:46:08 news innd: tnib-peers closed
> Jul 20 13:46:08 news innd: ka-peers closed
> Jul 20 13:46:09 news innfeed[4395]: ME source lost . Exiting
>
> # Oooops... Something was must have happened.
>
>
>
>
>   
>>> [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;
>>> }
>>>       
>
> We should do also increment group_params when these two conditions are
> not fulfilled.
>
> Index: rc.c
> ===================================================================
> --- rc.c        (révision 7607)
> +++ rc.c        (copie de travail)
> @@ -893,7 +893,9 @@
>            groups = xrealloc(groups, (groupcount + 4) * sizeof(REMOTEHOST));
>           maxgroup += 5;
>           group_params = groups + groupcount - 1;
> -       }
> +       } else {
> +         group_params++;
> +       }
>         group_params->Label = word;
>         group_params->Skip = groupcount > 1 ?
>           groups[groupcount - 2].Skip : default_params.Skip;
> @@ -1061,10 +1063,13 @@
>         }
>         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)
> +         if (groupcount == 0) {
>             free(groups);
> +           maxgroup = 0;
> +      }
>           else
>             group_params--;
>         }
>
>
>
>   
>> 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.
>>     
>
> You're right.
> I have also tested with two group peers and the second one is not
> well recognized (similar errors as before) if maxgroup is not reset to 0.
>
>   
Good, hopefully others will test, but it looks as if these patches 
should avoid the problem.

-- 
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