Date: Thu, 4 Apr 2019 00:16:58 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Dimitry Andric <dim@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r345807 - head/usr.bin/top Message-ID: <20190403234558.X1970@besplex.bde.org> In-Reply-To: <201904021801.x32I1sxX019439@repo.freebsd.org>
index | next in thread | previous in thread | raw e-mail
On Tue, 2 Apr 2019, Dimitry Andric wrote: > Author: dim > Date: Tue Apr 2 18:01:54 2019 > New Revision: 345807 > URL: https://svnweb.freebsd.org/changeset/base/345807 > > Log: > Fix regression in top(1) after r344381, causing informational messages > to no longer be displayed. This was because the reimplementation of > setup_buffer() did not copy the previous contents into any reallocated > buffer. > > Reported by: James Wright <james.wright@jigsawdezign.com> > PR: 236947 > MFC after: 3 days > > Modified: > head/usr.bin/top/display.c > > Modified: head/usr.bin/top/display.c > ============================================================================== > --- head/usr.bin/top/display.c Tue Apr 2 17:51:28 2019 (r345806) > +++ head/usr.bin/top/display.c Tue Apr 2 18:01:54 2019 (r345807) > @@ -1347,7 +1347,8 @@ i_uptime(struct timeval *bt, time_t *tod) > static char * > setup_buffer(char *buffer, int addlen) > { > - size_t len; > + size_t len, old_len; > + char *new_buffer; > > setup_buffer_bufsiz = screen_width; > if (setup_buffer_bufsiz < SETUPBUFFER_MIN_SCREENWIDTH) > @@ -1355,13 +1356,18 @@ setup_buffer(char *buffer, int addlen) > setup_buffer_bufsiz = SETUPBUFFER_MIN_SCREENWIDTH; > } > > - free(buffer); > len = setup_buffer_bufsiz + addlen + SETUPBUFFER_REQUIRED_ADDBUFSIZ; > - buffer = calloc(len, sizeof(char)); > - if (buffer == NULL) > + new_buffer = calloc(len, sizeof(char)); > + if (new_buffer == NULL) > { > errx(4, "can't allocate sufficient memory"); > } > + if (buffer != NULL) > + { > + old_len = strlen(buffer); > + memcpy(new_buffer, buffer, old_len < len - 1 ? old_len : len - 1); > + free(buffer); > + } > > - return buffer; > + return new_buffer; > } Looks like realloc() hasn't been invented yet. realloc() wouldn't clear the new part of the buffer, so a memset() or at least setting the first byte in a new buffer (starting with buffer == NULL might be needed). The above has some bugs when the new buffer is smaller the old buffer: - when old_len < len - 1, the new buffer has no space for the old buffer including its NUL terminator, so the new buffer is left unterminated after blind truncation - when old_len == len - 1, the new buffer has no space for the NUL terminator, so the new buffer is left unterminated after not overrunning it by copying the NUL terminator - when old_len > len - 1, the new buffer is NUL terminated in an obfuscated way (calloc() has filled it with NULs and the memcpy() doesn't overwrite them all). Brucehome | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190403234558.X1970>