* [PATCH v2 1/6] linux-user: Expose do_guest_openat() and do_guest_readlink()
2023-05-26 11:50 [PATCH v2 0/6] gdbstub: Add support for info proc mappings Ilya Leoshkevich
@ 2023-05-26 11:50 ` Ilya Leoshkevich
2023-05-26 11:50 ` [PATCH v2 2/6] gdbstub: Expose gdb_get_process() and gdb_get_first_cpu_in_process() Ilya Leoshkevich
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Ilya Leoshkevich @ 2023-05-26 11:50 UTC (permalink / raw)
To: Alex Bennée, Philippe Mathieu-Daudé, Laurent Vivier
Cc: Dominik 'Disconnect3d' Czarnota, Christian Borntraeger,
Andreas Arnez, qemu-devel, Ilya Leoshkevich
These functions will be required by the GDB stub in order to provide
the guest view of /proc to GDB.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
linux-user/qemu.h | 3 +++
linux-user/syscall.c | 54 ++++++++++++++++++++++++++++----------------
2 files changed, 38 insertions(+), 19 deletions(-)
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 92f9f5af41c..a5830ec2396 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -165,6 +165,9 @@ typedef struct TaskState {
} TaskState;
abi_long do_brk(abi_ulong new_brk);
+int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *pathname,
+ int flags, mode_t mode);
+ssize_t do_guest_readlink(const char *pathname, char *buf, size_t bufsiz);
/* user access */
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 89b58b386b1..3e6ed51ce62 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8347,7 +8347,8 @@ static int open_hardware(CPUArchState *cpu_env, int fd)
}
#endif
-static int do_openat(CPUArchState *cpu_env, int dirfd, const char *pathname, int flags, mode_t mode)
+int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *pathname,
+ int flags, mode_t mode)
{
struct fake_open {
const char *filename;
@@ -8418,6 +8419,36 @@ static int do_openat(CPUArchState *cpu_env, int dirfd, const char *pathname, int
return safe_openat(dirfd, path(pathname), flags, mode);
}
+ssize_t do_guest_readlink(const char *pathname, char *buf, size_t bufsiz)
+{
+ ssize_t ret;
+
+ if (!pathname || !buf) {
+ errno = EFAULT;
+ return -1;
+ }
+
+ if (!bufsiz) {
+ /* Short circuit this for the magic exe check. */
+ errno = EINVAL;
+ return -1;
+ }
+
+ if (is_proc_myself((const char *)pathname, "exe")) {
+ /*
+ * Don't worry about sign mismatch as earlier mapping
+ * logic would have thrown a bad address error.
+ */
+ ret = MIN(strlen(exec_path), bufsiz);
+ /* We cannot NUL terminate the string. */
+ memcpy(buf, exec_path, ret);
+ } else {
+ ret = readlink(path(pathname), buf, bufsiz);
+ }
+
+ return ret;
+}
+
static int do_execveat(CPUArchState *cpu_env, int dirfd,
abi_long pathname, abi_long guest_argp,
abi_long guest_envp, int flags)
@@ -8892,7 +8923,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
case TARGET_NR_open:
if (!(p = lock_user_string(arg1)))
return -TARGET_EFAULT;
- ret = get_errno(do_openat(cpu_env, AT_FDCWD, p,
+ ret = get_errno(do_guest_openat(cpu_env, AT_FDCWD, p,
target_to_host_bitmask(arg2, fcntl_flags_tbl),
arg3));
fd_trans_unregister(ret);
@@ -8902,7 +8933,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
case TARGET_NR_openat:
if (!(p = lock_user_string(arg2)))
return -TARGET_EFAULT;
- ret = get_errno(do_openat(cpu_env, arg1, p,
+ ret = get_errno(do_guest_openat(cpu_env, arg1, p,
target_to_host_bitmask(arg3, fcntl_flags_tbl),
arg4));
fd_trans_unregister(ret);
@@ -10127,22 +10158,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
void *p2;
p = lock_user_string(arg1);
p2 = lock_user(VERIFY_WRITE, arg2, arg3, 0);
- if (!p || !p2) {
- ret = -TARGET_EFAULT;
- } else if (!arg3) {
- /* Short circuit this for the magic exe check. */
- ret = -TARGET_EINVAL;
- } else if (is_proc_myself((const char *)p, "exe")) {
- /*
- * Don't worry about sign mismatch as earlier mapping
- * logic would have thrown a bad address error.
- */
- ret = MIN(strlen(exec_path), arg3);
- /* We cannot NUL terminate the string. */
- memcpy(p2, exec_path, ret);
- } else {
- ret = get_errno(readlink(path(p), p2, arg3));
- }
+ ret = get_errno(do_guest_readlink(p, p2, arg3));
unlock_user(p2, arg2, ret);
unlock_user(p, arg1, 0);
}
--
2.40.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 2/6] gdbstub: Expose gdb_get_process() and gdb_get_first_cpu_in_process()
2023-05-26 11:50 [PATCH v2 0/6] gdbstub: Add support for info proc mappings Ilya Leoshkevich
2023-05-26 11:50 ` [PATCH v2 1/6] linux-user: Expose do_guest_openat() and do_guest_readlink() Ilya Leoshkevich
@ 2023-05-26 11:50 ` Ilya Leoshkevich
2023-05-26 11:50 ` [PATCH v2 3/6] gdbstub: Report the actual qemu-user pid Ilya Leoshkevich
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Ilya Leoshkevich @ 2023-05-26 11:50 UTC (permalink / raw)
To: Alex Bennée, Philippe Mathieu-Daudé, Laurent Vivier
Cc: Dominik 'Disconnect3d' Czarnota, Christian Borntraeger,
Andreas Arnez, qemu-devel, Ilya Leoshkevich
These functions will be needed by user-target.c in order to retrieve
the name of the executable.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
gdbstub/gdbstub.c | 16 ++++++++--------
gdbstub/internals.h | 2 ++
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index be18568d0af..9139fec92af 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -211,7 +211,7 @@ static uint32_t gdb_get_cpu_pid(CPUState *cpu)
return cpu->cluster_index + 1;
}
-static GDBProcess *gdb_get_process(uint32_t pid)
+GDBProcess *gdb_get_process(uint32_t pid)
{
int i;
@@ -247,7 +247,7 @@ static CPUState *find_cpu(uint32_t thread_id)
return NULL;
}
-static CPUState *get_first_cpu_in_process(GDBProcess *process)
+CPUState *gdb_get_first_cpu_in_process(GDBProcess *process)
{
CPUState *cpu;
@@ -325,7 +325,7 @@ static CPUState *gdb_get_cpu(uint32_t pid, uint32_t tid)
return NULL;
}
- return get_first_cpu_in_process(process);
+ return gdb_get_first_cpu_in_process(process);
} else {
/* a specific thread */
cpu = find_cpu(tid);
@@ -354,7 +354,7 @@ static const char *get_feature_xml(const char *p, const char **newp,
size_t len;
int i;
const char *name;
- CPUState *cpu = get_first_cpu_in_process(process);
+ CPUState *cpu = gdb_get_first_cpu_in_process(process);
CPUClass *cc = CPU_GET_CLASS(cpu);
len = 0;
@@ -490,7 +490,7 @@ void gdb_register_coprocessor(CPUState *cpu,
static void gdb_process_breakpoint_remove_all(GDBProcess *p)
{
- CPUState *cpu = get_first_cpu_in_process(p);
+ CPUState *cpu = gdb_get_first_cpu_in_process(p);
while (cpu) {
gdb_breakpoint_remove_all(cpu);
@@ -653,7 +653,7 @@ static int gdb_handle_vcont(const char *p)
goto out;
}
- cpu = get_first_cpu_in_process(process);
+ cpu = gdb_get_first_cpu_in_process(process);
while (cpu) {
if (newstates[cpu->cpu_index] == 1) {
newstates[cpu->cpu_index] = cur_action;
@@ -1280,7 +1280,7 @@ static void handle_v_attach(GArray *params, void *user_ctx)
goto cleanup;
}
- cpu = get_first_cpu_in_process(process);
+ cpu = gdb_get_first_cpu_in_process(process);
if (!cpu) {
goto cleanup;
}
@@ -1403,7 +1403,7 @@ static void handle_query_curr_tid(GArray *params, void *user_ctx)
* first thread).
*/
process = gdb_get_cpu_process(gdbserver_state.g_cpu);
- cpu = get_first_cpu_in_process(process);
+ cpu = gdb_get_first_cpu_in_process(process);
g_string_assign(gdbserver_state.str_buf, "QC");
gdb_append_thread_id(cpu, gdbserver_state.str_buf);
gdb_put_strbuf();
diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index 33d21d64886..25e4d5eeaa6 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -129,6 +129,8 @@ void gdb_read_byte(uint8_t ch);
*/
bool gdb_got_immediate_ack(void);
/* utility helpers */
+GDBProcess *gdb_get_process(uint32_t pid);
+CPUState *gdb_get_first_cpu_in_process(GDBProcess *process);
CPUState *gdb_first_attached_cpu(void);
void gdb_append_thread_id(CPUState *cpu, GString *buf);
int gdb_get_cpu_index(CPUState *cpu);
--
2.40.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 3/6] gdbstub: Report the actual qemu-user pid
2023-05-26 11:50 [PATCH v2 0/6] gdbstub: Add support for info proc mappings Ilya Leoshkevich
2023-05-26 11:50 ` [PATCH v2 1/6] linux-user: Expose do_guest_openat() and do_guest_readlink() Ilya Leoshkevich
2023-05-26 11:50 ` [PATCH v2 2/6] gdbstub: Expose gdb_get_process() and gdb_get_first_cpu_in_process() Ilya Leoshkevich
@ 2023-05-26 11:50 ` Ilya Leoshkevich
2023-05-26 11:50 ` [PATCH v2 4/6] gdbstub: Add support for info proc mappings Ilya Leoshkevich
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Ilya Leoshkevich @ 2023-05-26 11:50 UTC (permalink / raw)
To: Alex Bennée, Philippe Mathieu-Daudé, Laurent Vivier
Cc: Dominik 'Disconnect3d' Czarnota, Christian Borntraeger,
Andreas Arnez, qemu-devel, Ilya Leoshkevich
Currently qemu-user reports pid 1 to GDB. Resolve the TODO and report
the actual PID. Using getpid() relies on the assumption that there is
only one GDBProcess. Add an assertion to make sure that future changes
don't break it.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
gdbstub/gdbstub.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 9139fec92af..c7e3ee71f2f 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -202,13 +202,16 @@ void gdb_memtox(GString *buf, const char *mem, int len)
static uint32_t gdb_get_cpu_pid(CPUState *cpu)
{
- /* TODO: In user mode, we should use the task state PID */
+#ifdef CONFIG_USER_ONLY
+ return getpid();
+#else
if (cpu->cluster_index == UNASSIGNED_CLUSTER_INDEX) {
/* Return the default process' PID */
int index = gdbserver_state.process_num - 1;
return gdbserver_state.processes[index].pid;
}
return cpu->cluster_index + 1;
+#endif
}
GDBProcess *gdb_get_process(uint32_t pid)
@@ -2146,19 +2149,25 @@ void gdb_read_byte(uint8_t ch)
void gdb_create_default_process(GDBState *s)
{
GDBProcess *process;
- int max_pid = 0;
+ int pid;
+#ifdef CONFIG_USER_ONLY
+ assert(gdbserver_state.process_num == 0);
+ pid = getpid();
+#else
if (gdbserver_state.process_num) {
- max_pid = s->processes[s->process_num - 1].pid;
+ pid = s->processes[s->process_num - 1].pid;
+ } else {
+ pid = 0;
}
+ /* We need an available PID slot for this process */
+ assert(pid < UINT32_MAX);
+ pid++;
+#endif
s->processes = g_renew(GDBProcess, s->processes, ++s->process_num);
process = &s->processes[s->process_num - 1];
-
- /* We need an available PID slot for this process */
- assert(max_pid < UINT32_MAX);
-
- process->pid = max_pid + 1;
+ process->pid = pid;
process->attached = false;
process->target_xml[0] = '\0';
}
--
2.40.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 4/6] gdbstub: Add support for info proc mappings
2023-05-26 11:50 [PATCH v2 0/6] gdbstub: Add support for info proc mappings Ilya Leoshkevich
` (2 preceding siblings ...)
2023-05-26 11:50 ` [PATCH v2 3/6] gdbstub: Report the actual qemu-user pid Ilya Leoshkevich
@ 2023-05-26 11:50 ` Ilya Leoshkevich
2023-05-26 11:50 ` [PATCH v2 5/6] docs: Document security implications of debugging Ilya Leoshkevich
2023-05-26 11:50 ` [PATCH v2 6/6] tests/tcg: Add a test for info proc mappings Ilya Leoshkevich
5 siblings, 0 replies; 7+ messages in thread
From: Ilya Leoshkevich @ 2023-05-26 11:50 UTC (permalink / raw)
To: Alex Bennée, Philippe Mathieu-Daudé, Laurent Vivier
Cc: Dominik 'Disconnect3d' Czarnota, Christian Borntraeger,
Andreas Arnez, qemu-devel, Ilya Leoshkevich
Currently the GDB's generate-core-file command doesn't work well with
qemu-user: the resulting dumps are huge [1] and at the same time
incomplete (argv and envp are missing). The reason is that GDB has no
access to proc mappings and therefore has to fall back to using
heuristics for discovering them. This is, in turn, because qemu-user
does not implement the Host I/O feature of the GDB Remote Serial
Protocol.
Implement vFile:{open,close,pread,readlink} and also
qXfer:exec-file:read+.
With that, generate-core-file begins to work on aarch64 and s390x,
albeit with two deficiencies:
* GDB still tries to dump the host mappings, because QEMU does not fake
/proc/$PID/smaps (as opposed to /proc/$PID/maps). The user-visible
effect is only a bunch of warnings.
* PT_LOAD segments lack PF_X flags (I have not debugged this).
The impact of these issues on usability is fairly low, so they can be
resolved later.
[1] https://sourceware.org/pipermail/gdb-patches/2023-May/199432.html
Co-developed-by: Dominik 'Disconnect3d' Czarnota <dominik.b.czarnota@gmail.com>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
gdbstub/gdbstub.c | 45 +++++++++++++-
gdbstub/internals.h | 5 ++
gdbstub/user-target.c | 139 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 187 insertions(+), 2 deletions(-)
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index c7e3ee71f2f..d2efefd3528 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -1337,6 +1337,36 @@ static const GdbCmdParseEntry gdb_v_commands_table[] = {
.cmd = "Kill;",
.cmd_startswith = 1
},
+#ifdef CONFIG_USER_ONLY
+ /*
+ * Host I/O Packets. See [1] for details.
+ * [1] https://sourceware.org/gdb/onlinedocs/gdb/Host-I_002fO-Packets.html
+ */
+ {
+ .handler = gdb_handle_v_file_open,
+ .cmd = "File:open:",
+ .cmd_startswith = 1,
+ .schema = "s,L,L0"
+ },
+ {
+ .handler = gdb_handle_v_file_close,
+ .cmd = "File:close:",
+ .cmd_startswith = 1,
+ .schema = "l0"
+ },
+ {
+ .handler = gdb_handle_v_file_pread,
+ .cmd = "File:pread:",
+ .cmd_startswith = 1,
+ .schema = "l,L,L0"
+ },
+ {
+ .handler = gdb_handle_v_file_readlink,
+ .cmd = "File:readlink:",
+ .cmd_startswith = 1,
+ .schema = "s0"
+ },
+#endif
};
static void handle_v_commands(GArray *params, void *user_ctx)
@@ -1482,11 +1512,14 @@ static void handle_query_supported(GArray *params, void *user_ctx)
";ReverseStep+;ReverseContinue+");
}
-#if defined(CONFIG_USER_ONLY) && defined(CONFIG_LINUX)
+#if defined(CONFIG_USER_ONLY)
+#if defined(CONFIG_LINUX)
if (gdbserver_state.c_cpu->opaque) {
g_string_append(gdbserver_state.str_buf, ";qXfer:auxv:read+");
}
#endif
+ g_string_append(gdbserver_state.str_buf, ";qXfer:exec-file:read+");
+#endif
if (params->len &&
strstr(get_param(params, 0)->data, "multiprocess+")) {
@@ -1625,13 +1658,21 @@ static const GdbCmdParseEntry gdb_gen_query_table[] = {
.cmd_startswith = 1,
.schema = "s:l,l0"
},
-#if defined(CONFIG_USER_ONLY) && defined(CONFIG_LINUX)
+#if defined(CONFIG_USER_ONLY)
+#if defined(CONFIG_LINUX)
{
.handler = gdb_handle_query_xfer_auxv,
.cmd = "Xfer:auxv:read::",
.cmd_startswith = 1,
.schema = "l,l0"
},
+#endif
+ {
+ .handler = gdb_handle_query_xfer_exec_file,
+ .cmd = "Xfer:exec-file:read:",
+ .cmd_startswith = 1,
+ .schema = "l:l,l0"
+ },
#endif
{
.handler = gdb_handle_query_attached,
diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index 25e4d5eeaa6..f2b46cce412 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -189,6 +189,11 @@ typedef union GdbCmdVariant {
void gdb_handle_query_rcmd(GArray *params, void *user_ctx); /* softmmu */
void gdb_handle_query_offsets(GArray *params, void *user_ctx); /* user */
void gdb_handle_query_xfer_auxv(GArray *params, void *user_ctx); /*user */
+void gdb_handle_v_file_open(GArray *params, void *user_ctx); /* user */
+void gdb_handle_v_file_close(GArray *params, void *user_ctx); /* user */
+void gdb_handle_v_file_pread(GArray *params, void *user_ctx); /* user */
+void gdb_handle_v_file_readlink(GArray *params, void *user_ctx); /* user */
+void gdb_handle_query_xfer_exec_file(GArray *params, void *user_ctx); /* user */
void gdb_handle_query_attached(GArray *params, void *user_ctx); /* both */
diff --git a/gdbstub/user-target.c b/gdbstub/user-target.c
index fa0e59ec9a5..09df05b5526 100644
--- a/gdbstub/user-target.c
+++ b/gdbstub/user-target.c
@@ -11,6 +11,10 @@
#include "exec/gdbstub.h"
#include "qemu.h"
#include "internals.h"
+#ifdef CONFIG_LINUX
+#include "linux-user/loader.h"
+#include "linux-user/qemu.h"
+#endif
/*
* Map target signal numbers to GDB protocol signal numbers and vice
@@ -281,3 +285,138 @@ void gdb_handle_query_xfer_auxv(GArray *params, void *user_ctx)
gdbserver_state.str_buf->len, true);
}
#endif
+
+static const char *get_filename_param(GArray *params, int i)
+{
+ const char *hex_filename = get_param(params, i)->data;
+ gdb_hextomem(gdbserver_state.mem_buf, hex_filename,
+ strlen(hex_filename) / 2);
+ g_byte_array_append(gdbserver_state.mem_buf, (const guint8 *)"", 1);
+ return (const char *)gdbserver_state.mem_buf->data;
+}
+
+static void hostio_reply_with_data(const void *buf, size_t n)
+{
+ g_string_printf(gdbserver_state.str_buf, "F%lx;", n);
+ gdb_memtox(gdbserver_state.str_buf, buf, n);
+ gdb_put_packet_binary(gdbserver_state.str_buf->str,
+ gdbserver_state.str_buf->len, true);
+}
+
+void gdb_handle_v_file_open(GArray *params, void *user_ctx)
+{
+ const char *filename = get_filename_param(params, 0);
+ uint64_t flags = get_param(params, 1)->val_ull;
+ uint64_t mode = get_param(params, 2)->val_ull;
+
+#ifdef CONFIG_LINUX
+ int fd = do_guest_openat(gdbserver_state.g_cpu->env_ptr, 0, filename,
+ flags, mode);
+#else
+ int fd = open(filename, flags, mode);
+#endif
+ if (fd < 0) {
+ g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
+ } else {
+ g_string_printf(gdbserver_state.str_buf, "F%d", fd);
+ }
+ gdb_put_strbuf();
+}
+
+void gdb_handle_v_file_close(GArray *params, void *user_ctx)
+{
+ int fd = get_param(params, 0)->val_ul;
+
+ if (close(fd) == -1) {
+ g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
+ gdb_put_strbuf();
+ return;
+ }
+
+ gdb_put_packet("F00");
+}
+
+#define BUFSIZ 8192
+
+void gdb_handle_v_file_pread(GArray *params, void *user_ctx)
+{
+ int fd = get_param(params, 0)->val_ul;
+ size_t count = get_param(params, 1)->val_ull;
+ off_t offset = get_param(params, 2)->val_ull;
+
+ size_t bufsiz = MIN(count, BUFSIZ);
+ g_autofree char *buf = g_try_malloc(bufsiz);
+ if (buf == NULL) {
+ gdb_put_packet("E12");
+ return;
+ }
+
+ ssize_t n = pread(fd, buf, bufsiz, offset);
+ if (n < 0) {
+ g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
+ gdb_put_strbuf();
+ return;
+ }
+ hostio_reply_with_data(buf, n);
+}
+
+void gdb_handle_v_file_readlink(GArray *params, void *user_ctx)
+{
+ const char *filename = get_filename_param(params, 0);
+
+ g_autofree char *buf = g_try_malloc(BUFSIZ);
+ if (buf == NULL) {
+ gdb_put_packet("E12");
+ return;
+ }
+
+#ifdef CONFIG_LINUX
+ ssize_t n = do_guest_readlink(filename, buf, BUFSIZ);
+#else
+ ssize_t n = readlink(filename, buf, BUFSIZ);
+#endif
+ if (n < 0) {
+ g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
+ gdb_put_strbuf();
+ return;
+ }
+ hostio_reply_with_data(buf, n);
+}
+
+void gdb_handle_query_xfer_exec_file(GArray *params, void *user_ctx)
+{
+ uint32_t pid = get_param(params, 0)->val_ul;
+ uint32_t offset = get_param(params, 1)->val_ul;
+ uint32_t length = get_param(params, 2)->val_ul;
+
+ GDBProcess *process = gdb_get_process(pid);
+ if (!process) {
+ gdb_put_packet("E00");
+ return;
+ }
+
+ CPUState *cpu = gdb_get_first_cpu_in_process(process);
+ if (!cpu) {
+ gdb_put_packet("E00");
+ return;
+ }
+
+ TaskState *ts = cpu->opaque;
+ if (!ts || !ts->bprm || !ts->bprm->filename) {
+ gdb_put_packet("E00");
+ return;
+ }
+
+ size_t total_length = strlen(ts->bprm->filename);
+ if (offset > total_length) {
+ gdb_put_packet("E00");
+ return;
+ }
+ if (offset + length > total_length) {
+ length = total_length - offset;
+ }
+
+ g_string_printf(gdbserver_state.str_buf, "l%.*s", length,
+ ts->bprm->filename + offset);
+ gdb_put_strbuf();
+}
--
2.40.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 5/6] docs: Document security implications of debugging
2023-05-26 11:50 [PATCH v2 0/6] gdbstub: Add support for info proc mappings Ilya Leoshkevich
` (3 preceding siblings ...)
2023-05-26 11:50 ` [PATCH v2 4/6] gdbstub: Add support for info proc mappings Ilya Leoshkevich
@ 2023-05-26 11:50 ` Ilya Leoshkevich
2023-05-26 11:50 ` [PATCH v2 6/6] tests/tcg: Add a test for info proc mappings Ilya Leoshkevich
5 siblings, 0 replies; 7+ messages in thread
From: Ilya Leoshkevich @ 2023-05-26 11:50 UTC (permalink / raw)
To: Alex Bennée, Philippe Mathieu-Daudé, Laurent Vivier
Cc: Dominik 'Disconnect3d' Czarnota, Christian Borntraeger,
Andreas Arnez, qemu-devel, Ilya Leoshkevich
Now that the GDB stub explicitly implements reading host files (note
that it was already possible by changing the emulated code to open and
read those files), concerns may arise that it undermines security.
Document the status quo, which is that the users are already
responsible for securing the GDB connection themselves.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
docs/system/gdb.rst | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/docs/system/gdb.rst b/docs/system/gdb.rst
index 7d3718deefb..9906991b841 100644
--- a/docs/system/gdb.rst
+++ b/docs/system/gdb.rst
@@ -214,3 +214,18 @@ The memory mode can be checked by sending the following command:
``maintenance packet Qqemu.PhyMemMode:0``
This will change it back to normal memory mode.
+
+Security considerations
+=======================
+
+Connecting to the GDB socket allows running arbitrary code inside the guest;
+in case of the TCG emulation, which is not considered a security boundary, this
+also means running arbitrary code on the host. Additionally, when debugging
+qemu-user, it allows directly downloading any file readable by QEMU from the
+host.
+
+The GDB socket is not protected by authentication, authorization or encryption.
+It is therefore a responsibility of the user to make sure that only authorized
+clients can connect to it, e.g., by using a unix socket with proper
+permissions, or by opening a TCP socket only on interfaces that are not
+reachable by potential attackers.
--
2.40.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 6/6] tests/tcg: Add a test for info proc mappings
2023-05-26 11:50 [PATCH v2 0/6] gdbstub: Add support for info proc mappings Ilya Leoshkevich
` (4 preceding siblings ...)
2023-05-26 11:50 ` [PATCH v2 5/6] docs: Document security implications of debugging Ilya Leoshkevich
@ 2023-05-26 11:50 ` Ilya Leoshkevich
5 siblings, 0 replies; 7+ messages in thread
From: Ilya Leoshkevich @ 2023-05-26 11:50 UTC (permalink / raw)
To: Alex Bennée, Philippe Mathieu-Daudé, Laurent Vivier
Cc: Dominik 'Disconnect3d' Czarnota, Christian Borntraeger,
Andreas Arnez, qemu-devel, Ilya Leoshkevich
Add a small test to prevent regressions.
Since there are issues with how GDB interprets QEMU's target.xml,
enable the test only on aarch64 and s390x for now.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tests/tcg/aarch64/Makefile.target | 3 +-
tests/tcg/multiarch/Makefile.target | 7 +++
.../multiarch/gdbstub/test-proc-mappings.py | 55 +++++++++++++++++++
tests/tcg/s390x/Makefile.target | 2 +-
4 files changed, 65 insertions(+), 2 deletions(-)
create mode 100644 tests/tcg/multiarch/gdbstub/test-proc-mappings.py
diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
index 03157954871..38402b0ba1f 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -97,7 +97,8 @@ run-gdbstub-sve-ioctls: sve-ioctls
--bin $< --test $(AARCH64_SRC)/gdbstub/test-sve-ioctl.py, \
basic gdbstub SVE ZLEN support)
-EXTRA_RUNS += run-gdbstub-sysregs run-gdbstub-sve-ioctls
+EXTRA_RUNS += run-gdbstub-sysregs run-gdbstub-sve-ioctls \
+ run-gdbstub-proc-mappings
endif
endif
diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
index 373db696481..cbc0b75787a 100644
--- a/tests/tcg/multiarch/Makefile.target
+++ b/tests/tcg/multiarch/Makefile.target
@@ -81,6 +81,13 @@ run-gdbstub-qxfer-auxv-read: sha1
--bin $< --test $(MULTIARCH_SRC)/gdbstub/test-qxfer-auxv-read.py, \
basic gdbstub qXfer:auxv:read support)
+run-gdbstub-proc-mappings: sha1
+ $(call run-test, $@, $(GDB_SCRIPT) \
+ --gdb $(HAVE_GDB_BIN) \
+ --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
+ --bin $< --test $(MULTIARCH_SRC)/gdbstub/test-proc-mappings.py, \
+ proc mappings support)
+
run-gdbstub-thread-breakpoint: testthread
$(call run-test, $@, $(GDB_SCRIPT) \
--gdb $(HAVE_GDB_BIN) \
diff --git a/tests/tcg/multiarch/gdbstub/test-proc-mappings.py b/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
new file mode 100644
index 00000000000..657e36a2fc7
--- /dev/null
+++ b/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
@@ -0,0 +1,55 @@
+"""Test that gdbstub has access to proc mappings.
+
+This runs as a sourced script (via -x, via run-test.py)."""
+from __future__ import print_function
+import gdb
+import sys
+
+
+n_failures = 0
+
+
+def report(cond, msg):
+ """Report success/fail of a test"""
+ if cond:
+ print("PASS: {}".format(msg))
+ else:
+ print("FAIL: {}".format(msg))
+ global n_failures
+ n_failures += 1
+
+
+def run_test():
+ """Run through the tests one by one"""
+ mappings = gdb.execute("info proc mappings", False, True)
+ report(isinstance(mappings, str), "Fetched the mappings from the inferior")
+ report("/sha1" in mappings, "Found the test binary name in the mappings")
+
+
+def main():
+ """Prepare the environment and run through the tests"""
+ try:
+ inferior = gdb.selected_inferior()
+ print("ATTACHED: {}".format(inferior.architecture().name()))
+ except (gdb.error, AttributeError):
+ print("SKIPPING (not connected)")
+ exit(0)
+
+ if gdb.parse_and_eval('$pc') == 0:
+ print("SKIP: PC not set")
+ exit(0)
+
+ try:
+ # These are not very useful in scripts
+ gdb.execute("set pagination off")
+ gdb.execute("set confirm off")
+
+ # Run the actual tests
+ run_test()
+ except gdb.error:
+ report(False, "GDB Exception: {}".format(sys.exc_info()[0]))
+ print("All tests complete: %d failures" % n_failures)
+ exit(n_failures)
+
+
+main()
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 23dc8b6a63f..73f7cb828e3 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -75,7 +75,7 @@ run-gdbstub-signals-s390x: signals-s390x
--bin $< --test $(S390X_SRC)/gdbstub/test-signals-s390x.py, \
mixing signals and debugging)
-EXTRA_RUNS += run-gdbstub-signals-s390x
+EXTRA_RUNS += run-gdbstub-signals-s390x run-gdbstub-proc-mappings
endif
# MVX versions of sha512
--
2.40.1
^ permalink raw reply related [flat|nested] 7+ messages in thread