Re: Patch: Initialise new file space to zero for Beos

Re: Patch: Initialise new file space to zero for Beos

[Nick Clifton]

Hi Ian,

: So I don't think you should write your patch as a change to bfd_seek.
: I think you should write a version of fseek which you put into
: libiberty, and you should fix up the libiberty configuration to build
: that version of fseek on BeOS.  It may turn out to be convenient to
: give it a different name, and to use an appropriate #define when
: building BFD, which would be fine by me.

OK, well I have a patch to libiberty (see below) that creates a new
function called zero_fseek() for BeOS hosts.  I do have two questions
though:

  1. At the moment zero_fseek is BeOS specific, should it be, or
     should there be some kind of test to determine if fseek zeroes a
     file ?  (eg create a file 512 bytes long.  Fill it with non-zero
     bytes.  Truncate it to 1 byte long.  Re-extended it to 512
     bytes.  Read in the 511 bytes and check that they are zero).

  2. How can BFD determine if it needs to use the zero_fseek function?
     At the moment the libiberty patch does not create a
     HAVE_zero_fseek define.  (I could not find a way to do this from
     inside config/mh-beos).  Should bfd_seek() just call zero_fseek
     if __BEOS__ is defined ?

Cheers
	Nick


Index: libiberty/config/mh-beos
===================================================================
RCS file: /cvs/cvsfiles/devo/libiberty/config/mh-beos,v
retrieving revision 1.2
diff -p -r1.2 mh-beos
*** mh-beos	1999/04/11 09:07:48	1.2
--- mh-beos	1999/09/20 09:49:36
***************
*** 4,7 ****
  # limit in BeOS is either increased or made user settable somehow.
  # This probably won't happen until after the DR9 release.
  
! EXTRA_OFILES = alloca.o
--- 4,10 ----
  # limit in BeOS is either increased or made user settable somehow.
  # This probably won't happen until after the DR9 release.
  
! # zero_fseek is included because the standard fseek under BeOS does
! # not initialise new file space to zero (yet).
! 
! EXTRA_OFILES = alloca.o zero_fseek.o

Index: libiberty/Makefile.in
===================================================================
RCS file: /cvs/cvsfiles/devo/libiberty/Makefile.in,v
retrieving revision 1.155
diff -p -r1.155 Makefile.in
*** Makefile.in	1999/07/26 19:44:45	1.155
--- Makefile.in	1999/09/20 09:49:36
*************** xexit.o: $(INCDIR)/libiberty.h
*** 266,268 ****
--- 266,269 ----
  xmalloc.o: $(INCDIR)/libiberty.h
  xstrdup.o: config.h $(INCDIR)/libiberty.h
  xstrerror.o: config.h $(INCDIR)/libiberty.h
+ zero_fseek.o: config.h $(INCDIR)/libiberty.h

