Path: utzoo!utgpu!jarvis.csri.toronto.edu!rutgers!gatech!tut.cis.ohio-state.edu!twinsun.com!eggert
From: eggert@twinsun.com (Paul Eggert)
Newsgroups: gnu.utils.bug
Subject: ``diff -'' uses uninitialized storage
Message-ID: <8908281744.AA20516@twinsun.com>
Date: 28 Aug 89 17:44:18 GMT
Sender: daemon@tut.cis.ohio-state.edu
Distribution: gnu
Organization: GNUs Not Usenet
Lines: 96

When given the ``-'' option, GNU diff 1.7 uses uninitialized storage, and
sometimes wrongly acts as though standard input is empty.  Here is a scenario
that illustrates the (machine-dependent) problem.

	% diff -c /etc/motd -
	*** /etc/motd	Mon Aug 14 22:23:16 1989
	--- Standard Input	Wed Dec 31 16:00:00 1969
	***************
	*** 1,1 ****
	- SunOS Release 4.0.3 (SDST_50) #1: Mon Aug 14 22:18:35 PDT 1989
	--- 0 ----
	% 

Here are the system calls that GNU diff executes, as reported by trace(1):

	 getpagesize () = 8192
	 brk (0x22d78) = 0
	 brk (0x24d7c) = 0
	 stat ("/etc/motd", 0xefffb28) = 0
	 open ("/etc/motd", 0, 0) = 3
	 read (3, "SunOS Release 4.0.3 (SDST_50) #1".., 63) = 63
	 read (0, "", 0) = 0
	 open ("/usr/share/lib/zoneinfo/localtim".., 0, 01) = 4
	 read (4, "".., 8192) = 754
	 close (4) = 0
	 ioctl (1, 0x40125401, 0xeffed78) = -1 ENODEV (No such device)
	 fstat (1, 0xeffeda8) = 0
	 brk (0x28d7c) = 0
	 close (3) = 0
	 write (1, "*** /etc/motd\tMon Aug 14 22:23:1".., 188) = 188
	 close (0) = 0
	 close (1) = 0
	 close (2) = 0
	 exit (1) = ?

The ``read (0, "", 0)'' is a symptom of the bug.  I traced it to the following
code in io.c's slurp():

	...
	  else if ((current->stat.st_mode & S_IFMT) == S_IFREG)
	    {
	      current->bufsize = current->stat.st_size;
	      current->buffer = (char *) xmalloc (current->bufsize + 2);
	      current->buffered_chars
		= read (current->desc, current->buffer, current->bufsize);
	      if (current->buffered_chars < 0)
		pfatal_with_name (current->name);
	    }
	  else
	    {
	      int cc;

	      current->bufsize = 4096;
	      current->buffer = (char *) xmalloc (current->bufsize + 2);
	      current->buffered_chars = 0;

	      /* Not a regular file; read it in a little at a time, growing the
		 buffer as necessary. */

When slurp() is applied to the standard input, current->stat.st_mode is
garbage; the if-test may succeed, and because st_size is zero, read() is asked
to read zero bytes (which it cheerfully does).  The problem stems from the
following code in diff.c's compare_files(), which does not initialize
stat.st_mode if inf[i].name is "-".

      if (inf[i].desc != -1
	  && strcmp (inf[i].name, "-"))
	{
	  char *filename = inf[i].name;

	  stat_result[i] = stat (filename, &inf[i].stat);
      ...

Here is a patch.  GNU diff refers to st_dev, st_ino, st_mode, st_mtime, and
st_size; instead of keeping a list of the referred-to members, it seems easier
to just zero the whole structure.

	*** old/diff.c	Aug 28 10:29:07 1989
	--- new/diff.c	Mon Aug 28 09:57:16 1989
	***************
	*** 384,391 ****

	    for (i = 0; i <= 1; i++)
	      {
	!       inf[i].stat.st_size = 0;
	!       inf[i].stat.st_mtime = 0;
		inf[i].dir_p = 0;
		stat_result[i] = 0;

	--- 384,390 ----

	    for (i = 0; i <= 1; i++)
	      {
	!       bzero(&inf[i].stat, sizeof(struct stat));
		inf[i].dir_p = 0;
		stat_result[i] = 0;