From owner-freebsd-bugs Sun Apr 2 4: 1:54 2000 Delivered-To: freebsd-bugs@freebsd.org Received: from gidora.zeta.org.au (gidora.zeta.org.au [203.26.10.25]) by hub.freebsd.org (Postfix) with SMTP id 31D6637B6AC for ; Sun, 2 Apr 2000 04:01:49 -0700 (PDT) (envelope-from bde@zeta.org.au) Received: (qmail 13525 invoked from network); 2 Apr 2000 11:01:44 -0000 Received: from bde.zeta.org.au (203.2.228.102) by gidora.zeta.org.au with SMTP; 2 Apr 2000 11:01:44 -0000 Date: Sun, 2 Apr 2000 21:01:26 +1000 (EST) From: Bruce Evans X-Sender: bde@alphplex.bde.org To: Anatoly Vorobey Cc: freebsd-bugs@FreeBSD.ORG Subject: Re: bin/12242 : segmentation fault running /usr/bin/fmt In-Reply-To: <20000402090809.A57640@happy.checkpoint.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-bugs@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org 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