Re: [Qemu-devel] [PATCH 3/6] gdbstub: Reject invalid RLE repeat counts

[Philippe Mathieu-Daudé]

On 5/13/19 2:39 PM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <address@hidden> writes:
> 
>> On 4/18/19 4:53 PM, Markus Armbruster wrote:
>>> "Debugging with GDB / Appendix E GDB Remote Serial Protocol /
>>> Overview" specifies "The printable characters '#' and '$' or with a
>>> numeric value greater than 126 must not be used."  gdb_read_byte()
>>> only rejects values < 32.  This is wrong.  Impact depends on the caller:
>>>
>>> * gdb_handlesig() passes a char.  Incorrectly accepts '#', '$' and
>>>   '\127'.
>>>
>>> * gdb_chr_receive() passes an uint8_t.  Additionally accepts
>>>   characters with the most-significant bit set.
>>>
>>> Correct the validity check to match the specification.
>>>
>>> Signed-off-by: Markus Armbruster <address@hidden>
>>> ---
>>>  gdbstub.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/gdbstub.c b/gdbstub.c
>>> index d54abd17cc..a6dce1b027 100644
>>> --- a/gdbstub.c
>>> +++ b/gdbstub.c
>>> @@ -2064,7 +2064,7 @@ static void gdb_read_byte(GDBState *s, int ch)
>>>              }
>>>              break;
>>>          case RS_GETLINE_RLE:
>>> -            if (ch < ' ') {
>>
>> Can you add a comment referring to the ""Debugging with GDB / Appendix E
>> GDB Remote Serial Protocol / Overview" here?
> 
> Like this?
> 
>         case RS_GETLINE_RLE:
>             /*
>              * Run-length encoding is explained in "Debugging with GDB /
>              * Appendix E GDB Remote Serial Protocol / Overview".
>              */
>             if (ch < ' ') {

Yes, thanks!

> 
>> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
> 
> Thanks!
> 
>>> +            if (ch < ' ' || ch == '#' || ch == '$' || ch > 126) {
>>>                  /* invalid RLE count encoding */
>>>                  trace_gdbstub_err_invalid_repeat((uint8_t)ch);
>>>                  s->state = RS_GETLINE;
>>>