This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix PR gas/7059


    Hi gang,

  HJ drew my attention to this PR, and it's a trivial extra fix in the area of
the coff long section name handling, so I figured I'd pick it up.  The
attached patch is the simple answer.

  In PR7059, the section name abbreviation "/nnnnnn" (where nnnnn is the ASCII
decimal representation of the offset into the string table at which the long
section name can be found) overflows the 8-byte s_name field and writes over
the first byte of the subsequent s_paddr field.  (This isn't a problem at
runtime as that field doesn't get initialised until after the name is written,
but it does trigger a fortify warning).  When the string table size reaches a
million, the trailing zero byte output by sprintf overwrites the low byte of
s_paddr; if the string table size exceeded ten million bytes, more bytes would
be overwritten, but I don't suppose that happens in practice).  The simple
solution is to sprintf it into a larger buffer and extract without copying the
excess NUL-terminator.

  While I was there, I added protection against the (mostly theoretical)
possibility of excessively large string tables requiring an offset that is too
large to fit in the sort name section field; we now get an error message that
looks like this:

/gnu/binutils/pr7059/obj-patched/gas $ ./.libs/as-new.exe -v -o divine-mc.o
divine-mc.s
GNU assembler version 2.19.51 (i686-pc-mingw32) using BFD version (GNU
Binutils) 2.19.51.20090217
./.libs/as-new: divine-mc.o: section
.text$_ZN6divine9algorithm5OwctyINS_9Algorithm5SetupINS_18FinalizeControllerINS_10controller4impl9PartitionEEENS_8FinalizeINS_7storage4impl15PooledPartitionEEENS9_INS_7visitor4impl5OrderINSF_6CommonEE3DF
SEEENS_9generator5DummyEEEE22findCounterexampleFromENS_5StateINSO_9ExtensionEEE:
string table overflow at offset 1000070
divine-mc.s: Assembler messages:
divine-mc.s:713168: Fatal error: can't close divine-mc.o: File too big

  (NB: I turned down the size limit to one million for this test).

  Now regtesting on i686-pc-linux-gnu x {a29k-unknown-coff alpha-freebsd
alpha-linuxecoff alpha-unknown-linux alpha-unknown-linuxecoff arm-epoc-pe
arm-unknown-coff arm-unknown-linux arm-vxworks arm-wince-pe
h8300-unknown-rtems h8500-unknown-rtems hppa-unknown-linux
hppa64-unknown-linux i370-unknown-linux i386-coff i386-msdos i386-pc-netbsdpe
i386-pc-pe i586-linux i586-pc-interix i586-pc-msdosdjgpp i586-unknown-beospe
i586-unknown-coff i686-pc-cygwin i686-pc-linux-gnu i686-pc-mingw32
i960-intel-nindy i960-unknown-coff ia64-unknown-linux m68k-coff
m68k-unknown-coff m68k-unknown-linux m68k-unknown-netbsd m88k-unknown-coff
mcore-unknown-pe mips-dec-bsd mips-unknown-ecoff mips-unknown-linux
mips-unknown-pe ns32k-unknown-netbsd or32-unknown-coff powerpc-unknown-aix5
powerpc-unknown-linux powerpc64-unknown-linux powerpcle-unknown-pe ppc-eabi
s390-unknown-linux s390x-unknown-linux sh-coff sh-unknown-linux sh-unknown-pe
sh-unknown-rtems sparc-unknown-coff sparc-unknown-linux sparc64-unknown-linux
thumb-epoc-pe tic30-unknown-coff tic54x-unknown-coff tic80-unknown-coff
vax-unknown-netbsd vax-unknown-vms vms-vax w65-unknown-coff x86_64-pc-freebsd
x86_64-pc-linux-gnu x86_64-pc-mingw32 x86_64-unknown-linux z8k-coff
z8k-unknown-coff}.  Assuming no regressions, OK to install?

bfd/ChangeLog

	PR gas/7059
	* coffcode.h (coff_write_object_contents):  Don't let the string
	table offset overflow the s_name field when using long section names.

    cheers,
      DaveK


Index: bfd/coffcode.h
===================================================================
RCS file: /cvs/src/src/bfd/coffcode.h,v
retrieving revision 1.146
diff -p -u -r1.146 coffcode.h
--- bfd/coffcode.h	16 Jan 2009 15:09:20 -0000	1.146
+++ bfd/coffcode.h	17 Feb 2009 16:39:23 -0000
@@ -3517,8 +3517,29 @@ coff_write_object_contents (bfd * abfd)
 	len = strlen (current->name);
 	if (len > SCNNMLEN)
 	  {
-	    memset (section.s_name, 0, SCNNMLEN);
-	    sprintf (section.s_name, "/%lu", (unsigned long) string_size);
+	    /* The s_name field is defined to be NUL-padded but need not be
+	       NUL-terminated.  We use a temporary buffer so that we can still
+	       sprintf all eight chars without splatting a terminating NUL
+	       over the first byte of the following member (s_paddr).  */
+	    char s_name_buf[SCNNMLEN + 1];
+
+	    /* An inherent limitation of the /nnnnnnn notation used to indicate
+	       the offset of the long name in the string table is that we
+	       cannot address entries beyone the ten million byte boundary.  */
+	    if (string_size >= 10000000)
+	      {
+		bfd_set_error (bfd_error_file_too_big);
+		(*_bfd_error_handler)
+		  (_("%B: section %s: string table overflow at offset %ld"),
+		  abfd, current->name, string_size);
+		return FALSE;
+	      }
+
+	    /* snprintf not strictly necessary now we've verified the value
+	       has less than eight ASCII digits, but never mind.  */
+	    snprintf (s_name_buf, SCNNMLEN + 1, "/%lu", (unsigned long) string_size);
+	    /* Then strncpy takes care of any padding for us.  */
+	    strncpy (section.s_name, s_name_buf, SCNNMLEN);
 	    string_size += len + 1;
 	    long_section_names = TRUE;
 	  }


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]