Re: [PATCH v2] kgdb: Fix buffer overflow in the 'M' and 'X' packet handlers

[Doug Anderson]

Hi,

On Tue, Sep 22, 2026 at 8:49 AM Fang Xieyan <fangxy@xxxxxxxxxxxx> wrote:
>
> write_mem_msg() passes the length claimed by an 'M' or 'X' packet directly
> to kgdb_hex2mem() or kgdb_ebin2mem(). A malformed packet can claim more
> data than was actually received, causing the decoders to read past
> remcom_in_buffer.
>
> Record the received payload length in get_packet() and use it to bound
> the decoders. 'M' carries two hex characters per byte, while 'X' carries
> one byte per output byte except that a 0x7d escape consumes an additional
> input byte. Pass the packet end to kgdb_ebin2mem() so both reads are
> checked against the received data.
>
> Keep zero-length 'X' packets valid, as GDB uses them to probe binary
> download support.

My analysis shows that the above isn't true and that zero-length 'X'
packets are no longer valid with your patch. I haven't tested this
myself, though. Yell if I got it wrong.


> @@ -43,6 +43,8 @@
>  /* Our I/O buffers. */
>  static char                    remcom_in_buffer[BUFMAX];
>  static char                    remcom_out_buffer[BUFMAX];
> +/* Payload bytes get_packet() stored in remcom_in_buffer, excluding the NUL. */
> +static int                     remcom_in_len;

I don't love making this a global. Can we just pass it down? ...and
maybe for functions where you pass the length you also pass
"remcom_in_buffer" as a parameter and stop accessing the global?

-Doug