Today, gdbproxy reuses the same buffer for both the qXfer reply and the actual GDB packet reply. This worked well, since each byte in the qXfer reply buffer matched 1:1 to each byte in the actual GDB reply packet. Since we escape special characters now, this property no longer holds and a single byte in qXfer reply will take up to two bytes in the GDB reply packet. This causes offsets to shift, preventing the offset/length response slicing (part of GDB protocol) from working correctly. Fix this by writing the qXfer reply data in a separate buffer, and performing slicing out of it. Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- Notes: v3 -> v4: - Fix pointer variable declaration style - Fix indentation - s/buffer_realloc/realloc/g - s/vl_buffer/reply_buffer/g; s/vl_/reply_buffer_/g; s/vlbuf/reply/g - Rewrite reply_buffer_add_uinthex(): convert for loop into while loop - Rewrite reply_buffer_append_xmlstr(): convert for loop into while loop - Split packet_reply_add() into packet_reply_add_data() here v4 -> v5: - Split out XML escaping in a separate patch - Don't realloc/free when emptying - Drop reply_buffer_resize(), merge into reply_buffer_grow() - Remove "more" variables for now - Don't empty buffer before and after v5 -> v6: - Edit commit message - Remove gratuitous blank line v6 -> v7: - Add function reply_buffer_append_str here - packet_reply_xfer: Change type of `off` and `len` parameters to size_t - packet_reply_xfer: Remove data and datalen parameters programs/winedbg/gdbproxy.c | 264 +++++++++++++++++++----------------- 1 file changed, 137 insertions(+), 127 deletions(-) diff --git a/programs/winedbg/gdbproxy.c b/programs/winedbg/gdbproxy.c index 120cc84e008..a1ecdea770f 100644 --- a/programs/winedbg/gdbproxy.c +++ b/programs/winedbg/gdbproxy.c @@ -87,6 +87,7 @@ struct gdb_context /* Unix environment */ ULONG_PTR wine_segs[3]; /* load addresses of the ELF wine exec segments (text, bss and data) */ BOOL no_ack_mode; + struct reply_buffer qxfer_buffer; }; /* assume standard signal and errno values */ @@ -255,6 +256,11 @@ static void reply_buffer_append(struct reply_buffer* reply, const void* data, si reply->len += size; } +static inline void reply_buffer_append_str(struct reply_buffer* reply, const char* str) +{ + reply_buffer_append(reply, str, strlen(str)); +} + static inline void reply_buffer_append_hex(struct reply_buffer* reply, const void* src, size_t len) { reply_buffer_grow(reply, len * 2); @@ -797,20 +803,20 @@ static inline BOOL is_gdb_special_char(unsigned char val) return gdb_special_chars_lookup_table[val % length] == val; } -static void packet_reply_add(struct gdb_context* gdbctx, const char* str) +static void packet_reply_add_data(struct gdb_context* gdbctx, const void* data, size_t len) { - const unsigned char *ptr = (unsigned char *)str, *curr; + const unsigned char *ptr = data, *end = ptr + len, *curr; unsigned char esc_seq[2]; - while (*ptr) + while (ptr != end) { curr = ptr; - while (*ptr && !is_gdb_special_char(*ptr)) + while (ptr != end && !is_gdb_special_char(*ptr)) ptr++; reply_buffer_append(&gdbctx->out_buf, curr, ptr - curr); - if (!*ptr) break; + if (ptr == end) break; esc_seq[0] = 0x7D; esc_seq[1] = 0x20 ^ *ptr++; @@ -818,6 +824,11 @@ static void packet_reply_add(struct gdb_context* gdbctx, const char* str) } } +static inline void packet_reply_add(struct gdb_context* gdbctx, const char* str) +{ + packet_reply_add_data(gdbctx, str, strlen(str)); +} + static void packet_reply_open(struct gdb_context* gdbctx) { assert(gdbctx->out_curr_packet == -1); @@ -837,35 +848,6 @@ static void packet_reply_close(struct gdb_context* gdbctx) gdbctx->out_curr_packet = -1; } -static void packet_reply_open_xfer(struct gdb_context* gdbctx) -{ - packet_reply_open(gdbctx); - packet_reply_add(gdbctx, "m"); -} - -static void packet_reply_close_xfer(struct gdb_context* gdbctx, unsigned int off, unsigned int len) -{ - int begin = gdbctx->out_curr_packet + 1; - int plen; - - if (begin + off < gdbctx->out_buf.len) - { - gdbctx->out_buf.len -= off; - memmove(gdbctx->out_buf.base + begin, gdbctx->out_buf.base + begin + off, gdbctx->out_buf.len); - } - else - { - gdbctx->out_buf.base[gdbctx->out_curr_packet] = 'l'; - gdbctx->out_buf.len = gdbctx->out_curr_packet + 1; - } - - plen = gdbctx->out_buf.len - begin; - if (plen > len) gdbctx->out_buf.len -= (plen - len); - else gdbctx->out_buf.base[gdbctx->out_curr_packet] = 'l'; - - packet_reply_close(gdbctx); -} - static enum packet_return packet_reply(struct gdb_context* gdbctx, const char* packet) { packet_reply_open(gdbctx); @@ -895,6 +877,28 @@ static inline void packet_reply_register_hex_to(struct gdb_context* gdbctx, dbg_ packet_reply_hex_to(gdbctx, cpu_register_ptr(gdbctx, ctx, idx), cpu_register_map[idx].length); } +static void packet_reply_xfer(struct gdb_context* gdbctx, size_t off, size_t len) +{ + size_t data_len, trunc_len; + + packet_reply_open(gdbctx); + data_len = gdbctx->qxfer_buffer.len; + + /* check if off + len would overflow */ + if (off < data_len && off + len < data_len) + packet_reply_add(gdbctx, "m"); + else + packet_reply_add(gdbctx, "l"); + + if (off < data_len) + { + trunc_len = min(len, data_len - off); + packet_reply_add_data(gdbctx, gdbctx->qxfer_buffer.base + off, trunc_len); + } + + packet_reply_close(gdbctx); +} + /* =============================================== * * P A C K E T H A N D L E R S * * =============================================== * @@ -1602,6 +1606,7 @@ static enum packet_return packet_query_remote_command(struct gdb_context* gdbctx static BOOL CALLBACK packet_query_libraries_cb(PCSTR mod_name, DWORD64 base, PVOID ctx) { struct gdb_context* gdbctx = ctx; + struct reply_buffer* reply = &gdbctx->qxfer_buffer; MEMORY_BASIC_INFORMATION mbi; IMAGE_SECTION_HEADER *sec; IMAGE_DOS_HEADER *dos = NULL; @@ -1614,11 +1619,11 @@ static BOOL CALLBACK packet_query_libraries_cb(PCSTR mod_name, DWORD64 base, PVO mod.SizeOfStruct = sizeof(mod); SymGetModuleInfo64(gdbctx->process->handle, base, &mod); - packet_reply_add(gdbctx, "<library name=\""); + reply_buffer_append_str(reply, "<library name=\""); if (strcmp(mod.LoadedImageName, "[vdso].so") == 0) - packet_reply_add(gdbctx, "linux-vdso.so.1"); + reply_buffer_append_str(reply, "linux-vdso.so.1"); else if (mod.LoadedImageName[0] == '/') - packet_reply_add(gdbctx, mod.LoadedImageName); + reply_buffer_append_str(reply, mod.LoadedImageName); else { UNICODE_STRING nt_name; @@ -1633,15 +1638,15 @@ static BOOL CALLBACK packet_query_libraries_cb(PCSTR mod_name, DWORD64 base, PVO if (IsWow64Process(gdbctx->process->handle, &is_wow64) && is_wow64 && (tmp = strstr(unix_path, "system32"))) memcpy(tmp, "syswow64", 8); - packet_reply_add(gdbctx, unix_path); + reply_buffer_append_str(reply, unix_path); } else - packet_reply_add(gdbctx, mod.LoadedImageName); + reply_buffer_append_str(reply, mod.LoadedImageName); HeapFree(GetProcessHeap(), 0, unix_path); RtlFreeUnicodeString(&nt_name); } - packet_reply_add(gdbctx, "\">"); + reply_buffer_append_str(reply, "\">"); size = sizeof(buffer); if (VirtualQueryEx(gdbctx->process->handle, (void *)(UINT_PTR)mod.BaseOfImage, &mbi, sizeof(mbi)) >= sizeof(mbi) && @@ -1672,72 +1677,75 @@ static BOOL CALLBACK packet_query_libraries_cb(PCSTR mod_name, DWORD64 base, PVO for (i = 0; i < max(nth->FileHeader.NumberOfSections, 1); ++i) { if ((char *)(sec + i) >= buffer + size) break; - packet_reply_add(gdbctx, "<segment address=\"0x"); - packet_reply_val(gdbctx, mod.BaseOfImage + sec[i].VirtualAddress, sizeof(ULONG_PTR)); - packet_reply_add(gdbctx, "\"/>"); + reply_buffer_append_str(reply, "<segment address=\"0x"); + reply_buffer_append_uinthex(reply, mod.BaseOfImage + sec[i].VirtualAddress, sizeof(ULONG_PTR)); + reply_buffer_append_str(reply, "\"/>"); } - packet_reply_add(gdbctx, "</library>"); + reply_buffer_append_str(reply, "</library>"); return TRUE; } static void packet_query_libraries(struct gdb_context* gdbctx) { + struct reply_buffer* reply = &gdbctx->qxfer_buffer; BOOL opt; /* this will resynchronize builtin dbghelp's internal ELF module list */ SymLoadModule(gdbctx->process->handle, 0, 0, 0, 0, 0); - packet_reply_add(gdbctx, "<library-list>"); + reply_buffer_append_str(reply, "<library-list>"); opt = SymSetExtendedOption(SYMOPT_EX_WINE_NATIVE_MODULES, TRUE); SymEnumerateModules64(gdbctx->process->handle, packet_query_libraries_cb, gdbctx); SymSetExtendedOption(SYMOPT_EX_WINE_NATIVE_MODULES, opt); - packet_reply_add(gdbctx, "</library-list>"); + reply_buffer_append_str(reply, "</library-list>"); } static void packet_query_threads(struct gdb_context* gdbctx) { + struct reply_buffer* reply = &gdbctx->qxfer_buffer; struct dbg_process* process = gdbctx->process; struct dbg_thread* thread; - packet_reply_add(gdbctx, "<threads>"); + reply_buffer_append_str(reply, "<threads>"); LIST_FOR_EACH_ENTRY(thread, &process->threads, struct dbg_thread, entry) { - packet_reply_add(gdbctx, "<thread "); - packet_reply_add(gdbctx, "id=\""); - packet_reply_val(gdbctx, thread->tid, 4); - packet_reply_add(gdbctx, "\" name=\""); - packet_reply_add(gdbctx, thread->name); - packet_reply_add(gdbctx, "\"/>"); + reply_buffer_append_str(reply, "<thread "); + reply_buffer_append_str(reply, "id=\""); + reply_buffer_append_uinthex(reply, thread->tid, 4); + reply_buffer_append_str(reply, "\" name=\""); + reply_buffer_append_str(reply, thread->name); + reply_buffer_append_str(reply, "\"/>"); } - packet_reply_add(gdbctx, "</threads>"); + reply_buffer_append_str(reply, "</threads>"); } static void packet_query_target_xml(struct gdb_context* gdbctx, struct backend_cpu* cpu) { + struct reply_buffer* reply = &gdbctx->qxfer_buffer; const char* feature_prefix = NULL; const char* feature = NULL; char buffer[256]; int i; - packet_reply_add(gdbctx, "<target>"); + reply_buffer_append_str(reply, "<target>"); switch (cpu->machine) { case IMAGE_FILE_MACHINE_AMD64: - packet_reply_add(gdbctx, "<architecture>i386:x86-64</architecture>"); + reply_buffer_append_str(reply, "<architecture>i386:x86-64</architecture>"); feature_prefix = "org.gnu.gdb.i386."; break; case IMAGE_FILE_MACHINE_I386: - packet_reply_add(gdbctx, "<architecture>i386</architecture>"); + reply_buffer_append_str(reply, "<architecture>i386</architecture>"); feature_prefix = "org.gnu.gdb.i386."; break; case IMAGE_FILE_MACHINE_ARMNT: - packet_reply_add(gdbctx, "<architecture>arm</architecture>"); + reply_buffer_append_str(reply, "<architecture>arm</architecture>"); feature_prefix = "org.gnu.gdb.arm."; break; case IMAGE_FILE_MACHINE_ARM64: - packet_reply_add(gdbctx, "<architecture>aarch64</architecture>"); + reply_buffer_append_str(reply, "<architecture>aarch64</architecture>"); feature_prefix = "org.gnu.gdb.aarch64."; break; } @@ -1746,87 +1754,87 @@ static void packet_query_target_xml(struct gdb_context* gdbctx, struct backend_c { if (cpu->gdb_register_map[i].feature) { - if (feature) packet_reply_add(gdbctx, "</feature>"); + if (feature) reply_buffer_append_str(reply, "</feature>"); feature = cpu->gdb_register_map[i].feature; - packet_reply_add(gdbctx, "<feature name=\""); - if (feature_prefix) packet_reply_add(gdbctx, feature_prefix); - packet_reply_add(gdbctx, feature); - packet_reply_add(gdbctx, "\">"); + reply_buffer_append_str(reply, "<feature name=\""); + if (feature_prefix) reply_buffer_append_str(reply, feature_prefix); + reply_buffer_append_str(reply, feature); + reply_buffer_append_str(reply, "\">"); if (strcmp(feature_prefix, "org.gnu.gdb.i386.") == 0 && strcmp(feature, "core") == 0) - packet_reply_add(gdbctx, "<flags id=\"i386_eflags\" size=\"4\">" - "<field name=\"CF\" start=\"0\" end=\"0\"/>" - "<field name=\"\" start=\"1\" end=\"1\"/>" - "<field name=\"PF\" start=\"2\" end=\"2\"/>" - "<field name=\"AF\" start=\"4\" end=\"4\"/>" - "<field name=\"ZF\" start=\"6\" end=\"6\"/>" - "<field name=\"SF\" start=\"7\" end=\"7\"/>" - "<field name=\"TF\" start=\"8\" end=\"8\"/>" - "<field name=\"IF\" start=\"9\" end=\"9\"/>" - "<field name=\"DF\" start=\"10\" end=\"10\"/>" - "<field name=\"OF\" start=\"11\" end=\"11\"/>" - "<field name=\"NT\" start=\"14\" end=\"14\"/>" - "<field name=\"RF\" start=\"16\" end=\"16\"/>" - "<field name=\"VM\" start=\"17\" end=\"17\"/>" - "<field name=\"AC\" start=\"18\" end=\"18\"/>" - "<field name=\"VIF\" start=\"19\" end=\"19\"/>" - "<field name=\"VIP\" start=\"20\" end=\"20\"/>" - "<field name=\"ID\" start=\"21\" end=\"21\"/>" - "</flags>"); + reply_buffer_append_str(reply, "<flags id=\"i386_eflags\" size=\"4\">" + "<field name=\"CF\" start=\"0\" end=\"0\"/>" + "<field name=\"\" start=\"1\" end=\"1\"/>" + "<field name=\"PF\" start=\"2\" end=\"2\"/>" + "<field name=\"AF\" start=\"4\" end=\"4\"/>" + "<field name=\"ZF\" start=\"6\" end=\"6\"/>" + "<field name=\"SF\" start=\"7\" end=\"7\"/>" + "<field name=\"TF\" start=\"8\" end=\"8\"/>" + "<field name=\"IF\" start=\"9\" end=\"9\"/>" + "<field name=\"DF\" start=\"10\" end=\"10\"/>" + "<field name=\"OF\" start=\"11\" end=\"11\"/>" + "<field name=\"NT\" start=\"14\" end=\"14\"/>" + "<field name=\"RF\" start=\"16\" end=\"16\"/>" + "<field name=\"VM\" start=\"17\" end=\"17\"/>" + "<field name=\"AC\" start=\"18\" end=\"18\"/>" + "<field name=\"VIF\" start=\"19\" end=\"19\"/>" + "<field name=\"VIP\" start=\"20\" end=\"20\"/>" + "<field name=\"ID\" start=\"21\" end=\"21\"/>" + "</flags>"); if (strcmp(feature_prefix, "org.gnu.gdb.i386.") == 0 && strcmp(feature, "sse") == 0) - packet_reply_add(gdbctx, "<vector id=\"v4f\" type=\"ieee_single\" count=\"4\"/>" - "<vector id=\"v2d\" type=\"ieee_double\" count=\"2\"/>" - "<vector id=\"v16i8\" type=\"int8\" count=\"16\"/>" - "<vector id=\"v8i16\" type=\"int16\" count=\"8\"/>" - "<vector id=\"v4i32\" type=\"int32\" count=\"4\"/>" - "<vector id=\"v2i64\" type=\"int64\" count=\"2\"/>" - "<union id=\"vec128\">" - "<field name=\"v4_float\" type=\"v4f\"/>" - "<field name=\"v2_double\" type=\"v2d\"/>" - "<field name=\"v16_int8\" type=\"v16i8\"/>" - "<field name=\"v8_int16\" type=\"v8i16\"/>" - "<field name=\"v4_int32\" type=\"v4i32\"/>" - "<field name=\"v2_int64\" type=\"v2i64\"/>" - "<field name=\"uint128\" type=\"uint128\"/>" - "</union>" - "<flags id=\"i386_mxcsr\" size=\"4\">" - "<field name=\"IE\" start=\"0\" end=\"0\"/>" - "<field name=\"DE\" start=\"1\" end=\"1\"/>" - "<field name=\"ZE\" start=\"2\" end=\"2\"/>" - "<field name=\"OE\" start=\"3\" end=\"3\"/>" - "<field name=\"UE\" start=\"4\" end=\"4\"/>" - "<field name=\"PE\" start=\"5\" end=\"5\"/>" - "<field name=\"DAZ\" start=\"6\" end=\"6\"/>" - "<field name=\"IM\" start=\"7\" end=\"7\"/>" - "<field name=\"DM\" start=\"8\" end=\"8\"/>" - "<field name=\"ZM\" start=\"9\" end=\"9\"/>" - "<field name=\"OM\" start=\"10\" end=\"10\"/>" - "<field name=\"UM\" start=\"11\" end=\"11\"/>" - "<field name=\"PM\" start=\"12\" end=\"12\"/>" - "<field name=\"FZ\" start=\"15\" end=\"15\"/>" - "</flags>"); + reply_buffer_append_str(reply, "<vector id=\"v4f\" type=\"ieee_single\" count=\"4\"/>" + "<vector id=\"v2d\" type=\"ieee_double\" count=\"2\"/>" + "<vector id=\"v16i8\" type=\"int8\" count=\"16\"/>" + "<vector id=\"v8i16\" type=\"int16\" count=\"8\"/>" + "<vector id=\"v4i32\" type=\"int32\" count=\"4\"/>" + "<vector id=\"v2i64\" type=\"int64\" count=\"2\"/>" + "<union id=\"vec128\">" + "<field name=\"v4_float\" type=\"v4f\"/>" + "<field name=\"v2_double\" type=\"v2d\"/>" + "<field name=\"v16_int8\" type=\"v16i8\"/>" + "<field name=\"v8_int16\" type=\"v8i16\"/>" + "<field name=\"v4_int32\" type=\"v4i32\"/>" + "<field name=\"v2_int64\" type=\"v2i64\"/>" + "<field name=\"uint128\" type=\"uint128\"/>" + "</union>" + "<flags id=\"i386_mxcsr\" size=\"4\">" + "<field name=\"IE\" start=\"0\" end=\"0\"/>" + "<field name=\"DE\" start=\"1\" end=\"1\"/>" + "<field name=\"ZE\" start=\"2\" end=\"2\"/>" + "<field name=\"OE\" start=\"3\" end=\"3\"/>" + "<field name=\"UE\" start=\"4\" end=\"4\"/>" + "<field name=\"PE\" start=\"5\" end=\"5\"/>" + "<field name=\"DAZ\" start=\"6\" end=\"6\"/>" + "<field name=\"IM\" start=\"7\" end=\"7\"/>" + "<field name=\"DM\" start=\"8\" end=\"8\"/>" + "<field name=\"ZM\" start=\"9\" end=\"9\"/>" + "<field name=\"OM\" start=\"10\" end=\"10\"/>" + "<field name=\"UM\" start=\"11\" end=\"11\"/>" + "<field name=\"PM\" start=\"12\" end=\"12\"/>" + "<field name=\"FZ\" start=\"15\" end=\"15\"/>" + "</flags>"); } snprintf(buffer, ARRAY_SIZE(buffer), "<reg name=\"%s\" bitsize=\"%Iu\"", cpu->gdb_register_map[i].name, 8 * cpu->gdb_register_map[i].length); - packet_reply_add(gdbctx, buffer); + reply_buffer_append_str(reply, buffer); if (cpu->gdb_register_map[i].type) { - packet_reply_add(gdbctx, " type=\""); - packet_reply_add(gdbctx, cpu->gdb_register_map[i].type); - packet_reply_add(gdbctx, "\""); + reply_buffer_append_str(reply, " type=\""); + reply_buffer_append_str(reply, cpu->gdb_register_map[i].type); + reply_buffer_append_str(reply, "\""); } - packet_reply_add(gdbctx, "/>"); + reply_buffer_append_str(reply, "/>"); } - if (feature) packet_reply_add(gdbctx, "</feature>"); - packet_reply_add(gdbctx, "</target>"); + if (feature) reply_buffer_append_str(reply, "</feature>"); + reply_buffer_append_str(reply, "</target>"); } static enum packet_return packet_query(struct gdb_context* gdbctx) @@ -1958,9 +1966,9 @@ static enum packet_return packet_query(struct gdb_context* gdbctx) { if (!gdbctx->process) return packet_error; - packet_reply_open_xfer(gdbctx); packet_query_libraries(gdbctx); - packet_reply_close_xfer(gdbctx, off, len); + packet_reply_xfer(gdbctx, off, len); + reply_buffer_clear(&gdbctx->qxfer_buffer); return packet_done; } @@ -1968,9 +1976,9 @@ static enum packet_return packet_query(struct gdb_context* gdbctx) { if (!gdbctx->process) return packet_error; - packet_reply_open_xfer(gdbctx); packet_query_threads(gdbctx); - packet_reply_close_xfer(gdbctx, off, len); + packet_reply_xfer(gdbctx, off, len); + reply_buffer_clear(&gdbctx->qxfer_buffer); return packet_done; } @@ -1979,9 +1987,9 @@ static enum packet_return packet_query(struct gdb_context* gdbctx) if (!gdbctx->process) return packet_error; if (!(cpu = gdbctx->process->be_cpu)) return packet_error; - packet_reply_open_xfer(gdbctx); packet_query_target_xml(gdbctx, cpu); - packet_reply_close_xfer(gdbctx, off, len); + packet_reply_xfer(gdbctx, off, len); + reply_buffer_clear(&gdbctx->qxfer_buffer); return packet_done; } break; @@ -2299,6 +2307,8 @@ static BOOL gdb_init_context(struct gdb_context* gdbctx, unsigned flags, unsigne for (i = 0; i < ARRAY_SIZE(gdbctx->wine_segs); i++) gdbctx->wine_segs[i] = 0; + memset(&gdbctx->qxfer_buffer, 0, sizeof(gdbctx->qxfer_buffer)); + /* wait for first trap */ while (WaitForDebugEvent(&gdbctx->de, INFINITE)) { -- 2.31.1