* [PATCH v4 1/8] linux-user: Expose do_guest_openat() and do_guest_readlink()
2023-06-21 20:36 [PATCH v4 0/8] gdbstub: Add support for info proc mappings Ilya Leoshkevich
@ 2023-06-21 20:36 ` Ilya Leoshkevich
2023-06-21 20:36 ` [PATCH v4 2/8] linux-user: Add "safe" parameter to do_guest_openat() Ilya Leoshkevich
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2023-06-21 20:36 UTC (permalink / raw)
To: Alex Bennée, Laurent Vivier, Peter Maydell,
Richard Henderson, David Hildenbrand
Cc: Philippe Mathieu-Daudé, qemu-devel, qemu-arm, qemu-s390x,
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>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
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 92f9f5af41..a5830ec239 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 f2cb101d83..fa83737192 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8448,7 +8448,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;
@@ -8520,6 +8521,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)
@@ -8994,7 +9025,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);
@@ -9004,7 +9035,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);
@@ -10229,22 +10260,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] 10+ messages in thread* [PATCH v4 2/8] linux-user: Add "safe" parameter to do_guest_openat()
2023-06-21 20:36 [PATCH v4 0/8] gdbstub: Add support for info proc mappings Ilya Leoshkevich
2023-06-21 20:36 ` [PATCH v4 1/8] linux-user: Expose do_guest_openat() and do_guest_readlink() Ilya Leoshkevich
@ 2023-06-21 20:36 ` Ilya Leoshkevich
2023-06-21 20:36 ` [PATCH v4 3/8] linux-user: Emulate /proc/self/smaps Ilya Leoshkevich
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2023-06-21 20:36 UTC (permalink / raw)
To: Alex Bennée, Laurent Vivier, Peter Maydell,
Richard Henderson, David Hildenbrand
Cc: Philippe Mathieu-Daudé, qemu-devel, qemu-arm, qemu-s390x,
Ilya Leoshkevich
gdbstub cannot meaningfully handle QEMU_ERESTARTSYS, and it doesn't
need to. Add a parameter to do_guest_openat() that makes it use
openat() instead of safe_openat(), so that it becomes usable from
gdbstub.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/qemu.h | 2 +-
linux-user/syscall.c | 18 +++++++++++++-----
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index a5830ec239..9b8e0860d7 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -166,7 +166,7 @@ typedef struct 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);
+ int flags, mode_t mode, bool safe);
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 fa83737192..ecd9f5e23d 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8449,7 +8449,7 @@ static int open_hardware(CPUArchState *cpu_env, int fd)
#endif
int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *pathname,
- int flags, mode_t mode)
+ int flags, mode_t mode, bool safe)
{
struct fake_open {
const char *filename;
@@ -8476,7 +8476,11 @@ int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *pathname,
};
if (is_proc_myself(pathname, "exe")) {
- return safe_openat(dirfd, exec_path, flags, mode);
+ if (safe) {
+ return safe_openat(dirfd, exec_path, flags, mode);
+ } else {
+ return openat(dirfd, exec_path, flags, mode);
+ }
}
for (fake_open = fakes; fake_open->filename; fake_open++) {
@@ -8518,7 +8522,11 @@ int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *pathname,
return fd;
}
- return safe_openat(dirfd, path(pathname), flags, mode);
+ if (safe) {
+ return safe_openat(dirfd, path(pathname), flags, mode);
+ } else {
+ return openat(dirfd, path(pathname), flags, mode);
+ }
}
ssize_t do_guest_readlink(const char *pathname, char *buf, size_t bufsiz)
@@ -9027,7 +9035,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
return -TARGET_EFAULT;
ret = get_errno(do_guest_openat(cpu_env, AT_FDCWD, p,
target_to_host_bitmask(arg2, fcntl_flags_tbl),
- arg3));
+ arg3, true));
fd_trans_unregister(ret);
unlock_user(p, arg1, 0);
return ret;
@@ -9037,7 +9045,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
return -TARGET_EFAULT;
ret = get_errno(do_guest_openat(cpu_env, arg1, p,
target_to_host_bitmask(arg3, fcntl_flags_tbl),
- arg4));
+ arg4, true));
fd_trans_unregister(ret);
unlock_user(p, arg2, 0);
return ret;
--
2.40.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 3/8] linux-user: Emulate /proc/self/smaps
2023-06-21 20:36 [PATCH v4 0/8] gdbstub: Add support for info proc mappings Ilya Leoshkevich
2023-06-21 20:36 ` [PATCH v4 1/8] linux-user: Expose do_guest_openat() and do_guest_readlink() Ilya Leoshkevich
2023-06-21 20:36 ` [PATCH v4 2/8] linux-user: Add "safe" parameter to do_guest_openat() Ilya Leoshkevich
@ 2023-06-21 20:36 ` Ilya Leoshkevich
2023-06-21 20:36 ` [PATCH v4 4/8] gdbstub: Expose gdb_get_process() and gdb_get_first_cpu_in_process() Ilya Leoshkevich
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2023-06-21 20:36 UTC (permalink / raw)
To: Alex Bennée, Laurent Vivier, Peter Maydell,
Richard Henderson, David Hildenbrand
Cc: Philippe Mathieu-Daudé, qemu-devel, qemu-arm, qemu-s390x,
Ilya Leoshkevich
/proc/self/smaps is an extension of /proc/self/maps: it provides the
same lines, plus additional information about each range.
GDB uses /proc/self/smaps when available, which means that
generate-core-file tries it first before falling back to
/proc/self/maps. This, in turn, causes it to dump the host mappings,
since /proc/self/smaps is not emulated and is just passed through.
Fix by emulating /proc/self/smaps. Provide true values only for
Size, KernelPageSize, MMUPageSize and VmFlags. Leave all other values
at 0, which is a valid conservative estimate.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/syscall.c | 58 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 57 insertions(+), 1 deletion(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ecd9f5e23d..08162cc966 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8042,7 +8042,36 @@ static int open_self_cmdline(CPUArchState *cpu_env, int fd)
return 0;
}
-static int open_self_maps(CPUArchState *cpu_env, int fd)
+static void show_smaps(int fd, unsigned long size)
+{
+ unsigned long page_size_kb = TARGET_PAGE_SIZE >> 10;
+ unsigned long size_kb = size >> 10;
+
+ dprintf(fd, "Size: %lu kB\n"
+ "KernelPageSize: %lu kB\n"
+ "MMUPageSize: %lu kB\n"
+ "Rss: 0 kB\n"
+ "Pss: 0 kB\n"
+ "Pss_Dirty: 0 kB\n"
+ "Shared_Clean: 0 kB\n"
+ "Shared_Dirty: 0 kB\n"
+ "Private_Clean: 0 kB\n"
+ "Private_Dirty: 0 kB\n"
+ "Referenced: 0 kB\n"
+ "Anonymous: 0 kB\n"
+ "LazyFree: 0 kB\n"
+ "AnonHugePages: 0 kB\n"
+ "ShmemPmdMapped: 0 kB\n"
+ "FilePmdMapped: 0 kB\n"
+ "Shared_Hugetlb: 0 kB\n"
+ "Private_Hugetlb: 0 kB\n"
+ "Swap: 0 kB\n"
+ "SwapPss: 0 kB\n"
+ "Locked: 0 kB\n"
+ "THPeligible: 0\n", size_kb, page_size_kb, page_size_kb);
+}
+
+static int open_self_maps_1(CPUArchState *cpu_env, int fd, bool smaps)
{
CPUState *cpu = env_cpu(cpu_env);
TaskState *ts = cpu->opaque;
@@ -8089,6 +8118,18 @@ static int open_self_maps(CPUArchState *cpu_env, int fd)
} else {
dprintf(fd, "\n");
}
+ if (smaps) {
+ show_smaps(fd, max - min);
+ dprintf(fd, "VmFlags:%s%s%s%s%s%s%s%s\n",
+ (flags & PAGE_READ) ? " rd" : "",
+ (flags & PAGE_WRITE_ORG) ? " wr" : "",
+ (flags & PAGE_EXEC) ? " ex" : "",
+ e->is_priv ? "" : " sh",
+ (flags & PAGE_READ) ? " mr" : "",
+ (flags & PAGE_WRITE_ORG) ? " mw" : "",
+ (flags & PAGE_EXEC) ? " me" : "",
+ e->is_priv ? "" : " ms");
+ }
}
}
@@ -8103,11 +8144,25 @@ static int open_self_maps(CPUArchState *cpu_env, int fd)
" --xp 00000000 00:00 0",
TARGET_VSYSCALL_PAGE, TARGET_VSYSCALL_PAGE + TARGET_PAGE_SIZE);
dprintf(fd, "%*s%s\n", 73 - count, "", "[vsyscall]");
+ if (smaps) {
+ show_smaps(fd, TARGET_PAGE_SIZE);
+ dprintf(fd, "VmFlags: ex\n");
+ }
#endif
return 0;
}
+static int open_self_maps(CPUArchState *cpu_env, int fd)
+{
+ return open_self_maps_1(cpu_env, fd, false);
+}
+
+static int open_self_smaps(CPUArchState *cpu_env, int fd)
+{
+ return open_self_maps_1(cpu_env, fd, true);
+}
+
static int open_self_stat(CPUArchState *cpu_env, int fd)
{
CPUState *cpu = env_cpu(cpu_env);
@@ -8459,6 +8514,7 @@ int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *pathname,
const struct fake_open *fake_open;
static const struct fake_open fakes[] = {
{ "maps", open_self_maps, is_proc_myself },
+ { "smaps", open_self_smaps, is_proc_myself },
{ "stat", open_self_stat, is_proc_myself },
{ "auxv", open_self_auxv, is_proc_myself },
{ "cmdline", open_self_cmdline, is_proc_myself },
--
2.40.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 4/8] gdbstub: Expose gdb_get_process() and gdb_get_first_cpu_in_process()
2023-06-21 20:36 [PATCH v4 0/8] gdbstub: Add support for info proc mappings Ilya Leoshkevich
` (2 preceding siblings ...)
2023-06-21 20:36 ` [PATCH v4 3/8] linux-user: Emulate /proc/self/smaps Ilya Leoshkevich
@ 2023-06-21 20:36 ` Ilya Leoshkevich
2023-06-21 20:36 ` [PATCH v4 5/8] gdbstub: Report the actual qemu-user pid Ilya Leoshkevich
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2023-06-21 20:36 UTC (permalink / raw)
To: Alex Bennée, Laurent Vivier, Peter Maydell,
Richard Henderson, David Hildenbrand
Cc: Philippe Mathieu-Daudé, qemu-devel, qemu-arm, qemu-s390x,
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 be18568d0a..9139fec92a 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 33d21d6488..25e4d5eeaa 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] 10+ messages in thread* [PATCH v4 5/8] gdbstub: Report the actual qemu-user pid
2023-06-21 20:36 [PATCH v4 0/8] gdbstub: Add support for info proc mappings Ilya Leoshkevich
` (3 preceding siblings ...)
2023-06-21 20:36 ` [PATCH v4 4/8] gdbstub: Expose gdb_get_process() and gdb_get_first_cpu_in_process() Ilya Leoshkevich
@ 2023-06-21 20:36 ` Ilya Leoshkevich
2023-06-21 20:36 ` [PATCH v4 6/8] gdbstub: Add support for info proc mappings Ilya Leoshkevich
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2023-06-21 20:36 UTC (permalink / raw)
To: Alex Bennée, Laurent Vivier, Peter Maydell,
Richard Henderson, David Hildenbrand
Cc: Philippe Mathieu-Daudé, qemu-devel, qemu-arm, qemu-s390x,
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 9139fec92a..c7e3ee71f2 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] 10+ messages in thread* [PATCH v4 6/8] gdbstub: Add support for info proc mappings
2023-06-21 20:36 [PATCH v4 0/8] gdbstub: Add support for info proc mappings Ilya Leoshkevich
` (4 preceding siblings ...)
2023-06-21 20:36 ` [PATCH v4 5/8] gdbstub: Report the actual qemu-user pid Ilya Leoshkevich
@ 2023-06-21 20:36 ` Ilya Leoshkevich
2023-06-21 20:36 ` [PATCH v4 7/8] docs: Document security implications of debugging Ilya Leoshkevich
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2023-06-21 20:36 UTC (permalink / raw)
To: Alex Bennée, Laurent Vivier, Peter Maydell,
Richard Henderson, David Hildenbrand
Cc: Philippe Mathieu-Daudé, qemu-devel, qemu-arm, qemu-s390x,
Ilya Leoshkevich, Dominik 'Disconnect3d' Czarnota
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.
[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 c7e3ee71f2..d2efefd352 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 25e4d5eeaa..f2b46cce41 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 fa0e59ec9a..5f0098c806 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%zx;", 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, false);
+#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] 10+ messages in thread* [PATCH v4 7/8] docs: Document security implications of debugging
2023-06-21 20:36 [PATCH v4 0/8] gdbstub: Add support for info proc mappings Ilya Leoshkevich
` (5 preceding siblings ...)
2023-06-21 20:36 ` [PATCH v4 6/8] gdbstub: Add support for info proc mappings Ilya Leoshkevich
@ 2023-06-21 20:36 ` Ilya Leoshkevich
2023-06-21 20:36 ` [PATCH v4 8/8] tests/tcg: Add a test for info proc mappings Ilya Leoshkevich
2023-06-27 15:14 ` [PATCH v4 0/8] gdbstub: Add support " Alex Bennée
8 siblings, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2023-06-21 20:36 UTC (permalink / raw)
To: Alex Bennée, Laurent Vivier, Peter Maydell,
Richard Henderson, David Hildenbrand
Cc: Philippe Mathieu-Daudé, qemu-devel, qemu-arm, qemu-s390x,
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 7d3718deef..9906991b84 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] 10+ messages in thread* [PATCH v4 8/8] tests/tcg: Add a test for info proc mappings
2023-06-21 20:36 [PATCH v4 0/8] gdbstub: Add support for info proc mappings Ilya Leoshkevich
` (6 preceding siblings ...)
2023-06-21 20:36 ` [PATCH v4 7/8] docs: Document security implications of debugging Ilya Leoshkevich
@ 2023-06-21 20:36 ` Ilya Leoshkevich
2023-06-27 15:14 ` [PATCH v4 0/8] gdbstub: Add support " Alex Bennée
8 siblings, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2023-06-21 20:36 UTC (permalink / raw)
To: Alex Bennée, Laurent Vivier, Peter Maydell,
Richard Henderson, David Hildenbrand
Cc: Philippe Mathieu-Daudé, qemu-devel, qemu-arm, qemu-s390x,
Ilya Leoshkevich
Add a small test to prevent regressions.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tests/tcg/multiarch/Makefile.target | 9 ++-
.../multiarch/gdbstub/test-proc-mappings.py | 65 +++++++++++++++++++
2 files changed, 73 insertions(+), 1 deletion(-)
create mode 100644 tests/tcg/multiarch/gdbstub/test-proc-mappings.py
diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
index 373db69648..43bddeaf21 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) \
@@ -97,7 +104,7 @@ run-gdbstub-%:
$(call skip-test, "gdbstub test $*", "need working gdb")
endif
EXTRA_RUNS += run-gdbstub-sha1 run-gdbstub-qxfer-auxv-read \
- run-gdbstub-thread-breakpoint
+ run-gdbstub-proc-mappings run-gdbstub-thread-breakpoint
# ARM Compatible Semi Hosting Tests
#
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 0000000000..a23bbcee7f
--- /dev/null
+++ b/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
@@ -0,0 +1,65 @@
+"""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"""
+ try:
+ mappings = gdb.execute("info proc mappings", False, True)
+ except gdb.error as exc:
+ exc_str = str(exc)
+ if "Not supported on this target." in exc_str:
+ # Detect failures due to an outstanding issue with how GDB handles
+ # the x86_64 QEMU's target.xml, which does not contain the
+ # definition of orig_rax. Skip the test in this case.
+ print("SKIP: {}".format(exc_str))
+ return
+ raise
+ 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()
--
2.40.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v4 0/8] gdbstub: Add support for info proc mappings
2023-06-21 20:36 [PATCH v4 0/8] gdbstub: Add support for info proc mappings Ilya Leoshkevich
` (7 preceding siblings ...)
2023-06-21 20:36 ` [PATCH v4 8/8] tests/tcg: Add a test for info proc mappings Ilya Leoshkevich
@ 2023-06-27 15:14 ` Alex Bennée
8 siblings, 0 replies; 10+ messages in thread
From: Alex Bennée @ 2023-06-27 15:14 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Laurent Vivier, Peter Maydell, Richard Henderson,
David Hildenbrand, Philippe Mathieu-Daudé, qemu-devel,
qemu-arm, qemu-s390x
Ilya Leoshkevich <iii@linux.ibm.com> writes:
> v3: https://lists.gnu.org/archive/html/qemu-devel/2023-06/msg01311.html
> v3 -> v4: Fix the 32-bit build (Alex).
> Enable the test on all architectures and ignore certain
> expected failures (Alex). I tried this with the latest
> gdb-multiarch and it works. The only skip is on x86_64,
> as expected.
>
> v2: https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg06837.html
> v2 -> v3: Use openat() instead of safe_openat() (new patch: 2/8).
> Add /proc/self/smaps emulation (new patch: 3/8).
> With these 2 changes, the minor issues previously mentioned in
> the patch 6/8 are gone.
>
> v1: https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg02614.html
> v1 -> v2: Reword the 5/6 commit message (Dominik).
> Add R-bs.
> Patches that need review:
> 4/6 gdbstub: Add support for info proc mappings
> 6/6 tests/tcg: Add a test for info proc mappings
>
> Hi,
>
> this series partially implements the Host I/O feature of the GDB Remote
> Serial Protocol in order to make generate-core-file work with qemu-user.
> It borrows heavily from the abandoned patch by Dominik [1], hence 4/6
> carries the respective Co-developed-by: tag. I also peeked at
> gdbserver/hostio.cc quite a few times.
Queued to gdbstub/next, thanks.
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 10+ messages in thread