interesting
/
riscv-openocd
mirror of https://github.com/riscv/riscv-openocd.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

binarybuffer: Fix str_to_buf() parsing function 

The function str_to_buf() was too benevolent and did
not perform sufficient error checking on the input
string being parsed. Especially:

- Invalid numbers were silently ignored.
- Out-of-range numbers were silently truncated.

The following commands that use str_to_buf()
were affected:

- reg (when writing a register value)
- set_reg
- jtag drscan

This pull request fixes that by:

- Rewriting str_to_buf() to add the missing checks.
- Adding function command_parse_str_to_buf() which can
  be used in command handlers. It parses the input
  numbers and provides user-readable error messages
  in case of parsing errors.

Examples:

jtag drscan 10 huh10

- Old behavior: The string "huh10" is silently
  converted to 10 and the command is then executed.
  No warning error or warning is shown to the user.
- New behavior: Error message is shown:
  "'huh10' is not a valid number"

reg pc 0x123456789

Assuming the "pc" is 32 bits wide:

- Old behavior: The register value is silently
  truncated to 0x23456789 and the command is performed.
- New behavior: Error message is shown to the user:
  "Number 0x123456789 exceeds 32 bits"

Change-Id: I079e19cd153aec853a3c2eb66953024b8542d0f4
Signed-off-by: Jan Matyas <jan.matyas@codasip.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/8315
Tested-by: jenkins
Reviewed-by: Marek Vrbka <marek.vrbka@codasip.com>
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
This commit is contained in:
Jan Matyas 2024-06-03 10:23:02 +02:00 committed by Antonio Borneo
parent c97a8ff10d
commit 53b94fad58
6 changed files with 220 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

166
src/helper/binarybuffer.c
View File

