inn 2.4.3 segfaults in certain incoming.conf situations

[Julien ÉLIE]

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.

-- 
Julien ÉLIE

« Passion is inversely proportional to the amount of real information
  available. » (Benford's law)