Date: Sun, 2 Apr 2000 21:01:26 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: Anatoly Vorobey <mellon@pobox.com> Cc: freebsd-bugs@FreeBSD.ORG Subject: Re: bin/12242 : segmentation fault running /usr/bin/fmt Message-ID: <Pine.BSF.4.21.0004022043500.1397-100000@alphplex.bde.org> In-Reply-To: <20000402090809.A57640@happy.checkpoint.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 2 Apr 2000, Anatoly Vorobey wrote:
> On Sun, Apr 02, 2000 at 04:52:21PM +1000, Bruce Evans wrote:
>
> > Both old_outbuf and outp are invalid after outbuf has been realloc'ed.
> > Just loading them may trap. The buffer offset should be computed
> > _before_ the realloc.
>
> Uhm, you are right. How about this? (I'm not abusing s, it's used for
> the same purpose later).
>
> --- fmt.c.orig Sat Aug 28 01:01:18 1999
> +++ fmt.c Sun Apr 2 09:05:24 2000
> @@ -443,14 +443,15 @@
> {
> register char *cp;
> register int s, t;
> -
> - if (((outp==NOSTR) ? wl : outp-outbuf + wl) >= outbuf_size) {
> - char *old_outbuf = outbuf;
> +
> + s = (outp==NOSTR) ? 0 : outp-outbuf;
> +
> + if (s + wl >= outbuf_size) {
> outbuf_size *= 2;
> outbuf = realloc(outbuf, outbuf_size);
> if (outbuf == 0)
> abort();
> - outp += outbuf-old_outbuf;
> + outp = outbuf + s;
> }
This breaks the (outp == NOSTR) case, at least if buffer expansion can
occur in that case. How about this? (untested):
s = (outp == NOSTR) ? 0 : outp - outbuf;
if (s + wl >= outbuf_size) {
outbuf_size *= 2;
outbuf = realloc(outbuf, outbuf_size);
if (outbuf == 0)
abort();
if (outp != NOSTR)
outp = outbuf + s;
}
This is still sloppy code. It doesn't check for overflow of outbuf_size,
and it asks for overflow by not using size_t for outbuf_size or ptrdiff_t
for `s'.
Bruce
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.0004022043500.1397-100000>