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