Index: include/libiberty.h
===================================================================
RCS file: /cvs/cvsfiles/devo/include/libiberty.h,v
retrieving revision 1.25
diff -p -r1.25 libiberty.h
*** libiberty.h	1999/03/24 01:47:34	1.25
--- libiberty.h	1999/09/20 09:49:36
*************** extern int pexecute PARAMS ((const char 
*** 172,177 ****
--- 172,184 ----
  
  extern int pwait PARAMS ((int, int *, int));
  
+ /* Perform an fseek which initialises new file space to zero.  */
+ #ifdef ANSI_PROTOTYPES
+ /* Get a definition for FILE.  */
+ #include <stdio.h>
+ #endif
+ extern int zero_fseek PARAMS ((FILE *, long, int));
+ 
  #ifdef __cplusplus
  }
  #endif

*** /dev/null	Tue May  5 21:32:27 1998
--- libiberty/zero_fseek.c	Mon Sep 20 10:39:30 1999
***************
*** 0 ****
--- 1,105 ----
+ /* Perform an fseek which explicitly zeros newly created file space.
+    Copyright (C) 1999 Free Software Foundation, Inc.
+    Written by Fred Fish <fnf@www.ninemoons.com>
+    and Nick Clifton <nickc@cygnus.com>
+ 
+ This file is part of the libiberty library.
+ Libiberty is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Library General Public
+ License as published by the Free Software Foundation; either
+ version 2 of the License, or (at your option) any later version.
+ 
+ Libiberty is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ Library General Public License for more details.
+ 
+ You should have received a copy of the GNU Library General Public
+ License along with libiberty; see the file COPYING.LIB.  If
+ not, write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ Boston, MA 02111-1307, USA.  */
+ 
+ 
+ /*
+ 
+ NAME
+ 
+ 	zero_fseek -- An fseek that zeroes newly created file space.
+ 
+ SYNOPSIS
+ 
+ 	#include <stdio.h>
+ 
+ 	int zero_fseek (FILE * stream, long offset, int whence)
+ 
+ DESCRIPTION
+ 
+ 	Sets the file pointer for STREAM to OFFSET bytes from the start of
+ 	the file, current file position, or end of the file, depending upon
+ 	whether WHENCE is equal to SEEK_SET, SEEK_CUR or SEEK_END.  If the
+ 	file is extended beyonds its current size, the newly created portion
+ 	of the file is initialised with zero bytes.
+ 
+ 	Returns 0 upon success, -1 upon failure (setting errno).
+ 
+ NOTES
+ 
+ 	For most OSes this function is unnecessary, as the standard fseek
+ 	will have exactly the same behaviour.   This function is provided
+ 	for those OSes where extending a file into new file space does not
+ 	initialise the new space to zero.
+ */
+ 
+ 
+ #include "ansidecl.h"
+ #include "libiberty.h"
+ 
+ #include <stdio.h>
+ 
+ int
+ zero_fseek (stream, offset, whence)
+      FILE * stream;
+      long   offset;
+      int    whence;
+ {
+   file_ptr eof;
+   file_ptr pos;
+ 
+   /* Get the current file position, and the length of the file.  */
+   pos = ftell (stream);
+   fseek (stream, 0L, SEEK_END);
+   eof = ftell (stream);
+ 
+   /* Make everything be in terms of SEEK_SET.  */
+   if (whence == SEEK_CUR)
+     offset += pos;
+   else if (whence == SEEK_END)
+     offset += eof;
+ 
+   /* See if we are extending beyond the current end of the file.  */
+   if (eof < offset)
+     {
+       static char zeros[512];
+       file_ptr diff;
+       
+       diff = offset - eof;
+       
+       while (diff >= sizeof (zeros))
+ 	{
+ 	  fwrite (zeros, sizeof (zeros), 1, stream);
+ 	  diff -= sizeof (zeros);
+ 	}
+       
+       if (diff > 0)
+ 	fwrite (zeros, diff, 1, stream);
+       
+       /* In theory we do not need to perform the fseek now, since the fwrite
+ 	 calls will have moved the file pointer to the correct location.  In
+ 	 practice however we leave the call in, just in case something went
+ 	 wrong with the fwrites and we missed it.  (After all we are not
+ 	 checking their return codes).  */
+     }
+ 
+   /* Now perform the normal fseek, as provided by the host OS.  */
+   return fseek (stream, offset, SEEK_SET);
+ }

Re: Patch: Initialise new file space to zero for Beos

[fnf]

> : So I don't think you should write your patch as a change to bfd_seek.
> : I think you should write a version of fseek which you put into
> : libiberty, and you should fix up the libiberty configuration to build
> : that version of fseek on BeOS.  It may turn out to be convenient to
> : give it a different name, and to use an appropriate #define when
> : building BFD, which would be fine by me.
> 
> OK, well I have a patch to libiberty (see below) that creates a new
> function called zero_fseek() for BeOS hosts. 

Although this solution will work, I have serious concerns about 
performance issues.

> + int
> + zero_fseek (stream, offset, whence)
> +      FILE * stream;
> +      long   offset;
> +      int    whence;
> + {
> +   file_ptr eof;
> +   file_ptr pos;
> + 
> +   /* Get the current file position, and the length of the file.  */
> +   pos = ftell (stream);
> +   fseek (stream, 0L, SEEK_END);
> +   eof = ftell (stream);

If this function is called everytime gcc, gas, ld, etc wants to do an
fseek of any kind, then there will be at least two ftell calls and one
fseek before the actual seek that needs to be done.

As far as I know, part of the reason that BeOS does not zero the new
blocks added to a file by seeking past the end of it is that we wish
to avoid the performance overhead (small as it might be) of always
zeroing space that is probably going to be simply overwritten anyway.
This additional overhead could be an issue with the BeOS mission of
being a media oriented OS, where any overhead in I/O operations needs
to be scrutinized closely.

Given that this BeOS feature is also a potential security hole, it is
possible that the low level BeOS behavior may change.  This would make
the original patch Nick provided for bfd, as well as the current
proposed change, unnecessary.  For the moment I think we can live with
the bfd change, even if we have to maintain it as a Be local change to
the tools.  If indeed the low level behavior of BeOS will NOT change,
then the more general solution of wedging zero_fseek into the tools
via the libiberty library may be a better longterm solution.

Note that the only bug we have observed in the tools to date is the
assembler writing random data to parts of object files that are
allocated by seeking past the current end of the file and then never
overwritten with "real data".

-Fred

Re: Patch: Initialise new file space to zero for Beos

[Jeffrey A Law]

  In message < 199909201346.GAA02460@toadfish.ninemoons.com >you write:
  > Given that this BeOS feature is also a potential security hole, it is
  > possible that the low level BeOS behavior may change.  This would make
  > the original patch Nick provided for bfd, as well as the current
  > proposed change, unnecessary.  For the moment I think we can live with
  > the bfd change, even if we have to maintain it as a Be local change to
  > the tools.  If indeed the low level behavior of BeOS will NOT change,
  > then the more general solution of wedging zero_fseek into the tools
  > via the libiberty library may be a better longterm solution.
  > 
  > Note that the only bug we have observed in the tools to date is the
  > assembler writing random data to parts of object files that are
  > allocated by seeking past the current end of the file and then never
  > overwritten with "real data".
While the only bug you have observed may be the gas bug, you've got a fairly
large security hole.

What's to stop joe badguy from playing the fseek trick to grab a bunch of 
disk blocks, then examine them for things he isn't supposed to see?  I
would never trust any of my personal data to a system that didn't scrub blocks
free blocks it gave to users.

This kind of hole was fixed on most systems 10-20 years ago.

The way I see it, you have two choices.  Scrub in the OS, or scrub at the
application.  The former is secure, the latter is not.  The former works
even for clueless programmers, the latter does not.

jeff

Re: Patch: Initialise new file space to zero for Beos

[Ian Lance Taylor]

   From: fnf@toadfish.ninemoons.com
   Date: Mon, 20 Sep 1999 06:46:29 -0700 (MST)

   If this function is called everytime gcc, gas, ld, etc wants to do an
   fseek of any kind, then there will be at least two ftell calls and one
   fseek before the actual seek that needs to be done.

As a practical matter, most calls to fseek in the binutils already go
through bfd_seek, so the performance impact of changing fseek compared
to changing bfd_seek should be fairly minimal.  For most targets, gcc
never calls fseek at all.

Unix defines a certain model of file I/O: if you seek past the end of
a file, the blocks you skipped will be read as zeroes.  Expecting this
to be the case is not unique to the binutils.  I think it is
inappropriate to patch bfd_seek specifically to solve a general
problem which can afflict any Unix program.  The binutils are not the
only program which expect this type of file I/O to succeed.

   As far as I know, part of the reason that BeOS does not zero the new
   blocks added to a file by seeking past the end of it is that we wish
   to avoid the performance overhead (small as it might be) of always
   zeroing space that is probably going to be simply overwritten anyway.

I would think that the performance overhead would be very close to
zero if you can do the test within the kernel.  And, as you say, it's
an exploitable security hole.  But it's not my call.

   Note that the only bug we have observed in the tools to date is the
   assembler writing random data to parts of object files that are
   allocated by seeking past the current end of the file and then never
   overwritten with "real data".

For BFD, I believe that this is the only bug you will observe.  BFD
will always write any data which any program is expected to read; that
is, it does not optimize for skipping zeroes.  Object file formats are
defined such that some bytes are never read, and BFD generally will
not write those bytes.

Ian

Re: Patch: Initialise new file space to zero for Beos

[Ian Lance Taylor]

   Date: Mon, 20 Sep 1999 10:51:08 +0100
   From: Nick Clifton <nickc@cygnus.com>

   OK, well I have a patch to libiberty (see below) that creates a new
   function called zero_fseek() for BeOS hosts.  I do have two questions
   though:

     1. At the moment zero_fseek is BeOS specific, should it be, or
	should there be some kind of test to determine if fseek zeroes a
	file ?  (eg create a file 512 bytes long.  Fill it with non-zero
	bytes.  Truncate it to 1 byte long.  Re-extended it to 512
	bytes.  Read in the 511 bytes and check that they are zero).

I don't think a configure test is necessary.  Very few systems have
these characteristics, and for them I think it is reasonable to simply
test the host configuration name.

     2. How can BFD determine if it needs to use the zero_fseek function?
	At the moment the libiberty patch does not create a
	HAVE_zero_fseek define.  (I could not find a way to do this from
	inside config/mh-beos).  Should bfd_seek() just call zero_fseek
	if __BEOS__ is defined ?

The conventional way to set flags for a host in config/mh-* is to set
CC.  However, that is not quite right in this case, since we want to
set the flag in all cases, not depending upon the compiler.

You could add new functionality to the top level Makefile.in to
support CFLAGS to the host, which you would then pass down in
EXTRA_HOST_FLAGS or some such place.  Or you could just modify
bfd/configure.in to use zero_fseek.  Either way, I think it should be
based on a test of $host.  For example, that is how we set
USE_BINARY_FOPEN in bfd/acinclude.m4.

Ian

Re: Patch: Initialise new file space to zero for Beos

[Ian Lance Taylor]

   Date: Mon, 20 Sep 1999 10:51:08 +0100

   OK, well I have a patch to libiberty (see below) that creates a new
   function called zero_fseek() for BeOS hosts.

By the way, I just noticed that mpw.c already has a similar function,
called mpw_fseek.

Ian

Re: Patch: Initialise new file space to zero for Beos

[Ian Lance Taylor]

   Date: Sat, 18 Sep 1999 10:43:14 +0100
   From: Nick Clifton <nickc@cygnus.com>

     Does anyone have any objections to my applying the following patch ?

     It enhances bfd_seek() so that, for BeOS only, any newly created
     file space is initialised to zero.  The BeOS fseek function does not
     do this automatically (unlike fseek on other OSes), and as a result
     binary comparisons of identical files fail.  This means that
     bootstrapping under BeOS does not work.

My general attitude about this sort of patch is that BFD is not the
only Unix program which expects that an fseek past the end of a file
fills in the intervening space with zeroes.  Therefore, I think that
patching BFD just fixes one case of the problem.  The proper thing to
do is to fix the stdio library, or to otherwise provide a version of
fseek which does the right Unix-like thing.

So I don't think you should write your patch as a change to bfd_seek.
I think you should write a version of fseek which you put into
libiberty, and you should fix up the libiberty configuration to build
that version of fseek on BeOS.  It may turn out to be convenient to
give it a different name, and to use an appropriate #define when
building BFD, which would be fine by me.

Ian

Patch: Initialise new file space to zero for Beos

[Nick Clifton]

Hi Guys,

  Does anyone have any objections to my applying the following patch ?

  It enhances bfd_seek() so that, for BeOS only, any newly created
  file space is initialised to zero.  The BeOS fseek function does not
  do this automatically (unlike fseek on other OSes), and as a result
  binary comparisons of identical files fail.  This means that
  bootstrapping under BeOS does not work.

Cheers
	Nick


1999-09-18  Nick Clifton  <nickc@cygnus.com>

	* libbfd.c (bfd_seek): For BeOS, write zeros into extended
	file space.  Patch developed with Fred Fish.

Index: libbfd.c
===================================================================
RCS file: /cvs/binutils/binutils/bfd/libbfd.c,v
retrieving revision 1.6
diff -p -r1.6 libbfd.c
*** libbfd.c	1999/09/12 14:27:21	1.6
--- libbfd.c	1999/09/18 09:38:57
*************** bfd_seek (abfd, position, direction)
*** 723,728 ****
--- 723,774 ----
    if (direction == SEEK_SET && abfd->my_archive != NULL)
      file_position += abfd->origin;
  
+ #if defined __BEOS__
+   {
+     file_ptr eof;
+     file_ptr pos;
+ 
+     pos = ftell (f);
+     fseek (f, 0L, SEEK_END);
+     eof = ftell (f);
+ 
+     if (direction == SEEK_CUR)
+       {
+ 	direction = SEEK_SET;
+ 	file_position += pos;
+       }
+   
+     /* When extending a file beyond its original length the fseek() function
+        under BeOS does not initialise the new space to zero.  This means that
+        random garbage can be picked up, which in turn means that two identical
+        files assembled and linked in the same way can nevertheless still fail
+        a binary compare.  We fix this here by explicitly initialising the
+        extra space.  */
+     if (eof < file_position)
+       {
+ 	file_ptr diff;
+ 	static char zeros[512];
+       
+ 	diff = file_position - eof;
+ 
+ 	while (diff >= sizeof (zeros))
+ 	  {
+ 	    fwrite (zeros, sizeof (zeros), 1, f);
+ 	    diff -= sizeof (zeros);
+ 	  }
+ 	
+ 	if (diff > 0)
+ 	  fwrite (zeros, diff, 1, f);
+ 	
+ 	/* In theory we do not need to perform the fseek now, since the fwrite
+ 	   calls will have moved the file pointer to the correct location.  In
+ 	   practice however we leave the call in, just in case something went
+ 	   wrong with the fwrites and we missed it.  (After all we are not
+ 	   checking their return codes).  */
+       }
+   }
+ #endif
+   
    result = fseek (f, file_position, direction);
  
    if (result != 0)