[PATCH] gdbstub: Implement qqemu.Pid packet

[PATCH] gdbstub: Implement qqemu.Pid packet

[Dominik 'Disconnect3d' Czarnota]

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.

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,
-- 
2.30.2

Re: [PATCH] gdbstub: Implement qqemu.Pid packet

[Alex Bennée]

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

Re: [PATCH] gdbstub: Implement qqemu.Pid packet

[Dominik Czarnota]

[-- Attachment #1: Type: text/plain, Size: 3085 bytes --]

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

On Fri, 2 May 2025 at 15:18, Alex Bennée <alex.bennee@linaro.org> wrote:

> 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
>

[-- Attachment #2: Type: text/html, Size: 4170 bytes --]

Re: [PATCH] gdbstub: Implement qqemu.Pid packet

[Alex Bennée]

Dominik Czarnota <dominik.b.czarnota@gmail.com> writes:

> 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.

Page tables of QEMU itself or the page tables for the guest its managing?

> 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.

That can be fixed. If it's useful information to clients of QEMU it
would be better to have a well defined API for accessing it.

This is a gdb monitor call and not an HMP one right?

> 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.

Sure - enabling gdbstub inherently gives the user quite privileged
access to QEMU's guests - including the ability to execute arbitrary
syscalls on behalf of the remote.

My main concern is adding support for a non-standard packet that is used
for a hacky workaround when a better solution might be possible.

>
> Best,
> Dominik 'Disconnect3d' Czarnota
>
> On Fri, 2 May 2025 at 15:18, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>  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

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro