This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
[PATCH] Fix PR gas/7059
- From: Dave Korn <dave dot korn dot cygwin at googlemail dot com>
- To: binutils at sourceware dot org
- Date: Tue, 17 Feb 2009 17:35:39 +0000
- Subject: [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;
}