Re: [PATCH] Fix buffer overrun in fetch_register_using_p

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2018-10-10 11:45, Richard Henderson wrote:
> If the packet returned from the gdbserver is too long,
> the stack would be clobbered and gdb would crash.
> 
> gdb/
> 	* remote.c (remote_target::fetch_register_using_p): Error if
> 	more data is received than expected in the packet.
> ---
> 
> I am adding SVE support to QEMU's gdbserver stub, and managed to
> tickle this bug in the process.
> 
> 
> r~
> 
> ---
>  gdb/remote.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 724f41cf71..d68faf1046 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -7958,7 +7958,8 @@ remote_target::fetch_register_using_p (struct
> regcache *regcache,
>    struct gdbarch *gdbarch = regcache->arch ();
>    struct remote_state *rs = get_remote_state ();
>    char *buf, *p;
> -  gdb_byte *regp = (gdb_byte *) alloca (register_size (gdbarch, 
> reg->regnum));
> +  int size = register_size (gdbarch, reg->regnum);
> +  gdb_byte *regp = (gdb_byte *) alloca (size);
>    int i;
> 
>    if (packet_support (PACKET_p) == PACKET_DISABLE)
> @@ -8003,6 +8004,8 @@ remote_target::fetch_register_using_p (struct
> regcache *regcache,
>      {
>        if (p[1] == 0)
>  	error (_("fetch_register_using_p: early buf termination"));
> +      if (i == size)
> +	error (_("fetch_register_using_p: late buf termination"));

Hi Richard,

As a user, I don't think I would understand "late buf termination".  I 
think it could be more straightforward, like "the received value is 
larger than the register size".

Otherwise, this is OK, thanks for this.

Simon