[5521] in Moira
Re: eunice segfault
daemon@ATHENA.MIT.EDU (Garry P Zacheiss)
Mon Jan 4 11:33:13 2010
From: Garry P Zacheiss <zacheiss@MIT.EDU>
To: Mitchell E Berger <mitchb@mit.edu>
CC: Evan Broder <broder@mit.edu>, "moiradev@mit.edu" <moiradev@mit.edu>,
"Mark W. Manley" <mmanley@mit.edu>
Date: Mon, 4 Jan 2010 11:33:05 -0500
Message-ID: <0B3BC623-2463-4A67-A3E0-7A9E08BF9938@mit.edu>
In-Reply-To: <201001040935.o049Z0X7009551@byte-me.mit.edu>
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Good catch, I checked in a blanche fix and the eunice change.
Garry
On Jan 4, 2010, at 4:35 AM, Mitchell E Berger wrote:
>
> Hi Garry,
>
> Evan asked about this on zephyr earlier, and I got around to debugging
> it before looking at the mail here and seeing that you had already
> found the problem and produced the same patch I was about to offer.
> However, while investigating this, I came across what I think is a
> related bug in blanche, explained in this zephyr:
>
> debathena / eunice / mitchb
> As for blanche, I think it has a bug related to all of this in that
> argv has length 14 when it's passed as save_list_info()'s callarg by main
> in preparation to call update_list (which is given an argc of 16, and
> save_list_info does shift 15 things over by one slot). Presently, glin
> returns 17 fields, and the last two version bumps of both the glin and
> ulis queries increased the involved number of fields by 2. I kind of
> suspect the array sizes and loop counters simply didn't all get updates
> in sync.
>
> I haven't seen blanche segfault as a result, but the buggy eunice also
> doesn't seem to segfault on the scripts.mit.edu servers or on Solaris
> Athena 9 machines. I think the 14 on blanche.c:493 was intended to
> be changed to 16 when mailman support was added.
>
> Mitch
>
>> Actually, all of that incrementing is intentional (although the segfault
>> obviously isn't). This patch should fix the problem; I can't reproduce the
>> segfault with it applied.
>>
>> Garry
>>
>> Index: eunice.c
>> ===================================================================
>> --- eunice.c (revision 3943)
>> +++ eunice.c (working copy)
>> @@ -59,7 +59,7 @@
>> int status, success;
>> char **arg = argv;
>> char *uargv[2];
>> - char *pargv[PRN_END];
>> + char *pargv[PRN_END + 1];
>> char *membervec[4];
>> struct member *memberstruct;
>> char *server = NULL, *p;
>> @@ -299,7 +299,7 @@
>> usage(argv);
>> }
>>
>> - for (i = 0; i < PRN_END; i++)
>> + for (i = 0; i < PRN_END + 1; i++)
>> pargv[i] = NULL;
>>
>> /* check for name conflicts. */
>> @@ -545,10 +545,10 @@
>> int show_printer_info(char *queuename)
>> {
>> char hwaddr[20];
>> - char *pargv[PRN_END];
>> + char *pargv[PRN_END + 1];
>> int status, banner, i;
>>
>> - for (i = 0; i < PRN_END; i++)
>> + for (i = 0; i < PRN_END + 1; i++)
>> pargv[i] = NULL;
>>
>> memset (hwaddr,'\0',sizeof(hwaddr));
>>
>> On Jan 3, 2010, at 1:02 PM, Evan Broder wrote:
>>
>>> Hello again -
>>> I noticed today that eunice segfaulted when printing out the "last
>>> mod" line of its status.
>>>
>>> It looks like this is because save_printer_info increments the
>>> destination index when it's copying the information from Moira. To
>>> compensate for this, all of the indices into the returned array were
>>> also incremented by one, but in the case of PRN_MODWITH, this put the
>>> index out past the end of the array, causing a segfault.
>>>
>>> eunice-segfault.diff removes the extra index increment everywhere it's
>>> used, and corrects the segfault.
>>>
>>> Thanks,
>>> - Evan
>>> <eunice-segfault.diff>
>
