From owner-cvs-all@FreeBSD.ORG Sun Apr 15 08:30:09 2007 Return-Path: X-Original-To: cvs-all@freebsd.org Delivered-To: cvs-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id B54B316A47A; Sun, 15 Apr 2007 08:30:09 +0000 (UTC) (envelope-from stas@FreeBSD.org) Received: from com1.ht-systems.ru (com1.ht-systems.ru [83.97.104.204]) by mx1.freebsd.org (Postfix) with ESMTP id D186513C45E; Sun, 15 Apr 2007 08:30:08 +0000 (UTC) (envelope-from stas@FreeBSD.org) Received: from [83.97.105.114] (helo=phonon.SpringDaemons.com ident=postfix) by com1.ht-systems.ru with esmtpa (Exim 4.62) (envelope-from ) id 1Hd07Z-0007yg-N7; Sun, 15 Apr 2007 12:30:06 +0400 Received: from localhost (localhost [127.0.0.1]) by phonon.SpringDaemons.com (Postfix) with SMTP id 4A15911B81; Sun, 15 Apr 2007 12:29:26 +0400 (MSD) Date: Sun, 15 Apr 2007 12:29:26 +0400 From: Stanislav Sedov To: Bruce Evans Message-Id: <20070415122926.a1b0ed46.stas@FreeBSD.org> Organization: The FreeBSD Project X-Mailer: carrier-pigeon X-Voice: +7 916 849 20 23 X-XMPP: ssedov@jabber.ru X-ICQ: 208105021 X-Yahoo: stanislav_sedov X-PGP-Fingerprint: F21E D6CC 5626 9609 6CE2 A385 2BF5 5993 EB26 9581 X-University: MEPhI Mime-Version: 1.0 Content-Type: multipart/signed; protocol="application/pgp-signature"; micalg="PGP-SHA1"; boundary="Signature=_Sun__15_Apr_2007_12_29_26_+0400_f8scMTifxtccmOai" X-Spam-Flag: SKIP Cc: cvs-src@freebsd.org, pav@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org, "Simon L. Nielsen" Subject: Fw: Re: cvs commit: src/contrib/top top.X top.c top.h src/usr.bin/top machine.c X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 15 Apr 2007 08:30:09 -0000 --Signature=_Sun__15_Apr_2007_12_29_26_+0400_f8scMTifxtccmOai Content-Type: multipart/mixed; boundary="Multipart=_Sun__15_Apr_2007_12_29_26_+0400_m/jNIVDiXUj32.PK" --Multipart=_Sun__15_Apr_2007_12_29_26_+0400_m/jNIVDiXUj32.PK Content-Type: text/plain; charset=US-ASCII Content-Disposition: inline Content-Transfer-Encoding: 7bit Begin forwarded message: Date: Sat, 14 Apr 2007 21:28:36 +0400 From: Stanislav Sedov To: Ruslan Ermilov Cc: Alfred Perlstein Subject: Re: cvs commit: src/contrib/top top.X top.c top.h src/usr.bin/top machine.c On Sat, 14 Apr 2007 21:14:39 +0400 Ruslan Ermilov mentioned: > On Sat, Apr 14, 2007 at 06:46:27PM +0400, Stanislav Sedov wrote: > > On Sat, 14 Apr 2007 18:17:30 +0400 > > Stanislav Sedov 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. > > > > > > > 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. > > > You cannot overwrite the code due to hardware page-level > protection. > Yeah, i've forgot about it. The patch attached makes top display buffers allocation fully dynamic, so the will be no bogus limit of 128 characters and it will scale to the entire available width. Also, I've added additional missed checks for NULL pointers after malloc. Alfred, can you review? PS: the "top" internal structure is still a bit ugly, probably it requires a major rewrite. I could work on this, if nobody minds. -- Stanislav Sedov ST4096-RIPE -- Stanislav Sedov ST4096-RIPE --Multipart=_Sun__15_Apr_2007_12_29_26_+0400_m/jNIVDiXUj32.PK Content-Type: text/x-diff; name="top.diff" Content-Disposition: attachment; filename="top.diff" Content-Transfer-Encoding: quoted-printable Index: usr.bin/top/machine.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /home/ncvs/src/usr.bin/top/machine.c,v retrieving revision 1.79 diff -u -r1.79 machine.c --- usr.bin/top/machine.c 14 Apr 2007 10:16:52 -0000 1.79 +++ usr.bin/top/machine.c 14 Apr 2007 17:16:45 -0000 @@ -647,8 +647,6 @@ return ((caddr_t)&handle); } =20 -static char fmt[128]; /* static area where result is built */ - char * format_next_process(caddr_t handle, char *(*get_userid)(int), int flags) { @@ -662,9 +660,20 @@ struct rusage ru, *rup; long p_tot, s_tot; char *proc_fmt, thr_buf[6]; - char *cmdbuf =3D NULL; + size_t cmdlen; + char *cmdbuf =3D NULL, *fmt; char **args; =20 + if (screen_width <=3D 0) + return NULL; + + /* Allocate buffer to store the result */ + fmt =3D (char *)malloc(screen_width + 1); + if (fmt =3D=3D NULL) { + warn("malloc(%d)", screen_width + 1); + return NULL; + } + /* find and remember the next proc structure */ hp =3D (struct handle *)handle; pp =3D *(hp->next_proc++); @@ -726,25 +735,31 @@ break; } =20 - cmdbuf =3D (char *)malloc(cmdlengthdelta + 1); + /* Find maximum command line lenght */ + if (screen_width > cmdlengthdelta) + cmdlen =3D screen_width - cmdlengthdelta + 100; + else + cmdlen =3D 0; + + cmdbuf =3D (char *)malloc(cmdlen + 1); if (cmdbuf =3D=3D NULL) { - warn("malloc(%d)", cmdlengthdelta + 1); + warn("malloc(%d)", cmdlen + 1); return NULL; } =20 if (!(flags & FMT_SHOWARGS)) { - snprintf(cmdbuf, cmdlengthdelta, "%s", pp->ki_comm); + snprintf(cmdbuf, cmdlen, "%s", pp->ki_comm); } else if (pp->ki_args =3D=3D NULL || - (args =3D kvm_getargv(kd, pp, cmdlengthdelta)) =3D=3D NULL || !(*args= )) - snprintf(cmdbuf, cmdlengthdelta, "[%s]", pp->ki_comm); + (args =3D kvm_getargv(kd, pp, cmdlen)) =3D=3D NULL || !(*args)) + snprintf(cmdbuf, cmdlen, "[%s]", pp->ki_comm); else { char *src, *dst, *argbuf; char *cmd; size_t argbuflen; size_t len; =20 - argbuflen =3D cmdlengthdelta * 4; + argbuflen =3D cmdlen * 4; argbuf =3D (char *)malloc(argbuflen + 1); if (argbuf =3D=3D NULL) { warn("malloc(%d)", argbuflen + 1); @@ -777,10 +792,10 @@ *dst =3D '\0'; =20 if (strcmp(cmd, pp->ki_comm) !=3D 0 ) - snprintf(cmdbuf, cmdlengthdelta, "%s (%s)",argbuf, \ + snprintf(cmdbuf, cmdlen, "%s (%s)",argbuf, \ pp->ki_comm); else - strlcpy(cmdbuf, argbuf, cmdlengthdelta); + strlcpy(cmdbuf, argbuf, cmdlen); =20 free(argbuf); } @@ -802,7 +817,7 @@ p_tot =3D rup->ru_inblock + rup->ru_oublock + rup->ru_majflt; s_tot =3D total_inblock + total_oublock + total_majflt; =20 - sprintf(fmt, io_Proc_format, + snprintf(fmt, screen_width, io_Proc_format, pp->ki_pid, namelength, namelength, (*get_userid)(pp->ki_ruid), rup->ru_nvcsw, @@ -812,9 +827,7 @@ rup->ru_majflt, p_tot, s_tot =3D=3D 0 ? 0.0 : (p_tot * 100.0 / s_tot), - screen_width > cmdlengthdelta ? - screen_width - cmdlengthdelta : 0, - printable(cmdbuf)); + cmdlen, printable(cmdbuf)); =20 free(cmdbuf); =20 @@ -829,7 +842,7 @@ snprintf(thr_buf, sizeof(thr_buf), "%*d ", sizeof(thr_buf) - 2, pp->ki_numthreads); =20 - sprintf(fmt, proc_fmt, + snprintf(fmt, screen_width, proc_fmt, pp->ki_pid, namelength, namelength, (*get_userid)(pp->ki_ruid), thr_buf, @@ -841,8 +854,7 @@ smpmode ? pp->ki_lastcpu : 0, format_time(cputime), ps.wcpu ? 100.0 * weighted_cpu(pct, pp) : 100.0 * pct, - screen_width > cmdlengthdelta ? screen_width - cmdlengthdelta : 0, - printable(cmdbuf)); + cmdlen, printable(cmdbuf)); =20 free(cmdbuf); =20 Index: contrib/top/display.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /home/ncvs/src/contrib/top/display.c,v retrieving revision 1.9 diff -u -r1.9 display.c --- contrib/top/display.c 19 May 2005 13:34:19 -0000 1.9 +++ contrib/top/display.c 14 Apr 2007 17:16:46 -0000 @@ -68,6 +68,10 @@ static char **memory_names; static char **swap_names; =20 +static char *procstates_buffer; +static char *memory_buffer; +static char *swap_buffer; + static int num_procstates; static int num_cpustates; static int num_memory; @@ -104,14 +108,8 @@ =20 if (lines < 0) lines =3D 0; - /* we don't want more than MAX_COLS columns, since the machine-depende= nt - modules make static allocations based on MAX_COLS and we don't want - to run off the end of their buffers */ + display_width =3D screen_width; - if (display_width >=3D MAX_COLS) - { - display_width =3D MAX_COLS - 1; - } =20 /* now, allocate space for the screen buffer */ screenbuf =3D (char *)malloc(lines * display_width); @@ -142,6 +140,11 @@ /* only do the rest if we need to */ if (lines > -1) { + /* allocate line buffers */ + procstates_buffer =3D (char *)malloc(screen_width + 1); + memory_buffer =3D (char *)malloc(screen_width + 1); + swap_buffer =3D (char *)malloc(screen_width + 1); + /* save pointers and allocate space for names */ procstate_names =3D statics->procstate_names; num_procstates =3D string_count(procstate_names); @@ -174,6 +177,11 @@ } } =20 + /* Check if memory was allocated */ + if (!procstates_buffer || !memory_buffer || !swap_buffer || !lprocstat= es || + !lswap || !lcpustates || !cpustate_columns || !lmemory) + return (-1); + /* return number of lines available */ return(lines); } @@ -281,7 +289,6 @@ } =20 static int ltotal =3D 0; -static char procstates_buffer[MAX_COLS]; =20 /* * *_procstates(total, brkdn, names) - print the process summary line @@ -323,7 +330,6 @@ int *brkdn; =20 { - static char new[MAX_COLS]; register int i; =20 /* update number of processes only if it has changed */ @@ -358,10 +364,16 @@ /* see if any of the state numbers has changed */ if (memcmp(lprocstates, brkdn, num_procstates * sizeof(int)) !=3D 0) { + char *new =3D (char *)malloc(display_width + 1); + if (new =3D=3D NULL) { + warn("malloc(%d)", display_width + 1); + return; + } /* format and update the line */ summary_format(new, brkdn, procstate_names); line_update(procstates_buffer, new, x_brkdn, y_brkdn); memcpy(lprocstates, brkdn, num_procstates * sizeof(int)); + free(new); } } =20 @@ -516,8 +528,6 @@ * for i_memory ONLY: cursor is on the previous line */ =20 -char memory_buffer[MAX_COLS]; - i_memory(stats) =20 int *stats; @@ -536,11 +546,18 @@ int *stats; =20 { - static char new[MAX_COLS]; + + char *new =3D (char *)malloc(display_width + 1); + if (new =3D=3D NULL) { + warn("malloc(%d)", display_width + 1); + return; + } =20 /* format the new line */ summary_format(new, stats, memory_names); line_update(memory_buffer, new, x_mem, y_mem); + + free(new); } =20 /* @@ -550,8 +567,6 @@ * for i_swap ONLY: cursor is on the previous line */ =20 -char swap_buffer[MAX_COLS]; - i_swap(stats) =20 int *stats; @@ -570,11 +585,17 @@ int *stats; =20 { - static char new[MAX_COLS]; + + char *new =3D (char *)malloc(display_width + 1); + if (new =3D=3D NULL) { + warn("malloc(%d)", display_width + 1); + return; + } =20 /* format the new line */ summary_format(new, stats, swap_names); line_update(swap_buffer, new, x_swap, y_swap); + free(new); } =20 /* @@ -713,6 +734,9 @@ register char *p; register char *base; =20 + if (thisline =3D=3D NULL) + return; + /* make sure we are on the correct line */ while (lastline < y_procs + line) { @@ -732,6 +756,8 @@ =20 /* zero fill the rest of it */ memzero(p, display_width - (p - base)); + + free(thisline); } =20 u_process(line, newline) @@ -744,6 +770,9 @@ register int screen_line =3D line + Header_lines; register char *bufferline; =20 + if (newline =3D=3D NULL) + return; + /* remember a pointer to the current line in the screen buffer */ bufferline =3D &screenbuf[lineindex(line)]; =20 @@ -779,6 +808,9 @@ { line_update(bufferline, newline, 0, line + Header_lines); } + + free(newline); + } =20 u_endscreen(hi) --Multipart=_Sun__15_Apr_2007_12_29_26_+0400_m/jNIVDiXUj32.PK-- --Signature=_Sun__15_Apr_2007_12_29_26_+0400_f8scMTifxtccmOai Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (FreeBSD) iD8DBQFGIeJmK/VZk+smlYERAo1jAJ0Wg6tJlmrW8cHHYcy3BTuHzqcgJQCdFkpm dh3o0OsrtmlCgECJWl9WsAw= =f/2U -----END PGP SIGNATURE----- --Signature=_Sun__15_Apr_2007_12_29_26_+0400_f8scMTifxtccmOai--