🚀 go-pugleaf

RetroBBS NetNews Server

Inspired by RockSolid Light RIP Retro Guy

1 total messages Started by Paul Eggert Mon, 28 Aug 1989 17:44
``diff -'' uses uninitialized storage
#380
Author: Paul Eggert
Date: Mon, 28 Aug 1989 17:44
96 lines
3385 bytes
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;
Thread Navigation

This is a paginated view of messages in the thread with full content displayed inline.

Messages are displayed in chronological order, with the original post highlighted in green.

Use pagination controls to navigate through all messages in large threads.

Back to All Threads