Re: [Qemu-devel] [PATCH for-2.12] gdbstub: fix off-by-one in gdb_handle_packet()
On 9 April 2018 at 10:39, Paolo Bonzini 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é >>> --- >>> (gdb) dump binary memory /tmp/dram.bin 0x9400 0x9410 >>> 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 > > 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
Re: [Qemu-devel] [PATCH for-2.12] gdbstub: fix off-by-one in gdb_handle_packet()
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é >> --- >> (gdb) dump binary memory /tmp/dram.bin 0x9400 0x9410 >> 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 Peter, can you apply this directly to master? Thanks, Paolo signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH for-2.12] gdbstub: fix off-by-one in gdb_handle_packet()
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é > --- > (gdb) dump binary memory /tmp/dram.bin 0x9400 0x9410 > 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 signature.asc Description: PGP signature
[Qemu-devel] [PATCH for-2.12] gdbstub: fix off-by-one in gdb_handle_packet()
memtohex() adds an extra trailing NUL character. Reported-by: AddressSanitizer Signed-off-by: Philippe Mathieu-Daudé --- (gdb) dump binary memory /tmp/dram.bin 0x9400 0x9410 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