bin/68527: Resizing 'top' running in a terminal to one column width causes a seg. fault in 'top'

[Giorgos Keramidas]

The following reply was made to PR bin/68527; it has been noted by GNATS.

From: Giorgos Keramidas <keramida at ceid.upatras.gr>
To: Daniel Simeone <traser at isn.net>
Cc: bug-followup at freebsd.org
Subject: Re: bin/68527: Resizing 'top' running in a terminal to one column width causes a seg. fault in 'top'
Date: Thu, 1 Jul 2004 00:24:31 +0300

 On 2004-06-30 19:21, Daniel Simeone <traser at isn.net> wrote:
 > Start up an xterm, an aterm or another similar graphical terminal
 > (does not affect konsole, as konsole does not allow such radical
 > resizing), run the program 'top,' resize to a width of one-column,
 > and 'top' will segfault.
 
 I can repeat this.  A backtrace from top compiled with CFLAGS="-ggdb" is
 shown below:
 
 : giorgos at gothmog:/tmp$ gdb ./top top.core
 : GNU gdb 6.1.1 [FreeBSD]
 : Copyright 2004 Free Software Foundation, Inc.
 : GDB is free software, covered by the GNU General Public License, and you are
 : welcome to change it and/or distribute copies of it under certain conditions.
 : Type "show copying" to see the conditions.
 : There is absolutely no warranty for GDB.  Type "show warranty" for details.
 : This GDB was configured as "i386-marcel-freebsd"...
 : Core was generated by `top'.
 : Program terminated with signal 11, Segmentation fault.
 : Reading symbols from /lib/libncurses.so.5...done.
 : Loaded symbols for /lib/libncurses.so.5
 : Reading symbols from /lib/libm.so.2...done.
 : Loaded symbols for /lib/libm.so.2
 : Reading symbols from /lib/libkvm.so.2...done.
 : Loaded symbols for /lib/libkvm.so.2
 : Reading symbols from /lib/libc.so.5...done.
 : Loaded symbols for /lib/libc.so.5
 : Reading symbols from /libexec/ld-elf.so.1...done.
 : Loaded symbols for /libexec/ld-elf.so.1
 : #0  0x0804f01b in strecpy (to=0x800 <Address 0x800 out of bounds>, from=0x80534a0 "")
 :     at /usr/src/contrib/top/utils.c:153
 : 153         while ((*to++ = *from++) != '\0');
 : (gdb) bt
 : #0  0x0804f01b in strecpy (to=0x800 <Address 0x800 out of bounds>, from=0x80534a0 "")
 :     at /usr/src/contrib/top/utils.c:153
 : #1  0x0804aac7 in i_process (line=0, thisline=0x80534a0 "") at /usr/src/contrib/top/display.c:697
 : #2  0x0804df0c in main (argc=1, argv=0xbfbfe974) at /usr/src/contrib/top/top.c:624
 : (gdb)
 
 The bug is caused by various parts of the top source that set the
 variable screen_width to (columns - 1) where `columns' is the width of
 the current terminal.  This subtraction is probably an attempt to avoid
 messing up the output window on terminals that have automatic right
 margin and wrapping capabilities.  It has a nasty side effect though in
 display.c near line 117 where a buffer is allocated to hold a memory
 image of the screen window:
 
     display.c:117:    screenbuf = (char *)malloc(lines * display_width);
 
 When the terminal width is 1 column, screen_width is zero (one less).
 malloc() is called with an argument of zero and returns whatever the
 current settings of /etc/malloc.conf or the default of malloc() happens
 to be set to (either a NULL pointer or a minimal allocation area).
 
 Of course writing to this buffer, which top later does, is wrong.  The
 crash stops if display_width never drops to 0 columns:
 
 --- patch start ---
 Index: display.c
 ===================================================================
 RCS file: /home/ncvs/src/contrib/top/display.c,v
 retrieving revision 1.7
 diff -u -r1.7 display.c
 --- display.c	11 Aug 2002 18:37:25 -0000	1.7
 +++ display.c	30 Jun 2004 21:02:46 -0000
 @@ -108,7 +108,7 @@
         modules make static allocations based on MAX_COLS and we don't want
         to run off the end of their buffers */
      display_width = screen_width;
 -    if (display_width >= MAX_COLS)
 +    if (display_width <= 0 || display_width >= MAX_COLS)
      {
  	display_width = MAX_COLS - 1;
      }
 --- patch end ---
 
 A more serious attempt at fixing top to work correctly with any terminal
 type would require looking at "am" and "YE" capabilities, fixing
 screen_width and display_width to one less column only if absolutely
 necessary.  This patch doesn't fix the other 'hidden' bug of top that
 results in garbage being printed for too low values of screen_width,
 but at least it avoids the crashes.
 
 - Giorgos