cvs commit: src/contrib/top top.X top.c top.h src/usr.bin/top machine.c

[Bruce Evans]

On Sat, 14 Apr 2007, Stanislav Sedov wrote:

> On Sat, 14 Apr 2007 18:17:30 +0400
> Stanislav Sedov <stas at FreeBSD.org> mentioned:
>
>> Well, not quite right. If you screen is wider then 128 symbols, there
>> could be an overflow, since the row buffer is 128 bytes length.
>>
>> I have not touched any limits, just replaced the string it displays. So
>> there can be overflow with patch or without it, if both the command
>> name and screen width is wider then 128.

It couldn't overflow before, because the command name width was limited
by sizeof(pp->ki_comm) = 20, and other field widths are mostly fixed and
must add up to less than about 60 to leave space for almost 20-character
command names on 80-column terminals.

> That's even better. In contrib src display.c it always NULL-terminate
> string at the screen_width point, so it's always write '\0' to the
> unknown area if screen_width is larger than 128. Hopefully, the storage
> is allocated last in .data section, so it doesn't overwrite any
> important data and code.

No, it NUL-terminates at display_width = min(screen_width, MAX_COLS - 1).
display_width's only reason for existence is to cache the result of
this min() so as to do the truncation efficiently as well as safely.

MAX_COLS is bogusly named.  It is the buffer size for various buffers. It
is certainly not the maximum nomber of columns, but is effectively 1
less than that, since if the physical display width is > MAX_COLS - 1
then the truncation has the non-null effect of limiting all output to
MAX_COLS - 1 columns.

screen_width is slightly bogusly named.  It is normally the physical
screen width less 1.  Here 1 is subtracted to avoid problems with line
wrap, so this name is OK if you think of the screen width as being the
usable screen width, but that width as actually given by the
display_width variable so the screen_width variable shouldn't be used
for it too.  I think screen_width should be the physical screen width
and private to screen.c and maybe display.c.  The only correct use of
screen_width is currently to initialize display_width from it.  Other
uses give the buffer overrun in top.c and completely wrong right
justification of some things in display.c in cases where
screen_width != display_width.  Right justification to display_width
would be less wrong.

The interaction of the subtraction of 1 from screen_width with the
subtraction of 1 from MAX_COLS is a bit confusing and gives potential
buffer overrun for the default initialization of screen_width to
MAX_COLS (should be to MAX_COLS - 1).  This initialization is used if
top can't figure out the terminal width.  Then it has the same effect
as a terminal of width MAX_COLS + 1 -- there is no problem in display.c
since display_width = MAX_COLS - 1 is used there, but the printf() in
top.c can overrun by 1 now that the command name width can be large.

Bruce