Re: [patch] [gdbserver] Fix memory corruption

Pedro Alves
2011-03-02T15:35:25Z

On Tuesday 01 March 2011 21:34:28, Jan Kratochvil wrote:
> Hi,
>
> gdb.server/ext-run.exp always crashes during the nightly regression tests:
> info os processes
> memory clobbered past end of allocated block
> Remote communication error. Target disconnected.: Connection reset by peer.
> (gdb) FAIL: gdb.server/ext-run.exp: get process list (pattern 1)
>
> Probably OK to check in but I rather ask.
>
> To make it easily reproducible one can disable try_rle() by patching it:
> +return 1;
> /* Don't go past '~'. */

I can't reproduce it.

>
> So that putpkt_binary_1's cnt == 16383 will overrun PBUFSIZ 16384 by 4 bytes.

How did you get that large of a `cnt' in the first place? The largest
I get is 16379.

gdb does:

/* Request only enough to fit in a single packet. The actual data
may not, since we don't know how much of it will need to be escaped;
the target is free to respond with slightly less data. We subtract
five to account for the response type and the protocol frame. */
n = min (get_remote_packet_size () - 5, len);
snprintf (rs->buf, get_remote_packet_size () - 4, "qXfer:%s:read:%s:%s,%s",
object_name, annex ? annex : "",
phex_nz (offset, sizeof offset),
phex_nz (n, sizeof n));

that is, you shouldn't get a read request that big.

It looks like server.c:handle_qxfer's len caping is forgetting
to account for the $, # and checksum (should be fixed), but I don't
think that's the real cause in your example, since it only pushes back
to gdb as much data as it requested.

gdb's putpkt_binary reads:

/* Send a packet to the remote machine, with error checking. The data
of the packet is in BUF. The string in BUF can be at most
get_remote_packet_size () - 5 to account for the $, # and checksum,
and for a possible /0 if we are debugging (remote_debug) and want
to print the sent packet as a string. */

gdbserver's version does not have that comment, although it should.

Both versions lack an assertion catching the case of trying
to send too much for the packet buffer size.

>
>
> Thanks,
> Jan
>
>
> gdb/gdbserver/
> 2011-03-01 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * remote-utils.c (putpkt_binary_1): Calculate BUF2 size dynamically.
>
> --- a/gdb/gdbserver/remote-utils.c
> +++ b/gdb/gdbserver/remote-utils.c
> @@ -725,7 +725,7 @@ putpkt_binary_1 (char *buf, int cnt, int is_notif)
> char *p;
> int cc;
>
> - buf2 = xmalloc (PBUFSIZ);
> + buf2 = xmalloc (1 + cnt + 4);
>
> /* Copy the packet into buffer BUF2, encapsulating it
> and giving it a checksum. */
>

--
Pedro Alves