From 40d4c554adf656dc2d61fbe6cb408be3e005648d Mon Sep 17 00:00:00 2001 From: David Banks Date: Fri, 15 Nov 2019 11:38:55 +0000 Subject: [PATCH] Firmware: Improve missing parameter checking on commands (#7) Change-Id: I2581bda0136386103973059545d963196d973db7 --- firmware/AtomBusMon.c | 104 ++++++++++++++++++++++++++++++++---------- firmware/AtomBusMon.h | 1 + firmware/status.c | 99 +++++++++++++++++++++++++--------------- firmware/status.h | 2 + 4 files changed, 144 insertions(+), 62 deletions(-) diff --git a/firmware/AtomBusMon.c b/firmware/AtomBusMon.c index 987083b..e1d4310 100644 --- a/firmware/AtomBusMon.c +++ b/firmware/AtomBusMon.c @@ -141,7 +141,7 @@ void (*cmdFuncs[])(char *params) = { doCmdTrigger }; -#ifdef EXTENDED_HELP +#if defined(EXTENDED_HELP) static const char ARGS00[] PROGMEM = "
"; static const char ARGS01[] PROGMEM = "[
]"; @@ -596,6 +596,9 @@ long trace; // Bit 1 indicates memory access timeout errors uint8_t error_flag = 0; +// Index of the current command +uint8_t cmd_id = 0xff; + #define MASK_CLOCK_ERROR 1 #define MASK_TIMEOUT_ERROR 2 @@ -721,6 +724,20 @@ void logIllegalCommand(char *c) { logc('\n'); } +uint8_t checkargs(char *params) { + if (!params) { +#if defined(EXTENDED_HELP) + logstr("usage:\n"); + helpForCommand(cmd_id); +#else + logstr("syntax error\n"); +#endif + return 1; + } else { + return 0; + } +} + /******************************************************** * Low-level hardware commands ********************************************************/ @@ -890,8 +907,11 @@ void genericDump(char *params, data_t (*readFunc)()) { void genericWrite(char *params, void (*writeFunc)()) { data_t data; long count = 1; - params = parsehex4(params, &memAddr); - params = parsehex2(params, &data); + params = parsehex4required(params, &memAddr); + params = parsehex2required(params, &data); + if (checkargs(params)) { + return; + } params = parselong(params, &count); logstr("Wr: "); log_addr_data(memAddr, data); @@ -1075,6 +1095,9 @@ bknum_t lookupBreakpointN(addr_t n) { bknum_t lookupBreakpoint(char *params) { addr_t addr = 0xFFFF; params = parsehex4(params, &addr); + if (checkargs(params)) { + return -1; + } bknum_t n = lookupBreakpointN(addr); if (n < 0) { logstr("Breakpoint/watch not set at "); @@ -1162,7 +1185,10 @@ void genericBreakpoint(char *params, unsigned int mode) { addr_t addr; addr_t mask = 0xFFFF; trigger_t trigger = TRIGGER_UNDEFINED; - params = parsehex4(params, &addr); + params = parsehex4required(params, &addr); + if (checkargs(params)) { + return; + } params = parsehex4(params, &mask); params = parsehex2(params, &trigger); // First, see if a breakpoint with this address already exists @@ -1325,16 +1351,16 @@ void resetCpu() { * User Commands *******************************************/ -#ifdef EXTENDED_HELP +#if defined(EXTENDED_HELP) void helpForCommand(uint8_t i) { uint8_t args = pgm_read_byte(helpMeta + i + i + 1); uint8_t tmp; const char* ip = (PGM_P) pgm_read_word(argsStrings + args); - logstr(" "); + logstr(" "); logs(cmdStrings[i]); tmp = strlen(cmdStrings[i]); - while (tmp++ < 10) { + while (tmp++ < 9) { logc(' '); } while ((tmp = pgm_read_byte(ip++))) { @@ -1452,9 +1478,12 @@ void doCmdFill(char *params) { addr_t start; addr_t end; data_t data; - params = parsehex4(params, &start); - params = parsehex4(params, &end); - params = parsehex2(params, &data); + params = parsehex4required(params, &start); + params = parsehex4required(params, &end); + params = parsehex2required(params, &data); + if (checkargs(params)) { + return; + } logstr("Wr: "); loghex4(start); logstr(" to "); @@ -1473,7 +1502,10 @@ void doCmdFill(char *params) { void doCmdGo(char *params) { addr_t addr; - params = parsehex4(params, &addr); + params = parsehex4required(params, &addr); + if (checkargs(params)) { + return; + } loadData(0x4C); loadAddr(addr); hwCmd(CMD_EXEC_GO, 0); @@ -1484,7 +1516,10 @@ void doCmdExec(char *params) { data_t op1 = 0; data_t op2 = 0; data_t op3 = 0; - params = parsehex2(params, &op1); + params = parsehex2required(params, &op1); + if (checkargs(params)) { + return; + } params = parsehex2(params, &op2); params = parsehex2(params, &op3); // Read the current PC value @@ -1510,8 +1545,11 @@ void doCmdCrc(char *params) { addr_t end; data_t data; uint32_t crc = 0; - params = parsehex4(params, &start); - params = parsehex4(params, &end); + params = parsehex4required(params, &start); + params = parsehex4required(params, &end); + if (checkargs(params)) { + return; + } loadAddr(start); for (i = start; i <= end; i++) { data = readMemByteInc(); @@ -1534,9 +1572,12 @@ void doCmdCopy(char *params) { addr_t end; addr_t to; data_t data; - params = parsehex4(params, &start); - params = parsehex4(params, &end); - params = parsehex4(params, &to); + params = parsehex4required(params, &start); + params = parsehex4required(params, &end); + params = parsehex4required(params, &to); + if (checkargs(params)) { + return; + } for (i = 0; i <= end - start; i++) { loadAddr(start + i); data = readMemByte(); @@ -1553,9 +1594,12 @@ void doCmdCompare(char *params) { addr_t with; data_t data1; data_t data2; - params = parsehex4(params, &start); - params = parsehex4(params, &end); - params = parsehex4(params, &with); + params = parsehex4required(params, &start); + params = parsehex4required(params, &end); + params = parsehex4required(params, &with); + if (checkargs(params)) { + return; + } for (i = 0; i <= end - start; i++) { loadAddr(start + i); data1 = readMemByte(); @@ -1604,8 +1648,11 @@ void doCmdSave(char *params) { addr_t start; addr_t end; data_t data; - params = parsehex4(params, &start); - params = parsehex4(params, &end); + params = parsehex4required(params, &start); + params = parsehex4required(params, &end); + if (checkargs(params)) { + return; + } logstr("Press any key to start transmission (and again at end)\n"); Serial_RxByte0(); loadAddr(start); @@ -1622,7 +1669,10 @@ void doCmdLoad(char *params) { data_t data; uint16_t timeout; - params = parsehex4(params, &start); + params = parsehex4required(params, &start); + if (checkargs(params)) { + return; + } addr = start; log_send_file(); do { @@ -1653,8 +1703,11 @@ void doCmdTest(char *params) { addr_t end; long data =-100; int8_t i; - params = parsehex4(params, &start); - params = parsehex4(params, &end); + params = parsehex4required(params, &start); + params = parsehex4required(params, &end); + if (checkargs(params)) { + return; + } params = parselong(params, &data); if (data == -100) { test(start, end, 0x55); @@ -1978,6 +2031,7 @@ void dispatchCmd(char *cmd) { } else #endif if (i < NUM_CMDS) { + cmd_id = i; (*cmdFuncs[i])(cmd); } else { logIllegalCommand(cmd); diff --git a/firmware/AtomBusMon.h b/firmware/AtomBusMon.h index 2775cdf..2e64247 100644 --- a/firmware/AtomBusMon.h +++ b/firmware/AtomBusMon.h @@ -60,6 +60,7 @@ void doCmdGo(char *params); void doCmdHelp(char *params); #if defined(COMMAND_HISTORY) void doCmdHistory(char *params); +void helpForCommand(uint8_t i); #endif void doCmdIO(char *params); void doCmdList(char *params); diff --git a/firmware/status.c b/firmware/status.c index fa91b83..85e55f1 100644 --- a/firmware/status.c +++ b/firmware/status.c @@ -229,53 +229,78 @@ char *parselong(char *params, long *val) { long ret = 0; int8_t sign = 1; // Skip any spaces - while (*params == ' ') { - params++; - } - // Note sign - if (*params == '-') { - sign = -1; - params++; - } - do { - int8_t c = convhex(*params); - if (c < 0) { - break; + if (params) { + while (*params == ' ') { + params++; } - ret *= 10; - ret += c; - if (val) { - *val = sign * ret; + // Note sign + if (*params == '-') { + sign = -1; + params++; } - params++; - } while (1); + do { + int8_t c = convhex(*params); + if (c < 0) { + break; + } + ret *= 10; + ret += c; + if (val) { + *val = sign * ret; + } + params++; + } while (1); + } + return params; +} + +static char *parsehex4common(char *params, uint16_t *val, uint8_t required) { + uint16_t ret = 0; + if (params) { + // Skip any spaces + while (*params == ' ') { + params++; + } + char *tmp = params; + do { + int8_t c = convhex(*params); + if (c < 0) { + break; + } + ret <<= 4; + ret += c; + if (val) { + *val = ret; + } + params++; + } while (1); + if (required && params == tmp) { + return 0; + } + } return params; } char *parsehex4(char *params, uint16_t *val) { - uint16_t ret = 0; - // Skip any spaces - while (*params == ' ') { - params++; - } - do { - int8_t c = convhex(*params); - if (c < 0) { - break; - } - ret <<= 4; - ret += c; - if (val) { - *val = ret; - } - params++; - } while (1); - return params; + return parsehex4common(params, val, 0); +} + +char *parsehex4required(char *params, uint16_t *val) { + return parsehex4common(params, val, 1); } char *parsehex2(char *params, uint8_t *val) { uint16_t tmp = 0xffff; - params = parsehex4(params, &tmp); + params = parsehex4common(params, &tmp, 0); + if (tmp != 0xffff) { + *val = (tmp & 0xff); + } + return params; +} + +char *parsehex2required(char *params, uint8_t *val) { + uint16_t tmp = 0xffff; + params = parsehex4common(params, &tmp, 1); if (tmp != 0xffff) { *val = (tmp & 0xff); } diff --git a/firmware/status.h b/firmware/status.h index b758773..7530796 100644 --- a/firmware/status.h +++ b/firmware/status.h @@ -45,6 +45,8 @@ char *strlong(char *buffer, long i); char *strinsert(char *buffer, const char *s); char *parselong(char *params, long *val); +char *parsehex2required(char *params, uint8_t *val); +char *parsehex4required(char *params, uint16_t *val); char *parsehex2(char *params, uint8_t *val); char *parsehex4(char *params, uint16_t *val);