svn commit: r345807 - head/usr.bin/top

[Bruce Evans]

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 at 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).

Bruce