public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Fix skipping stack protector on arm
@ 2014-10-25  2:10 Yao Qi
  2014-10-29  5:45 ` Yao Qi
  0 siblings, 1 reply; 2+ messages in thread
From: Yao Qi @ 2014-10-25  2:10 UTC (permalink / raw)
  To: gdb-patches

This patch fixes the bug in my patch skipping stack protector
https://www.sourceware.org/ml/gdb-patches/2010-12/msg00110.html

In my skipping stack protector patch, I misunderstood the constant vs.
immediate on instruction encodings, and treated immediate as constant
by mistake.  The instruction 'ldr Rd, [PC, #immed]' loads the
address of __stack_chk_guard to Rd, and #immed is an offset from PC.
We should get the __stack_chk_guard from *(pc + #immed).

As a result of this mistake, arm_analyze_load_stack_chk_guard returns
the wrong address of __stack_chk_guard, and the symbol
__stack_chk_guard can't be found.  However, we continue to match the
following instructions when symbol isn't found, so the code still
works.  In other words, the code just matches the instruction pattern
without checking __stack_chk_guard symbol correctly.

Joel's patch <https://sourceware.org/ml/gdb-patches/2014-10/msg00605.html>
makes the heuristics stricter that we stop matching instructions if
symbol __stack_chk_guard isn't found.  Then the bug is exposed.  This
patch is to correct the load address computation for ldr instruction,
and it fixes some fails in gdb.mi/gdb792.exp on armv4t both arm and
thumb mode.

Regression tested on arm-linux-gnueabi target with
{armv4t, armv7-a} x {marm, mthumb} x {-fstack-protector,-fno-stack-protector}

gdb:

2014-10-25  Yao Qi  <yao@codesourcery.com>

	* arm-tdep.c (arm_analyze_load_stack_chk_guard): Compute the
	loaded address correctly of ldr instruction.
---
 gdb/arm-tdep.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 5dccf0a..5e3c6c9 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -1204,7 +1204,9 @@ arm_analyze_load_stack_chk_guard(CORE_ADDR pc, struct gdbarch *gdbarch,
 	{
 	  *destreg = bits (insn1, 8, 10);
 	  *offset = 2;
-	  address = bits (insn1, 0, 7);
+	  address = (pc & 0xfffffffc) + 4 + (bits (insn1, 0, 7) << 2);
+	  address = read_memory_unsigned_integer (address, 4,
+						  byte_order_for_code);
 	}
       else if ((insn1 & 0xfbf0) == 0xf240) /* movw Rd, #const */
 	{
@@ -1233,9 +1235,12 @@ arm_analyze_load_stack_chk_guard(CORE_ADDR pc, struct gdbarch *gdbarch,
       unsigned int insn
 	= read_memory_unsigned_integer (pc, 4, byte_order_for_code);
 
-      if ((insn & 0x0e5f0000) == 0x041f0000) /* ldr Rd, #immed */
+      if ((insn & 0x0e5f0000) == 0x041f0000) /* ldr Rd, [PC, #immed] */
 	{
-	  address = bits (insn, 0, 11);
+	  address = bits (insn, 0, 11) + pc + 8;
+	  address = read_memory_unsigned_integer (address, 4,
+						  byte_order_for_code);
+
 	  *destreg = bits (insn, 12, 15);
 	  *offset = 4;
 	}
-- 
1.9.3

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] Fix skipping stack protector on arm
  2014-10-25  2:10 [PATCH] Fix skipping stack protector on arm Yao Qi
@ 2014-10-29  5:45 ` Yao Qi
  0 siblings, 0 replies; 2+ messages in thread
From: Yao Qi @ 2014-10-29  5:45 UTC (permalink / raw)
  To: gdb-patches

Yao Qi <yao@codesourcery.com> writes:

> 2014-10-25  Yao Qi  <yao@codesourcery.com>
>
> 	* arm-tdep.c (arm_analyze_load_stack_chk_guard): Compute the
> 	loaded address correctly of ldr instruction.

Patch is pushed in.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2014-10-29  5:45 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-25  2:10 [PATCH] Fix skipping stack protector on arm Yao Qi
2014-10-29  5:45 ` Yao Qi

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).