[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] gdbstub: Implement qqemu.Pid packet
From: |
Dominik Czarnota |
Subject: |
Re: [PATCH] gdbstub: Implement qqemu.Pid packet |
Date: |
Wed, 7 May 2025 13:26:24 +0200 |
Hi,
We need the `qqemu.Pid` packet only for the QEMU system part in order to figure out the page tables and to do it efficiently. We do have a fallback to using `monitor info mem` but it is slow, (iirc) doesn't provide all the necessary information and (iirc) isn't implemented on all architectures.
Also, is the QEMU own pid leak such a problem? There are worse issues than that available with current features though I believe they are not really security issues. I will detail this in a private email to you.
Best,
Dominik 'Disconnect3d' Czarnota
Dominik 'Disconnect3d' Czarnota <dominik.b.czarnota@gmail.com> writes:
> This patch adds support for the `qqemu.Pid` packet to the qemu
> gdbstub which can be used by clients to get the QEMU process PID.
>
> This is useful for plugins like Pwndbg [0] or gdb-pt-dump in order to
> inspect the QEMU process memory through the /proc/self/{maps,mem}
> interfaces. Without this feature, they have to rely on doing an
> unreliable pgrep/ps output processing.
That seems a little thin a reason for QEMU to expose its own PID. For
user-mode you can already get that detail through anything using
gdb_append_thread_id().
For system-mode leaking QEMU's own pid seems like an information leak at
best. There are modes like semihosting which give a remote even more
power but you need to at least opt in to that.
>
> This patch has been developed by Patryk, who I included in the
> Co-authored-by and who asked me to send the patch.
>
> [0] https://github.com/pwndbg/pwndbg
> [1] https://github.com/martinradev/gdb-pt-dump
>
> Co-authored-by: Patryk 'patryk4815' Sondej <patryk.sondej@gmail.com>
> Signed-off-by: Dominik 'Disconnect3d' Czarnota <dominik.b.czarnota@gmail.com>
> ---
> gdbstub/gdbstub.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> index 282e13e163..a077c2c5ed 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -1746,6 +1746,12 @@ static void handle_query_qemu_supported(GArray *params, void *user_ctx)
> gdb_put_strbuf();
> }
>
> +static void handle_query_qemu_pid(GArray *params, void *user_ctx)
> +{
> + g_string_printf(gdbserver_state.str_buf, "F%x", getpid());
> + gdb_put_strbuf();
> +}
> +
> static const GdbCmdParseEntry gdb_gen_query_set_common_table[] = {
> /* Order is important if has same prefix */
> {
> @@ -1902,6 +1908,10 @@ static const GdbCmdParseEntry gdb_gen_query_table[] = {
> .handler = handle_query_qemu_supported,
> .cmd = "qemu.Supported",
> },
> + {
> + .handler = handle_query_qemu_pid,
> + .cmd = "qemu.Pid",
> + },
> #ifndef CONFIG_USER_ONLY
> {
> .handler = gdb_handle_query_qemu_phy_mem_mode,
--
Alex Bennée
Virtualisation Tech Lead @ Linaro