| Message ID | 20181010154553.11515-1-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fix buffer overrun in fetch_register_using_p | expand |
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
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")); regp[i++] = fromhex (p[0]) * 16 + fromhex (p[1]); p += 2;