Re: [PATCH PR gdb/15236] gdbserver write to linux memory with zero length corrupts stack

Pedro Alves
2013-03-07T09:56:38Z

On 03/07/2013 09:12 AM, Jeremy Bennett wrote:
> On Thu, 2013-03-07 at 17:06 +0800, Yao Qi wrote:
>> On 03/07/2013 04:52 PM, Jeremy Bennett wrote:
>>> 2013-03-07 Jeremy Bennett<jeremy.bennett@embecosm.com>
>>>
>>> PR gdb/15236
>> ^^^ It should be "server".
>>> * linux-low.c (linux_write_memory): Return early success if LEN is
>>> zero.
>
> Hi Yao,
>
> My misunderstanding. Revised ChangeLog entry:
>
> 2013-03-07 Jeremy Bennett <jeremy.bennett@embecosm.com>
>
> PR server/15236
> * linux-low.c (linux_write_memory): Return early success if LEN is
> zero.

Thanks. Here's what I've applied, with a minor tweak (double-spaces
after periods; s/immediately/always/).

-- >8 --
PR gdb/15236: gdbserver write to linux memory with zero length corrupts stack

PROBLEM:

The function linux_write_memory () in linux-low.c allocates a buffer
on the stack to hold a copy of the data to be written.

register PTRACE_XFER_TYPE *buffer = (PTRACE_XFER_TYPE *)
alloca (count * sizeof (PTRACE_XFER_TYPE));

"count" is the number of bytes to be written, rounded up to the
nearest multiple of sizeof (PTRACE_XFER_TYPE) and allowing for not
being an aligned address. The function later uses

buffer[0] = ptrace (PTRACE_PEEKTEXT, pid,
(PTRACE_ARG3_TYPE) (uintptr_t) addr, 0);

The problem is that this function can be called to write zero bytes on
an aligned address, for example when receiving an X packet of length 0
(used to test if 8-bit write is supported). Under these circumstances,
count can be zero.

Since in this case, buffer[0] may never have been allocated, the stack
is corrupted and gdbserver may crash.

SOLUTION:

Writing zero bytes should always succeed. The patch below returns
successfully early if the length is zero, so avoiding the stack
corruption.

Verified on the ARC GDB 7.5.1 port.

2013-03-07 Jeremy Bennett <jeremy.bennett@embecosm.com>

PR server/15236
* linux-low.c (linux_write_memory): Return early success if LEN is
zero.
---
gdb/gdbserver/ChangeLog | 6 ++++++
gdb/gdbserver/linux-low.c | 8 +++++++-
2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index a8cf78c..67bc149 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,9 @@
+2013-03-07 Jeremy Bennett <jeremy.bennett@embecosm.com>
+
+ PR server/15236
+ * linux-low.c (linux_write_memory): Return early success if LEN is
+ zero.
+
2013-03-05 Corinna Vinschen <vinschen@redhat.de>

* configure.srv: Add x86_64-*-cygwin* as target.
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index c52cd2e..5f03628 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -4481,7 +4481,7 @@ linux_read_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len)

/* Copy LEN bytes of data from debugger memory at MYADDR to inferior's
memory at MEMADDR. On failure (cannot write to the inferior)
- returns the value of errno. */
+ returns the value of errno. Always succeeds if LEN is zero. */

static int
linux_write_memory (CORE_ADDR memaddr, const unsigned char *myaddr, int len)
@@ -4500,6 +4500,12 @@ linux_write_memory (CORE_ADDR memaddr, const unsigned char *myaddr, int len)

int pid = lwpid_of (get_thread_lwp (current_inferior));

+ if (len == 0)
+ {
+ /* Zero length write always succeeds. */
+ return 0;
+ }
+
if (debug_threads)
{
/* Dump up to four bytes. */
--
1.7.11.7