[PATCH] gdbstub: use 0 ("any process") on packets with no PID

Matheus Tavares Bernardino posted 1 patch 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/78a3b06f6ab90a7ff8e73ae14a996eb27ec76c85.1690904195.git.quic._5Fmathbern@quicinc.com
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>
gdbstub/gdbstub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] gdbstub: use 0 ("any process") on packets with no PID
Posted by Matheus Tavares Bernardino 9 months ago
Previously, qemu-user would always report PID 1 to GDB. This was changed
at dc14a7a6e9 (gdbstub: Report the actual qemu-user pid, 2023-06-30),
but read_thread_id() still considers GDB packets with "no PID" as "PID
1", which is not the qemu-user PID. Fix that by parsing "no PID" as "0",
which the GDB Remote Protocol defines as "any process".

Note that this should have no effect for system emulation as, in this
case, gdb_create_default_process() will assign PID 1 for the first
process and that is what the gdbstub uses for GDB requests with no PID,
or PID 0.

This issue was found with hexagon-lldb, which sends a "Hq" packet with
only the thread-id, but no process-id, leading to the invalid usage of
"PID 1" by qemu-hexagon and a subsequent "E22" reply.

Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
---
 gdbstub/gdbstub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index ce8b42eb15..e74ecc78cc 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -537,7 +537,7 @@ static GDBThreadIdKind read_thread_id(const char *buf, const char **end_buf,
         /* Skip '.' */
         buf++;
     } else {
-        p = 1;
+        p = 0;
     }
 
     ret = qemu_strtoul(buf, &buf, 16, &t);
-- 
2.37.2
Re: [PATCH] gdbstub: use 0 ("any process") on packets with no PID
Posted by Ilya Leoshkevich 9 months ago
On Tue, 2023-08-01 at 12:37 -0300, Matheus Tavares Bernardino wrote:
> Previously, qemu-user would always report PID 1 to GDB. This was
> changed
> at dc14a7a6e9 (gdbstub: Report the actual qemu-user pid, 2023-06-30),
> but read_thread_id() still considers GDB packets with "no PID" as
> "PID
> 1", which is not the qemu-user PID. Fix that by parsing "no PID" as
> "0",
> which the GDB Remote Protocol defines as "any process".
> 
> Note that this should have no effect for system emulation as, in this
> case, gdb_create_default_process() will assign PID 1 for the first
> process and that is what the gdbstub uses for GDB requests with no
> PID,
> or PID 0.
> 
> This issue was found with hexagon-lldb, which sends a "Hq" packet
> with
> only the thread-id, but no process-id, leading to the invalid usage
> of
> "PID 1" by qemu-hexagon and a subsequent "E22" reply.

Did you mean "Hg"?

> Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
> ---
>  gdbstub/gdbstub.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

The change looks good to me.
Thanks for looking into this and sorry for the breakage.

Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
Re: [PATCH] gdbstub: use 0 ("any process") on packets with no PID
Posted by Matheus Tavares Bernardino 9 months ago
Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> On Tue, 2023-08-01 at 12:37 -0300, Matheus Tavares Bernardino wrote:
> > Previously, qemu-user would always report PID 1 to GDB. This was
> > changed
> > at dc14a7a6e9 (gdbstub: Report the actual qemu-user pid, 2023-06-30),
> > but read_thread_id() still considers GDB packets with "no PID" as
> > "PID
> > 1", which is not the qemu-user PID. Fix that by parsing "no PID" as
> > "0",
> > which the GDB Remote Protocol defines as "any process".
> > 
> > Note that this should have no effect for system emulation as, in this
> > case, gdb_create_default_process() will assign PID 1 for the first
> > process and that is what the gdbstub uses for GDB requests with no
> > PID,
> > or PID 0.
> > 
> > This issue was found with hexagon-lldb, which sends a "Hq" packet
> > with
> > only the thread-id, but no process-id, leading to the invalid usage
> > of
> > "PID 1" by qemu-hexagon and a subsequent "E22" reply.
>
> Did you mean "Hg"?

Oops, that's right, thanks.
Re: [PATCH] gdbstub: use 0 ("any process") on packets with no PID
Posted by Richard Henderson 9 months ago
On 8/2/23 04:04, Matheus Tavares Bernardino wrote:
> Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>>
>> On Tue, 2023-08-01 at 12:37 -0300, Matheus Tavares Bernardino wrote:
>>> Previously, qemu-user would always report PID 1 to GDB. This was
>>> changed
>>> at dc14a7a6e9 (gdbstub: Report the actual qemu-user pid, 2023-06-30),
>>> but read_thread_id() still considers GDB packets with "no PID" as
>>> "PID
>>> 1", which is not the qemu-user PID. Fix that by parsing "no PID" as
>>> "0",
>>> which the GDB Remote Protocol defines as "any process".
>>>
>>> Note that this should have no effect for system emulation as, in this
>>> case, gdb_create_default_process() will assign PID 1 for the first
>>> process and that is what the gdbstub uses for GDB requests with no
>>> PID,
>>> or PID 0.
>>>
>>> This issue was found with hexagon-lldb, which sends a "Hq" packet
>>> with
>>> only the thread-id, but no process-id, leading to the invalid usage
>>> of
>>> "PID 1" by qemu-hexagon and a subsequent "E22" reply.
>>
>> Did you mean "Hg"?
> 
> Oops, that's right, thanks.
> 

Queued to tcg-next, with the typo fixed.


r~