[Bug 1027454] Re: misdisassembly/and seg of ARM on 32bit hosts

Bug Watch Updater 1027454 at bugs.launchpad.net
Wed Jul 25 09:14:37 UTC 2012


Launchpad has imported 6 comments from the remote bug at
http://sourceware.org/bugzilla/show_bug.cgi?id=13135.

If you reply to an imported comment from within Launchpad, your comment
will be sent to the remote bug automatically. Read more about
Launchpad's inter-bugtracker facilities at
https://help.launchpad.net/InterBugTracking.

------------------------------------------------------------------------
On 2011-08-25T22:20:28+00:00 Stephen McCamant wrote:

Created attachment 5913
Patch to fix printf argument types

The change to using bfd_vma for offset computations, described in the
fix for PR/12752 (CVS revision 1.146 of opcodes/arm-dis.c) causes the
code to crash in configurations where the size of bfd_vma is different
than the size of "int", because the code is now passing values of type
bfd_vma to a "%d" format specifier.

For instance one such code snippet in print_insn_coprocessor looks like:

   1876                     bfd_vma offset = given & 0xff;
 ...
   1892                         if (offset)
   1893                           func (stream, ", #%d]%s",
   1894                                 offset,
   1895                                 WRITEBACK_BIT_SET ? "!" : "");

When this code passes "offset" as a 64-bit value, the printf function
will interpret the low 32 bits as the %d argument, and the high 32 bits
as the %s argument, but if the value is negative, the high bits will be
equivalent to -1, which causes a segfault when used as character
pointer.

For instance, I see this when I compile a version of the binutils that
supports 32-bit x86, 64-bit AMD64, and ARM on a 32-bit x86/Linux host
system; then "int" is 32 bits, but bfd_vma is 64 bits.

This is the sort of error that is supposed to be caught by GCC's format
string checking. I see that that checking was enabled at the relevant
place in 2005 (change 1.54 to include/dis-asm.h), but then it was
disabled again (perhaps inadvertently; I don't see anything about it in
the log message) in 2007 (change 1.67 to the same file).

I've attached a proof-of-concept patch which re-enables the warnings,
and then adds casts on all the printf arguments in arm-dis.c that cause
warnings under this configuration. I've verified that this fixes all the
crashes I've seen in my configuration; objdump can now disassembly 50MB
of random bytes without crashing. However I haven't investigated whether
this problem occurs elsewhere, and I haven't checked whether this
respects all the signs correctly, which was the issue in PR/12752. (For
instance, could there be a configuration where bfd_vma is 32 bits but
int is 64?)

Reply at: https://bugs.launchpad.net/binutils/+bug/1027454/comments/0

------------------------------------------------------------------------
On 2011-08-25T22:35:01+00:00 Stephen McCamant wrote:

Here's a short command sequence to reproduce (in the appropriate
configuration):

% perl -e 'print pack("V*", 0xed69cdcb)' >/tmp/bad.insn
% objdump -b binary -m arm -EL -D /tmp/bad.insn

The expected result is something like:

   0:   ed69cdcb        stcl    13, cr12, [r9, #-812]!  ; 0xfffffcd4

whereas what I see instead is a segmentation fault.

Reply at: https://bugs.launchpad.net/binutils/+bug/1027454/comments/1

------------------------------------------------------------------------
On 2012-07-21T01:31:28+00:00 Dave Gilbert wrote:

Hi Stephen,
  I think I've also hit this case.
Watch out though with reenabling that attribute check that there are a few
broken printf's elsewhere; mips-dis.c seems to have a load of int's passed to long's.
(I'll attach a patch tomorrow or so).

(A friend managed to type random hex and hit cd45b43a which segs in this
way on ARM).

Dave

Reply at: https://bugs.launchpad.net/binutils/+bug/1027454/comments/2

------------------------------------------------------------------------
On 2012-07-21T19:27:38+00:00 Dave Gilbert wrote:

Created attachment 6543
more printf format fixes

This diff includes Stephen's diff and also patches mips (that has mismatches)
and hppa, 68k and sparc that used printf without a format string, and newer gcc's seem to be (rightly) picky about that.

make check seems OK with this, but it should get checked by guys who
know those arch.

Reply at: https://bugs.launchpad.net/binutils/+bug/1027454/comments/5

------------------------------------------------------------------------
On 2012-07-24T12:56:51+00:00 Cvs-commit wrote:

CVSROOT:	/cvs/src
Module name:	src
Changes by:	nickc at sourceware.org	2012-07-24 12:56:48

Modified files:
	opcodes        : ChangeLog arm-dis.c hppa-dis.c m68k-dis.c 
	                 microblaze-dis.c mips-dis.c ppc-dis.c 
	                 sparc-dis.c 
	include        : ChangeLog dis-asm.h 

Log message:
	PR binutils/13135
	* arm-dis.c: Add necessary casts for printing integer values.
	Use %s when printing string values.
	* hppa-dis.c: Likewise.
	* m68k-dis.c: Likewise.
	* microblaze-dis.c: Likewise.
	* mips-dis.c: Likewise.
	* ppc-dis.c: Likewise.
	* sparc-dis.c: Likewise.
	
	* dis-asm.h (fprintf_ftype): Add ATTRIBUTE_FPTR_PRINTF_2.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/opcodes/ChangeLog.diff?cvsroot=src&r1=1.1828&r2=1.1829
http://sourceware.org/cgi-bin/cvsweb.cgi/src/opcodes/arm-dis.c.diff?cvsroot=src&r1=1.151&r2=1.152
http://sourceware.org/cgi-bin/cvsweb.cgi/src/opcodes/hppa-dis.c.diff?cvsroot=src&r1=1.50&r2=1.51
http://sourceware.org/cgi-bin/cvsweb.cgi/src/opcodes/m68k-dis.c.diff?cvsroot=src&r1=1.37&r2=1.38
http://sourceware.org/cgi-bin/cvsweb.cgi/src/opcodes/microblaze-dis.c.diff?cvsroot=src&r1=1.4&r2=1.5
http://sourceware.org/cgi-bin/cvsweb.cgi/src/opcodes/mips-dis.c.diff?cvsroot=src&r1=1.91&r2=1.92
http://sourceware.org/cgi-bin/cvsweb.cgi/src/opcodes/ppc-dis.c.diff?cvsroot=src&r1=1.58&r2=1.59
http://sourceware.org/cgi-bin/cvsweb.cgi/src/opcodes/sparc-dis.c.diff?cvsroot=src&r1=1.23&r2=1.24
http://sourceware.org/cgi-bin/cvsweb.cgi/src/include/ChangeLog.diff?cvsroot=src&r1=1.580&r2=1.581
http://sourceware.org/cgi-bin/cvsweb.cgi/src/include/dis-asm.h.diff?cvsroot=src&r1=1.86&r2=1.87

Reply at: https://bugs.launchpad.net/binutils/+bug/1027454/comments/10

------------------------------------------------------------------------
On 2012-07-24T12:58:04+00:00 Nickc wrote:

Patch approved and applied.

Cheers
  Nick

Reply at: https://bugs.launchpad.net/binutils/+bug/1027454/comments/11


** Changed in: binutils
       Status: Unknown => Confirmed

** Changed in: binutils
   Importance: Unknown => Medium

-- 
You received this bug notification because you are a member of Ubuntu
Foundations Bugs, which is subscribed to binutils in Ubuntu.
https://bugs.launchpad.net/bugs/1027454

Title:
  misdisassembly/and seg of ARM on 32bit hosts

Status in binutils:
  Confirmed
Status in “binutils” package in Ubuntu:
  New

Bug description:
  This corresponds to binutils bug
  http://sourceware.org/bugzilla/show_bug.cgi?id=13135

  There are mismatched printf formats and arguments that cause segs or incorrect offsets in ARM disassembly in
  binutils 2.22; the simplest example (on 32bit x86 Quantal) from the upstream bug is:

  
  % perl -e 'print pack("V*", 0xed69cdcb)' >/tmp/bad.insn
  % objdump -b binary -m arm -EL -D /tmp/bad.insn

  Dave

  ProblemType: Bug
  DistroRelease: Ubuntu 12.10
  Package: binutils-multiarch 2.22.52.20120713-0ubuntu1
  ProcVersionSignature: Ubuntu 3.5.0-5.5-generic 3.5.0-rc7
  Uname: Linux 3.5.0-5-generic i686
  ApportVersion: 2.4-0ubuntu4
  Architecture: i386
  Date: Sat Jul 21 18:40:25 2012
  ProcEnviron:
   LANGUAGE=en_GB:en
   TERM=xterm
   PATH=(custom, no user)
   LANG=en_GB.UTF-8
   SHELL=/bin/bash
  SourcePackage: binutils
  UpgradeStatus: Upgraded to quantal on 2012-07-21 (0 days ago)

To manage notifications about this bug go to:
https://bugs.launchpad.net/binutils/+bug/1027454/+subscriptions




More information about the foundations-bugs mailing list