Re: [PATCH] kgdb: Fix buffer overflow in the qfThreadInfo packet handler

[Doug Anderson]

Hi,

On Tue, Nov 3, 2026 at 11:06 AM Christian Aurich Zanettini Martins
<christian.aurichzm@xxxxxxxxx> wrote:
>
> gdb_cmd_query() answers qfThreadInfo with a thread ID and a comma for
> every online CPU, 9 bytes each, followed by the first page of tasks.
> Only the tasks are paged, KGDB_MAX_THREAD_QUERY at a time, so the CPU
> entries overrun remcom_out_buffer[BUFMAX] once there are enough CPUs.
> On x86, where BUFMAX is 1024, a typical boot hits this from 109 online
> CPUs (222 on arm64 and riscv), and GDB sends qfThreadInfo as soon as it
> connects:
>
>   BUG: KASAN: global-out-of-bounds in pack_threadid+0x2a8/0x2e0
>   Write of size 1 at addr ffffffff86eecc20 by task swapper/0/1
>   ...
>   The buggy address belongs to the variable:
>    remcom_out_buffer+0x400/0x420
>
> Page the CPU entries together with the tasks, so no reply holds more
> than KGDB_MAX_THREAD_QUERY threads. The CPUs still come first, which
> scripts/gdb relies on to detect KGDB.
>
> Fixes: 25fc99991383 ("kgdb: fix gdb serial thread queries")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Christian Aurich Zanettini Martins <christian.aurichzm@xxxxxxxxx>
> ---
>
> Notes:
>     Tested in QEMU with KASAN and GDB 15.1. Unpatched, connecting GDB to
>     a 109-CPU guest gives the report above; patched, 109, 128 and 255 CPUs
>     return the full thread list with no report.
>
>     Based on v7.4-rc1.
>
>  kernel/debug/gdbstub.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c
> index fc6a7a8c1f6e..02f27e70993c 100644
> --- a/kernel/debug/gdbstub.c
> +++ b/kernel/debug/gdbstub.c
> @@ -752,15 +752,20 @@ static void gdb_cmd_query(struct kgdb_state *ks)
>                 i = 0;
>                 remcom_out_buffer[0] = 'm';
>                 ptr = remcom_out_buffer + 1;
> -               if (remcom_in_buffer[1] == 'f') {
> -                       /* Each cpu is a shadow thread */
> -                       for_each_online_cpu(cpu) {
> -                               ks->thr_query = 0;
> +               if (remcom_in_buffer[1] == 'f')
> +                       ks->thr_query = 0;
> +
> +               /* Each cpu is a shadow thread */
> +               for_each_online_cpu(cpu) {
> +                       if (i >= ks->thr_query && !finished) {
>                                 int_to_threadref(thref, -cpu - 2);
>                                 ptr = pack_threadid(ptr, thref);
>                                 *(ptr++) = ',';
> -                               i++;
> +                               ks->thr_query++;
> +                               if (ks->thr_query % KGDB_MAX_THREAD_QUERY == 0)
> +                                       finished = 1;
>                         }
> +                       i++;
>                 }

While I'm not an expert on the gdbstub, this looks reasonable to me.
One request I'd make, though: can you unify the CPU and thread code?
Something like:

for_each_online_cpu(cpu)
  append_threadid(ks, &ptr, &i, &finished, -cpu - 2);

for_each_process_thread(g, p)
  append_threadid(ks, &ptr, &i, &finished, p->pid);

-Doug