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

[Markus Armbruster <armbru@redhat.com>]

Philippe Mathieu-Daudé <philmd@redhat.com> 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 <armbru@redhat.com>
>> ---
>>  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 < ' ') {


> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks!

>> +            if (ch < ' ' || ch == '#' || ch == '$' || ch > 126) {
>>                  /* invalid RLE count encoding */
>>                  trace_gdbstub_err_invalid_repeat((uint8_t)ch);
>>                  s->state = RS_GETLINE;
>>