[5520] in Moira
Re: eunice segfault
daemon@ATHENA.MIT.EDU (Mitchell E Berger)
Mon Jan 4 04:35:08 2010
Message-Id: <201001040935.o049Z0X7009551@byte-me.mit.edu>
To: Garry P Zacheiss <zacheiss@MIT.EDU>
CC: Evan Broder <broder@MIT.EDU>, "moiradev@mit.edu" <moiradev@MIT.EDU>,
"Mark W. Manley" <mmanley@MIT.EDU>
In-Reply-To: [5519] in moira
Date: Mon, 04 Jan 2010 04:35:00 -0500
From: Mitchell E Berger <mitchb@MIT.EDU>
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>