@ -102,7 +102,6 @@ bool buf_cmp_mask(const void *_buf1, const void *_buf2,
return buf_cmp_trailing(buf1[last], buf2[last], mask[last], trailing);
}
void *buf_set_ones(void *_buf, unsigned size)
{
uint8_t *buf = _buf;
@ -206,36 +205,75 @@ char *buf_to_hex_str(const void *_buf, unsigned buf_len)
return str;
}
/** identify radix, and skip radix-prefix (0, 0x or 0X) */
static void str_radix_guess(const char **_str, unsigned *_str_len,
unsigned *_radix)
static bool str_has_hex_prefix(const char *s)
{
unsigned radix = *_radix;
if (radix != 0)
return;
const char *str = *_str;
unsigned str_len = *_str_len;
if (str[0] == '0' && (str[1] == 'x' || str[1] == 'X')) {
radix = 16;
str += 2;
str_len -= 2;
} else if ((str[0] == '0') && (str_len != 1)) {
radix = 8;
str += 1;
str_len -= 1;
} else
radix = 10;
*_str = str;
*_str_len = str_len;
*_radix = radix;
/* Starts with "0x" or "0X" */
return (s[0] == '0') && (s[1] == 'x' || s[1] == 'X');
}
int str_to_buf(const char *str, unsigned str_len,
void *_buf, unsigned buf_len, unsigned radix)
static bool str_has_octal_prefix(const char *s)
{
str_radix_guess(&str, &str_len, &radix);
/* - starts with '0',
* - has at least two characters, and
* - the second character is not 'x' or 'X' */
return (s[0] == '0') && (s[1] != '\0') && (s[1] != 'x') && (s[1] != 'X');
}
float factor;
/**
* Try to identify the radix of the number by looking at its prefix.
* No further validation of the number is preformed.
*/
static unsigned int str_radix_guess(const char *str)
{
assert(str);
if (str_has_hex_prefix(str))
return 16;
if (str_has_octal_prefix(str))
return 8;
/* Otherwise assume a decadic number. */
return 10;
}
/** Strip leading "0x" or "0X" from hex numbers or "0" from octal numbers. */
static void str_strip_number_prefix_if_present(const char **_str, unsigned int radix)
{
assert(radix == 16 || radix == 10 || radix == 8);
assert(_str);
const char *str = *_str;
assert(str);
if (radix == 16 && str_has_hex_prefix(str))
str += 2;
else if (radix == 8 && str_has_octal_prefix(str))
str += 1;
/* No prefix to strip for radix == 10. */
*_str = str;
}
int str_to_buf(const char *str, void *_buf, unsigned int buf_len,
unsigned int radix, unsigned int *_detected_radix)
{
assert(radix == 0 || radix == 8 || radix == 10 || radix == 16);
if (radix == 0)
radix = str_radix_guess(str);
if (_detected_radix)
*_detected_radix = radix;
str_strip_number_prefix_if_present(&str, radix);
const size_t str_len = strlen(str);
if (str_len == 0)
return ERROR_INVALID_NUMBER;
float factor = 0.0;
if (radix == 16)
factor = 0.5; /* log(16) / log(256) = 0.5 */
else if (radix == 10)
@ -243,41 +281,69 @@ int str_to_buf(const char *str, unsigned str_len,
else if (radix == 8)
factor = 0.375; /* log(8) / log(256) = 0.375 */
else
return 0;
assert(false);
/* copy to zero-terminated buffer */
char *charbuf = strndup(str, str_len);
const unsigned int b256_len = ceil_f_to_u32(str_len * factor);
/* number of digits in base-256 notation */
unsigned b256_len = ceil_f_to_u32(str_len * factor);
/* Allocate a buffer for digits in base-256 notation */
uint8_t *b256_buf = calloc(b256_len, 1);
if (!b256_buf) {
LOG_ERROR("Unable to allocate memory");
return ERROR_FAIL;
}
/* go through zero terminated buffer
* input digits (ASCII) */
unsigned i;
for (i = 0; charbuf[i]; i++) {
uint32_t tmp = charbuf[i];
if ((tmp >= '0') && (tmp <= '9'))
/* Go through the zero-terminated buffer
* of input digits (ASCII) */
for (unsigned int i = 0; str[i]; i++) {
uint32_t tmp = str[i];
if ((tmp >= '0') && (tmp <= '9')) {
tmp = (tmp - '0');
else if ((tmp >= 'a') && (tmp <= 'f'))
} else if ((tmp >= 'a') && (tmp <= 'f')) {
tmp = (tmp - 'a' + 10);
else if ((tmp >= 'A') && (tmp <= 'F'))
} else if ((tmp >= 'A') && (tmp <= 'F')) {
tmp = (tmp - 'A' + 10);
else
continue; /* skip characters other than [0-9,a-f,A-F] */
} else {
/* Characters other than [0-9,a-f,A-F] are invalid */
free(b256_buf);
return ERROR_INVALID_NUMBER;
}
if (tmp >= radix)
continue; /* skip digits invalid for the current radix */
if (tmp >= radix) {
/* Encountered a digit that is invalid for the current radix */
free(b256_buf);
return ERROR_INVALID_NUMBER;
}
/* base-256 digits */
for (unsigned j = 0; j < b256_len; j++) {
/* Add the current digit (tmp) to the intermediate result
* in b256_buf (base-256 digits) */
for (unsigned int j = 0; j < b256_len; j++) {
tmp += (uint32_t)b256_buf[j] * radix;
b256_buf[j] = (uint8_t)(tmp & 0xFF);
b256_buf[j] = (uint8_t)(tmp & 0xFFu);
tmp >>= 8;
}
/* The b256_t buffer is large enough to contain the whole result. */
assert(tmp == 0);
}
/* The result must not contain more bits than buf_len. */
/* Check the whole bytes: */
for (unsigned int j = DIV_ROUND_UP(buf_len, 8); j < b256_len; j++) {
if (b256_buf[j] != 0x0) {
free(b256_buf);
return ERROR_NUMBER_EXCEEDS_BUFFER;
}
}
/* Check the partial byte: */
if (buf_len % 8) {
const uint8_t mask = 0xFFu << (buf_len % 8);
if ((b256_buf[(buf_len / 8)] & mask) != 0x0) {
free(b256_buf);
return ERROR_NUMBER_EXCEEDS_BUFFER;
}
}
/* Copy the digits to the output buffer */
uint8_t *buf = _buf;
for (unsigned j = 0; j < DIV_ROUND_UP(buf_len, 8); j++) {
if (j < b256_len)
@ -286,14 +352,8 @@ int str_to_buf(const char *str, unsigned str_len,
buf[j] = 0;
}
/* mask out bits that don't belong to the buffer */
if (buf_len % 8)
buf[(buf_len / 8)] &= 0xff >> (8 - (buf_len % 8));
free(b256_buf);
free(charbuf);
return i;
return ERROR_OK;
}
void bit_copy_queue_init(struct bit_copy_queue *q)

17
src/helper/binarybuffer.h
View File

@ -14,6 +14,9 @@
#include <helper/list.h>
#include <helper/types.h>
#define ERROR_INVALID_NUMBER (-1700)
#define ERROR_NUMBER_EXCEEDS_BUFFER (-1701)
/** @file
* Support functions to access arbitrary bits in a byte array
*/
@ -189,8 +192,18 @@ void *buf_set_ones(void *buf, unsigned size);
void *buf_set_buf(const void *src, unsigned src_start,
void *dst, unsigned dst_start, unsigned len);
int str_to_buf(const char *str, unsigned len,
void *bin_buf, unsigned buf_size, unsigned radix);
/**
* Parse an unsigned number (provided as a zero-terminated string)
* into a bit buffer whose size is buf_len bits.
* @param str Input number, zero-terminated string
* @param _buf Output buffer, allocated by the caller
* @param buf_len Output buffer size in bits
* @param radix Base of the input number - 16, 10, 8 or 0.
* 0 means auto-detect the radix.
*/
int str_to_buf(const char *str, void *_buf, unsigned int buf_len,
unsigned int radix, unsigned int *_detected_radix);
char *buf_to_hex_str(const void *buf, unsigned size);
/* read a uint32_t from a buffer in target memory endianness */

40
src/helper/command.c
View File

@ -1360,6 +1360,46 @@ int command_parse_bool_arg(const char *in, bool *out)
return ERROR_COMMAND_SYNTAX_ERROR;
}
static const char *radix_to_str(unsigned int radix)
{
switch (radix) {
case 16: return "hexadecimal";
case 10: return "decadic";
case 8: return "octal";
}
assert(false);
return "";
}
COMMAND_HELPER(command_parse_str_to_buf, const char *str, void *buf, unsigned int buf_len,
unsigned int radix)
{
assert(str);
assert(buf);
int ret = str_to_buf(str, buf, buf_len, radix, NULL);
if (ret == ERROR_OK)
return ret;
/* Provide a clear error message to the user */
if (ret == ERROR_INVALID_NUMBER) {
if (radix == 0) {
/* Any radix is accepted, so don't include it in the error message. */
command_print(CMD, "'%s' is not a valid number", str);
} else {
/* Specific radix is required - tell the user what it is. */
command_print(CMD, "'%s' is not a valid number (requiring %s number)",
str, radix_to_str(radix));
}
} else if (ret == ERROR_NUMBER_EXCEEDS_BUFFER) {
command_print(CMD, "Number %s exceeds %u bits", str, buf_len);
} else {
command_print(CMD, "Could not parse number '%s'", str);
}
return ERROR_COMMAND_ARGUMENT_INVALID;
}
COMMAND_HELPER(handle_command_parse_bool, bool *out, const char *label)
{
switch (CMD_ARGC) {

11
src/helper/command.h
View File

@ -517,6 +517,17 @@ DECLARE_PARSE_WRAPPER(_target_addr, target_addr_t);
int command_parse_bool_arg(const char *in, bool *out);
COMMAND_HELPER(handle_command_parse_bool, bool *out, const char *label);
/**
* Parse a number (base 10, base 16 or base 8) and store the result
* into a bit buffer.
*
* In case of parsing error, a user-readable error message is produced.
*
* If radix = 0 is given, the function guesses the radix by looking at the number prefix.
*/
COMMAND_HELPER(command_parse_str_to_buf, const char *str, void *buf, unsigned int buf_len,
unsigned int radix);
/** parses an on/off command argument */
#define COMMAND_PARSE_ON_OFF(in, out) \
COMMAND_PARSE_BOOL(in, out, "on", "off")

5
src/jtag/tcl.c
View File

@ -87,8 +87,11 @@ static COMMAND_HELPER(handle_jtag_command_drscan_fields, struct scan_field *fiel
LOG_ERROR("Out of memory");
return ERROR_FAIL;
}
fields[field_count].out_value = t;
str_to_buf(CMD_ARGV[i + 1], strlen(CMD_ARGV[i + 1]), t, bits, 0);
int ret = CALL_COMMAND_HANDLER(command_parse_str_to_buf, CMD_ARGV[i + 1], t, bits, 0);
if (ret != ERROR_OK)
return ret;
fields[field_count].in_value = t;
field_count++;
}

66
src/target/target.c
View File

@ -3128,11 +3128,18 @@ COMMAND_HANDLER(handle_reg_command)
/* set register value */
if (CMD_ARGC == 2) {
uint8_t *buf = malloc(DIV_ROUND_UP(reg->size, 8));
if (!buf)
if (!buf) {
LOG_ERROR("Failed to allocate memory");
return ERROR_FAIL;
str_to_buf(CMD_ARGV[1], strlen(CMD_ARGV[1]), buf, reg->size, 0);
}
int retval = reg->type->set(reg, buf);
int retval = CALL_COMMAND_HANDLER(command_parse_str_to_buf, CMD_ARGV[1], buf, reg->size, 0);
if (retval != ERROR_OK) {
free(buf);
return retval;
}
retval = reg->type->set(reg, buf);
if (retval != ERROR_OK) {
LOG_ERROR("Could not write to register '%s'", reg->name);
} else {
@ -4788,63 +4795,64 @@ static int target_jim_get_reg(Jim_Interp *interp, int argc,
return JIM_OK;
}
static int target_jim_set_reg(Jim_Interp *interp, int argc,
Jim_Obj * const *argv)
COMMAND_HANDLER(handle_set_reg_command)
{
if (argc != 2) {
Jim_WrongNumArgs(interp, 1, argv, "dict");
return JIM_ERR;
}
if (CMD_ARGC != 1)
return ERROR_COMMAND_SYNTAX_ERROR;
int tmp;
#if JIM_VERSION >= 80
Jim_Obj **dict = Jim_DictPairs(interp, argv[1], &tmp);
Jim_Obj **dict = Jim_DictPairs(CMD_CTX->interp, CMD_JIMTCL_ARGV[0], &tmp);
if (!dict)
return JIM_ERR;
return ERROR_FAIL;
#else
Jim_Obj **dict;
int ret = Jim_DictPairs(interp, argv[1], &dict, &tmp);
int ret = Jim_DictPairs(CMD_CTX->interp, CMD_JIMTCL_ARGV[0], &dict, &tmp);
if (ret != JIM_OK)
return ret;
return ERROR_FAIL;
#endif
const unsigned int length = tmp;
struct command_context *cmd_ctx = current_command_context(interp);
assert(cmd_ctx);
const struct target *target = get_current_target(cmd_ctx);
const struct target *target = get_current_target(CMD_CTX);
assert(target);
for (unsigned int i = 0; i < length; i += 2) {
const char *reg_name = Jim_String(dict[i]);
const char *reg_value = Jim_String(dict[i + 1]);
struct reg *reg = register_get_by_name(target->reg_cache, reg_name,
false);
struct reg *reg = register_get_by_name(target->reg_cache, reg_name, false);
if (!reg || !reg->exist) {
Jim_SetResultFormatted(interp, "unknown register '%s'", reg_name);
return JIM_ERR;
command_print(CMD, "unknown register '%s'", reg_name);
return ERROR_FAIL;
}
uint8_t *buf = malloc(DIV_ROUND_UP(reg->size, 8));
if (!buf) {
LOG_ERROR("Failed to allocate memory");
return JIM_ERR;
return ERROR_FAIL;
}
str_to_buf(reg_value, strlen(reg_value), buf, reg->size, 0);
int retval = reg->type->set(reg, buf);
int retval = CALL_COMMAND_HANDLER(command_parse_str_to_buf,
reg_value, buf, reg->size, 0);
if (retval != ERROR_OK) {
free(buf);
return retval;
}
retval = reg->type->set(reg, buf);
free(buf);
if (retval != ERROR_OK) {
Jim_SetResultFormatted(interp, "failed to set '%s' to register '%s'",
command_print(CMD, "failed to set '%s' to register '%s'",
reg_value, reg_name);
return JIM_ERR;
return retval;
}
}
return JIM_OK;
return ERROR_OK;
}
/**
@ -5584,7 +5592,7 @@ static const struct command_registration target_instance_command_handlers[] = {
{
.name = "set_reg",
.mode = COMMAND_EXEC,
.jim_handler = target_jim_set_reg,
.handler = handle_set_reg_command,
.help = "Set target register values",
.usage = "dict",
},
@ -6719,7 +6727,7 @@ static const struct command_registration target_exec_command_handlers[] = {
{
.name = "set_reg",
.mode = COMMAND_EXEC,
.jim_handler = target_jim_set_reg,
.handler = handle_set_reg_command,
.help = "Set target register values",
.usage = "dict",
},