@@ -1987,7 +1987,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...)
va_end(va);
}
-static void gdb_read_byte(GDBState *s, int ch)
+static void gdb_read_byte(GDBState *s, uint8_t ch)
{
uint8_t reply;
@@ -2001,7 +2001,7 @@ static void gdb_read_byte(GDBState *s, int ch)
} else if (ch == '+') {
trace_gdbstub_io_got_ack();
} else {
- trace_gdbstub_io_got_unexpected((uint8_t)ch);
+ trace_gdbstub_io_got_unexpected(ch);
}
if (ch == '+' || ch == '$')
@@ -2024,7 +2024,7 @@ static void gdb_read_byte(GDBState *s, int ch)
s->line_sum = 0;
s->state = RS_GETLINE;
} else {
- trace_gdbstub_err_garbage((uint8_t)ch);
+ trace_gdbstub_err_garbage(ch);
}
break;
case RS_GETLINE:
@@ -2070,11 +2070,11 @@ static void gdb_read_byte(GDBState *s, int ch)
*/
if (ch < ' ' || ch == '#' || ch == '$' || ch > 126) {
/* invalid RLE count encoding */
- trace_gdbstub_err_invalid_repeat((uint8_t)ch);
+ trace_gdbstub_err_invalid_repeat(ch);
s->state = RS_GETLINE;
} else {
/* decode repeat length */
- int repeat = (unsigned char)ch - ' ' + 3;
+ int repeat = ch - ' ' + 3;
if (s->line_buf_index + repeat >= sizeof(s->line_buf) - 1) {
/* that many repeats would overrun the command buffer */
trace_gdbstub_err_overrun();
@@ -2096,7 +2096,7 @@ static void gdb_read_byte(GDBState *s, int ch)
case RS_CHKSUM1:
/* get high hex digit of checksum */
if (!isxdigit(ch)) {
- trace_gdbstub_err_checksum_invalid((uint8_t)ch);
+ trace_gdbstub_err_checksum_invalid(ch);
s->state = RS_GETLINE;
break;
}
@@ -2107,7 +2107,7 @@ static void gdb_read_byte(GDBState *s, int ch)
case RS_CHKSUM2:
/* get low hex digit of checksum */
if (!isxdigit(ch)) {
- trace_gdbstub_err_checksum_invalid((uint8_t)ch);
+ trace_gdbstub_err_checksum_invalid(ch);
s->state = RS_GETLINE;
break;
}
gdb_read_byte() passes its @ch argument to isxdigit(). Undefined behavior when the value is negative. Two callers: * gdb_chr_receive() passes an uint8_t value. Safe. * gdb_handlesig() a char value. Unsafe. Not a security issue, because the characters come from the gdb client, which is trusted. The obvious fix would be casting @ch to unsigned char. But note that gdb_read_byte() already casts @ch to uint8_t in many places. Uses of @ch without such a cast: (1) Compare to a character constant with == or != (2) s->linesum += ch (3) Store ch or ch ^ 0x20 into s->line_buf[] (4) Check for invalid RLE count: ch < ' ' || ch == '#' || ch == '$' || ch > 126 (5) Pass to isxdigit() (6) Pass to fromhex() Change the parameter type from int to uint8_t, and drop the now redundant casts. Affects the above uses as follows: (1) No change: the character constants are all non-negative. (2) Effectively no change: we only ever use s->linesum & 0xff, and s->linesum is int. (3) No change: s->line_buf[] is char[]. (4) No change. (5) Avoid undefined behavior. (6) No change: only reached when isxdigit(ch) Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20190514180311.16028-5-armbru@redhat.com> --- gdbstub.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)