Subject: pkg/11273: iozone 3.9 can dump core when given '-M'
To: None <gnats-bugs@gnats.netbsd.org>
From: None <woods@weird.com>
List: netbsd-bugs
Date: 10/19/2000 17:32:12
>Number:         11273
>Category:       pkg
>Synopsis:       iozone 3.9 can dump core when given '-M'
>Confidential:   no
>Severity:       critical
>Priority:       medium
>Responsible:    pkg-manager
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Oct 19 17:32:00 PDT 2000
>Closed-Date:
>Last-Modified:
>Originator:     Greg A. Woods
>Release:        iozone 3.9
>Organization:
Planix, Inc.; Toronto, Ontario; Canada
>Environment:

System: NetBSD proven 1.4V i386


>Description:

	iozone 3.9 dumps core when given the '-M' option on some systems.

>How-To-Repeat:

	First build iozone with '-g' then:

# iozone -M
        Iozone: Performance Test of File I/O
                Version $Revision: 3.9 $
                Compiled for 32 bit mode.

        Contributors:William Norcott, Don Capps, Isom Crawford, Kirby Collins
                     Al Slater, Scott Rhine, Mike Wisner, Ken Goss
                     Steve Landherr, Brad Smith.

        Run began: Thu Oct 19 19:44:51 2000

Bus error (core dumped) 
# gdb ./iozone ./iozone.core                                      
GNU gdb 4.17
Copyright 1998 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--netbsd"...
Core was generated by `iozone'.
Program terminated with signal 10, Bus error.
Reading symbols from /usr/libexec/ld.elf_so...done.
Reading symbols from /usr/lib/libc.so.12...done.
#0  0x8049c45 in main (argc=2, argv=0xbfbfcf4c) at iozone.c:913
913                                     m++;
(gdb) where
#0  0x8049c45 in main (argc=2, argv=0xbfbfcf4c) at iozone.c:913
#1  0x80491a9 in ___start ()
(gdb) list
908                     case 'M':       /* Report machine name and OS */
909                             pi=popen("uname -a", "r");
910                             fread(reply,99,1,pi);
911                             m=reply;
912                             while(*m!='\n') /* Strip after new line */
913                                     m++;
914                             *m=0;
915                             printf("\n\tMachine = %s\n",reply);
916                             pclose(pi);
917                             break;
(gdb) print m
$1 = 0xbfbfe000 <Address 0xbfbfe000 out of bounds>
(gdb) print reply
$2 = "NetBSD proven 1.4V NetBSD 1.4V (PROVEN) #0: Fri Jul 28 20:01:18 EDT 2000     woods@proven:/work/woo"


>Fix:

	Programmers who read output from some other program,
	especially one that is simply found in the user's search path,
	into a fixed length buffer, forget to forcibly terminate that
	buffer as a C string, and then wander unchecked off the end
	of that buffer and try to change something wherever they end up
	should really be sent back to school!  :-(

	Never mind that there's no error checking being done.....

	I don't know who's responsible for these particular lines of
	code, so sorry if I offend anyone, but even such a simple
	test program doesn't deserve to be so bug ridden!

	The code should look more like this (untested hack):

		if (!(pi = popen("uname -a", "r"))) {
			perror("popen('uname -a')");
			exit(1);
		}
		if ((n = fread(reply, sizeof(reply), 1, pi) == 0)) {
			if (feof(pi))
				fprintf(stderr, "got EOF reading from 'uname -a'\n");
			else
				perror("fread('uname -a')");
			exit(1);
		}
		reply[n] = '\0';
		if (pclose(pi) == -1) {
			perror("pclose('uname -a')");
			exit(1);
		}
		if ((m = strrchr(reply, '\n')))
			*m = '\0';
		printf("\n\tMachine = %s\n", reply);
		break;

	I'd send a real patch but I'm afraid if I tried to edit that
	code I'd end up changing almost every line in the file!


	Note that since the 'uname -a' output is only ever sent to
	stdout, perhaps this code could be simplified to just:

		puts("\n\tMachine = ");
		system("uname -a");
		break;

>Release-Note:
>Audit-Trail:
>Unformatted: