public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: Binutils <binutils@sourceware.org>
Subject: [PATCH v2] ELF32: don't silently truncate relocation addends
Date: Mon, 21 Mar 2022 15:16:58 +0100	[thread overview]
Message-ID: <20dd63c2-09c7-6308-c8a6-d2ecd9726430@suse.com> (raw)
In-Reply-To: <d69de13e-e169-8ac2-62a3-8b28b8fad0f3@suse.com>

At least x86-64's x32 sub-mode and RISC-V's 32-bit mode calculate
addends as 64-bit values, but store them in signed 32-bit fields when
generating the file without encountering any earlier error. When the
relocated field is a 64-bit one, the value resulting after processing
the relocation record when linking (or the latest when loading) may
thus be wrong due to the truncation.

With the code change in place, one x32 testcase actually triggers the
new diagnostic. That one case of too large a (negative) addend is being
adjusted alongside the addition of a new testcase to actually trigger
the new error. (Note that due to internal BFD behavior the relocation in
.data doesn't get processed anymore after the errors in .text.)

Note that in principle it is possible to express 64-bit relocations in
ELF32, but this would require .rel relocations, i.e. with the addend
stored in the 64-bit field being relocated. But I guess it would be a
lot of effort for little gain to actually support this.
---
v2: Restrict to RELA relocations, as was originally intended, and to
    relocations of fields wider than 32 bits.
---
I guess this may not be the only place which wants such a check, but at
least this looks to be one central place.

Originally I had the conditional as

  if ((int64_t) ptr->addend != (int32_t) ptr->addend)

but I thought it would be better to avoid the non-portability of the
conversion from unsigned to signed types. Afaict the way it is now it
ought to be portable, as it involves only a signed->unsigned conversion.

--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -997,6 +997,19 @@ elf_write_relocs (bfd *abfd, asection *s
 	  return;
 	}
 
+#if defined(BFD64) && ARCH_SIZE == 32
+      if (rela_hdr->sh_type == SHT_RELA
+	  && ptr->howto->bitsize > 32
+	  && ptr->addend - INT32_MIN > UINT32_MAX)
+	{
+	  _bfd_error_handler (_("%pB: %pA+%"BFD_VMA_FMT"x: "
+				"relocation addend %"BFD_VMA_FMT"x too large"),
+			      abfd, sec, ptr->address, ptr->addend);
+	  *failedp = true;
+	  bfd_set_error (bfd_error_bad_value);
+	}
+#endif
+
       src_rela.r_offset = ptr->address + addr_offset;
       src_rela.r_info = ELF_R_INFO (n, ptr->howto->type);
       src_rela.r_addend = ptr->addend;
--- a/gas/testsuite/gas/i386/ilp32/ilp32.exp
+++ b/gas/testsuite/gas/i386/ilp32/ilp32.exp
@@ -38,6 +38,7 @@ if [expr ([istarget "i*86-*-*"] || [ista
     }
 
     run_list_test "reloc64" "--defsym _bad_=1"
+    run_list_test "reloc-2"
 
     set ASFLAGS "$old_ASFLAGS"
 }
--- /dev/null
+++ b/gas/testsuite/gas/i386/ilp32/reloc-2.l
@@ -0,0 +1,4 @@
+.*: \.text\+2:.*addend.*too large.*
+.*: \.text\+b:.*addend.*too large.*
+.*: Assembler messages:
+.*: Fatal error: .*
--- /dev/null
+++ b/gas/testsuite/gas/i386/ilp32/reloc-2.s
@@ -0,0 +1,7 @@
+	.text
+_start:
+	movabs	$x+0x123456789, %rax
+	movabs	x+0x123456789, %eax
+
+	.data
+	.quad x+0x123456789
--- a/gas/testsuite/gas/i386/ilp32/reloc64.d
+++ b/gas/testsuite/gas/i386/ilp32/reloc64.d
@@ -61,7 +61,8 @@ Disassembly of section \.text:
 .*[ 	]+R_X86_64_TPOFF32[ 	]+xtrn
 .*[ 	]+R_X86_64_TPOFF32[ 	]+xtrn
 .*[ 	]+R_X86_64_TPOFF32[ 	]+xtrn
-.*[ 	]+R_X86_64_64[ 	]+xtrn\+0x1
+.*[ 	]+R_X86_64_64[ 	]+xtrn\+0x7fffffff
+.*[ 	]+R_X86_64_64[ 	]+xtrn\-0x80000000
 Disassembly of section \.data:
 #...
 .*[ 	]+R_X86_64_32[ 	]+xtrn
--- a/gas/testsuite/gas/i386/ilp32/reloc64.l
+++ b/gas/testsuite/gas/i386/ilp32/reloc64.l
@@ -51,17 +51,17 @@
 .*:175: Error: .*
 .*:176: Error: .*
 .*:177: Error: .*
-.*:189: Error: .*
-.*:192: Error: .* too large for field of 4 bytes at .*
+.*:190: Error: .*
 .*:193: Error: .* too large for field of 4 bytes at .*
 .*:194: Error: .* too large for field of 4 bytes at .*
 .*:195: Error: .* too large for field of 4 bytes at .*
-.*:196: Error: .* too large for field of 2 bytes at .*
-.*:196: Error: .* too large for field of 1 byte at .*
+.*:196: Error: .* too large for field of 4 bytes at .*
 .*:197: Error: .* too large for field of 2 bytes at .*
 .*:197: Error: .* too large for field of 1 byte at .*
-.*:200: Error: .* too large for field of 4 bytes at .*
-.*:201: Error: .* too large for field of 2 bytes at .*
+.*:198: Error: .* too large for field of 2 bytes at .*
+.*:198: Error: .* too large for field of 1 byte at .*
+.*:201: Error: .* too large for field of 4 bytes at .*
 .*:202: Error: .* too large for field of 2 bytes at .*
-.*:203: Error: .* too large for field of 1 byte at .*
+.*:203: Error: .* too large for field of 2 bytes at .*
 .*:204: Error: .* too large for field of 1 byte at .*
+.*:205: Error: .* too large for field of 1 byte at .*
--- a/gas/testsuite/gas/i386/ilp32/reloc64.s
+++ b/gas/testsuite/gas/i386/ilp32/reloc64.s
@@ -178,7 +178,8 @@ bad	.byte	xtrn@tpoff
 
 	.text
 	mov	xtrn@tpoff (%rbx), %eax
-	movabsq	$xtrn - 4294967295, %rbp
+	movabsq	$xtrn + 0x7fffffff, %rbx
+	movabsq	$xtrn - 0x80000000, %rbp
 
 	.data
 	.quad	xtrn


  parent reply	other threads:[~2022-03-21 14:17 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-25 11:18 [PATCH 0/2] x86/x32: relocation overflow detection Jan Beulich
2022-02-25 11:19 ` [PATCH 1/2] x86: don't suppress overflow diagnostics in x32 mode Jan Beulich
2022-02-25 15:14   ` H.J. Lu
2022-02-25 11:20 ` [PATCH 2/2] ELF32: don't silently truncate relocation addends Jan Beulich
2022-02-27 14:01   ` Alan Modra
2022-02-28  7:21     ` Jan Beulich
2022-03-21 14:16 ` Jan Beulich [this message]
2022-03-22 14:20   ` [PATCH v2] " Nick Clifton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20dd63c2-09c7-6308-c8a6-d2ecd9726430@suse.com \
    --to=jbeulich@suse.com \
    --cc=binutils@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).