* [Qemu-devel] [PATCH for-2.12] gdbstub: fix off-by-one in gdb_handle_packet()
@ 2018-04-08 14:59 Philippe Mathieu-Daudé
2018-04-09 5:58 ` Stefan Hajnoczi
0 siblings, 1 reply; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-04-08 14:59 UTC (permalink / raw)
To: Marc-André Lureau, Paolo Bonzini
Cc: Philippe Mathieu-Daudé, qemu-devel
memtohex() adds an extra trailing NUL character.
Reported-by: AddressSanitizer
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
(gdb) dump binary memory /tmp/dram.bin 0x94000000 0x94100000
Remote connection closed
=================================================================
==22732==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffe43018340 at pc 0x55f2655fde81 bp 0x7ffe43017210 sp 0x7ffe43017208
WRITE of size 1 at 0x7ffe43018340 thread T0
#0 0x55f2655fde80 in memtohex /source/qemu/gdbstub.c:520
#1 0x55f26560254d in gdb_handle_packet /source/qemu/gdbstub.c:1140
#2 0x55f2656073c3 in gdb_read_byte /source/qemu/gdbstub.c:1703
#3 0x55f2656076a7 in gdb_chr_receive /source/qemu/gdbstub.c:1909
#4 0x55f266457656 in qemu_chr_be_write_impl /source/qemu/chardev/char.c:175
#5 0x55f2664576f9 in qemu_chr_be_write /source/qemu/chardev/char.c:187
#6 0x55f26646f6f0 in tcp_chr_read /source/qemu/chardev/char-socket.c:470
#7 0x55f2664bc9e3 in qio_channel_fd_source_dispatch /source/qemu/io/channel-watch.c:84
#8 0x7f17d01b30f4 in g_main_context_dispatch (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4c0f4)
#9 0x55f2665c7f10 in glib_pollfds_poll /source/qemu/util/main-loop.c:215
#10 0x55f2665c8100 in os_host_main_loop_wait /source/qemu/util/main-loop.c:263
#11 0x55f2665c82d6 in main_loop_wait /source/qemu/util/main-loop.c:522
#12 0x55f26599e13b in main_loop /source/qemu/vl.c:1943
#13 0x55f2659b0869 in main /source/qemu/vl.c:4734
Address 0x7ffe43018340 is located in stack of thread T0 at offset 4192 in frame
#0 0x55f265601266 in gdb_handle_packet /source/qemu/gdbstub.c:996
This frame has 3 object(s):
[32, 40) 'p'
[96, 4192) 'buf' <== Memory access at offset 4192 overflows this variable
[4224, 8320) 'mem_buf'
SUMMARY: AddressSanitizer: stack-buffer-overflow /source/qemu/gdbstub.c:520 in memtohex
Shadow bytes around the buggy address:
0x1000485fb010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1000485fb020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1000485fb030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1000485fb040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1000485fb050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x1000485fb060: 00 00 00 00 00 00 00 00[f2]f2 f2 f2 00 00 00 00
0x1000485fb070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1000485fb080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1000485fb090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1000485fb0a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1000485fb0b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==22732==ABORTING
---
gdbstub.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/gdbstub.c b/gdbstub.c
index a76b2fa481..18a8d8a710 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -507,6 +507,7 @@ static inline int tohex(int v)
return v - 10 + 'a';
}
+/* writes 2*len+1 bytes in buf */
static void memtohex(char *buf, const uint8_t *mem, int len)
{
int i, c;
@@ -999,8 +1000,9 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
const char *p;
uint32_t thread;
int ch, reg_size, type, res;
- char buf[MAX_PACKET_LENGTH];
uint8_t mem_buf[MAX_PACKET_LENGTH];
+ char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
uint8_t *registers;
target_ulong addr, len;
--
2.17.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.12] gdbstub: fix off-by-one in gdb_handle_packet()
2018-04-08 14:59 [Qemu-devel] [PATCH for-2.12] gdbstub: fix off-by-one in gdb_handle_packet() Philippe Mathieu-Daudé
@ 2018-04-09 5:58 ` Stefan Hajnoczi
2018-04-09 9:39 ` Paolo Bonzini
0 siblings, 1 reply; 4+ messages in thread
From: Stefan Hajnoczi @ 2018-04-09 5:58 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Marc-André Lureau, Paolo Bonzini, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3721 bytes --]
On Sun, Apr 08, 2018 at 11:59:33AM -0300, Philippe Mathieu-Daudé wrote:
> memtohex() adds an extra trailing NUL character.
>
> Reported-by: AddressSanitizer
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> (gdb) dump binary memory /tmp/dram.bin 0x94000000 0x94100000
> Remote connection closed
>
> =================================================================
> ==22732==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffe43018340 at pc 0x55f2655fde81 bp 0x7ffe43017210 sp 0x7ffe43017208
> WRITE of size 1 at 0x7ffe43018340 thread T0
> #0 0x55f2655fde80 in memtohex /source/qemu/gdbstub.c:520
> #1 0x55f26560254d in gdb_handle_packet /source/qemu/gdbstub.c:1140
> #2 0x55f2656073c3 in gdb_read_byte /source/qemu/gdbstub.c:1703
> #3 0x55f2656076a7 in gdb_chr_receive /source/qemu/gdbstub.c:1909
> #4 0x55f266457656 in qemu_chr_be_write_impl /source/qemu/chardev/char.c:175
> #5 0x55f2664576f9 in qemu_chr_be_write /source/qemu/chardev/char.c:187
> #6 0x55f26646f6f0 in tcp_chr_read /source/qemu/chardev/char-socket.c:470
> #7 0x55f2664bc9e3 in qio_channel_fd_source_dispatch /source/qemu/io/channel-watch.c:84
> #8 0x7f17d01b30f4 in g_main_context_dispatch (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4c0f4)
> #9 0x55f2665c7f10 in glib_pollfds_poll /source/qemu/util/main-loop.c:215
> #10 0x55f2665c8100 in os_host_main_loop_wait /source/qemu/util/main-loop.c:263
> #11 0x55f2665c82d6 in main_loop_wait /source/qemu/util/main-loop.c:522
> #12 0x55f26599e13b in main_loop /source/qemu/vl.c:1943
> #13 0x55f2659b0869 in main /source/qemu/vl.c:4734
>
> Address 0x7ffe43018340 is located in stack of thread T0 at offset 4192 in frame
> #0 0x55f265601266 in gdb_handle_packet /source/qemu/gdbstub.c:996
>
> This frame has 3 object(s):
> [32, 40) 'p'
> [96, 4192) 'buf' <== Memory access at offset 4192 overflows this variable
> [4224, 8320) 'mem_buf'
> SUMMARY: AddressSanitizer: stack-buffer-overflow /source/qemu/gdbstub.c:520 in memtohex
> Shadow bytes around the buggy address:
> 0x1000485fb010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x1000485fb020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x1000485fb030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x1000485fb040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x1000485fb050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> =>0x1000485fb060: 00 00 00 00 00 00 00 00[f2]f2 f2 f2 00 00 00 00
> 0x1000485fb070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x1000485fb080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x1000485fb090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x1000485fb0a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x1000485fb0b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> Shadow byte legend (one shadow byte represents 8 application bytes):
> Addressable: 00
> Partially addressable: 01 02 03 04 05 06 07
> Heap left redzone: fa
> Freed heap region: fd
> Stack left redzone: f1
> Stack mid redzone: f2
> Stack right redzone: f3
> Stack after return: f5
> Stack use after scope: f8
> Global redzone: f9
> Global init order: f6
> Poisoned by user: f7
> Container overflow: fc
> Array cookie: ac
> Intra object redzone: bb
> ASan internal: fe
> Left alloca redzone: ca
> Right alloca redzone: cb
> ==22732==ABORTING
> ---
> gdbstub.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.12] gdbstub: fix off-by-one in gdb_handle_packet()
2018-04-09 5:58 ` Stefan Hajnoczi
@ 2018-04-09 9:39 ` Paolo Bonzini
2018-04-09 15:25 ` Peter Maydell
0 siblings, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2018-04-09 9:39 UTC (permalink / raw)
To: Stefan Hajnoczi, Philippe Mathieu-Daudé
Cc: Marc-André Lureau, qemu-devel, Peter Maydell
[-- Attachment #1: Type: text/plain, Size: 855 bytes --]
On 09/04/2018 07:58, Stefan Hajnoczi wrote:
> On Sun, Apr 08, 2018 at 11:59:33AM -0300, Philippe Mathieu-Daudé wrote:
>> memtohex() adds an extra trailing NUL character.
>>
>> Reported-by: AddressSanitizer
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> (gdb) dump binary memory /tmp/dram.bin 0x94000000 0x94100000
>> Remote connection closed
>>
>> =================================================================
>> ==22732==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffe43018340 at pc 0x55f2655fde81 bp 0x7ffe43017210 sp 0x7ffe43017208
>> WRITE of size 1 at 0x7ffe43018340 thread T0
>>
>> ---
>> gdbstub.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Peter, can you apply this directly to master?
Thanks,
Paolo
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.12] gdbstub: fix off-by-one in gdb_handle_packet()
2018-04-09 9:39 ` Paolo Bonzini
@ 2018-04-09 15:25 ` Peter Maydell
0 siblings, 0 replies; 4+ messages in thread
From: Peter Maydell @ 2018-04-09 15:25 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Stefan Hajnoczi, Philippe Mathieu-Daudé,
Marc-André Lureau, QEMU Developers
On 9 April 2018 at 10:39, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 09/04/2018 07:58, Stefan Hajnoczi wrote:
>> On Sun, Apr 08, 2018 at 11:59:33AM -0300, Philippe Mathieu-Daudé wrote:
>>> memtohex() adds an extra trailing NUL character.
>>>
>>> Reported-by: AddressSanitizer
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>> (gdb) dump binary memory /tmp/dram.bin 0x94000000 0x94100000
>>> Remote connection closed
>>>
>>> =================================================================
>>> ==22732==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffe43018340 at pc 0x55f2655fde81 bp 0x7ffe43017210 sp 0x7ffe43017208
>>> WRITE of size 1 at 0x7ffe43018340 thread T0
>>>
>>> ---
>>> gdbstub.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>
> Peter, can you apply this directly to master?
Applied, thanks. (patchwork and patches made a pig's ear of this
for some reason, I think they got confused about where the
commit message stopped and the patch started, so I had to
hand-edit the files.)
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-04-09 15:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-08 14:59 [Qemu-devel] [PATCH for-2.12] gdbstub: fix off-by-one in gdb_handle_packet() Philippe Mathieu-Daudé
2018-04-09 5:58 ` Stefan Hajnoczi
2018-04-09 9:39 ` Paolo Bonzini
2018-04-09 15:25 ` Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.