Re: [patch] [gdbserver] Fix memory corruption

[Pedro Alves <pedro@codesourcery.com>]

On Wednesday 02 March 2011 16:51:35, Jan Kratochvil wrote:
> On Wed, 02 Mar 2011 16:35:16 +0100, Pedro Alves wrote:
> > I can't reproduce it.
> 
> You may need also -lmcheck for gdbserver.

> Maybe your system does not have
> # cat /proc/*/cmdline|tr -cd '$#}*'|wc -c
> 84

Ah!  Yes, that's the reason.  Thanks.  I had tried valgrind before
and nothing complained.  Running a bunch of
"tail -f }}}}}}}}}}}}}}}}}}/a" processes helped.  :-)

> 
> > > 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.
> 
> remote_escape_output is called with OUT_MAXLEN == PBUFSIZ - 2 == 16382 and

> returns it +1, therefore 16383.
> #8  0x0000000000404ca3 in putpkt_binary_1 (buf=0x258f2c0 "mmn"..., cnt=16381, is_notif=0) at remote-utils.c:801
> 801	  free (buf2);
> 

> Yes, the request is just for 16378 but gdbserver returns more.

I see now.

> Before starting to chase off-by-one here and off-by-one there what is the
> practical purpose of such strict packet limits?

The remote protocol is designed to be implementable in tiny chips as
well, where you typically have a static buffer for the incoming packet
buffer.  malloc is a luxury you don't have in many of those scenarios.
So for outgoing packets, gdb needs to be careful about that.  For
incoming packets, gdb dynamically grows the buffer as it finds its
receiving larger packets.  So I think your patch is indeed okay.
I wouldn't mind a comment explaining the magic numbers, or replacing
them with 'strlen ("$#NN")' like in remote.c:

  /* Compute the size of the actual payload by subtracting out the
     packet header and footer overhead: "$M<memaddr>,<len>:...#nn".  */

  payload_size -= strlen ("$,:#NN");

> When TCP is in use shouldn't the code all around just support arbitrary packet
> sizes?  To get rid any constant buffer sizes etc.  Then there still can remain
> some vague fragmentation on the GDB client side but both sides should be able
> to accept arbitrary packet sizes, shouldn't they?

FYI, gdbserver works with rs232 as well.

-- 
Pedro Alves