Stop arrayprinter from sizing its stack buffer from the input
printHex and printDec built a variable length array of (size + 1) * 7 + 1 and size * 4 + 1 + lines bytes respectively, on the stack, from the caller's buffer size. Nothing bounded that against the stack it ran on. This reset an iOBC on the flatsat. A CFDP uplink driven without inter packet spacing filled the USLP receive buffer with about 12 KB in one 300 ms cycle, a frame parse error asked for the serial stream to be dumped, and printHex tried to place an 84 KB array on an 8 KB task stack. FreeRTOS caught it as STACK OVERFLOW DETECTED in USLP_RX and restarted the OBC. The trigger needs a parse error, so it hid for as long as the link stayed clean: USLP_RX sat at 800 bytes of its 8 KB, and the first corrupted frame with a full receive buffer behind it was fatal. Emit the output in fixed 128 byte chunks instead, flushing as it is built. The rendered text is unchanged - verified byte for byte against the previous implementation for sizes 0 to 4096 and several line widths, including the line break boundaries. printBin was already safe. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4eBFnanCACYWdKCzhHMcC
This commit is contained in:
co-authored by
Claude Opus 5
parent
e256fa4b92
commit
844faf850f
@@ -56,26 +56,33 @@ void arrayprinter::printHex(const uint8_t *data, size_t size, size_t maxCharPerL
|
||||
std::cout << std::dec << std::setfill(' ');
|
||||
std::cout << "]" << std::endl;
|
||||
#else
|
||||
// General format: 0x01, 0x02, 0x03 so it is number of chars times 6
|
||||
// plus line break plus small safety margin.
|
||||
char printBuffer[(size + 1) * 7 + 1] = {};
|
||||
#if FSFW_DISABLE_PRINTOUT == 0
|
||||
// Emitted in fixed size chunks. This used to size one buffer from the input - a variable length
|
||||
// array of (size + 1) * 7 + 1 bytes on the stack - which is unusable for the sizes this is
|
||||
// actually called with: dumping a 12 KB receive buffer asks for 84 KB of stack, and overflowed
|
||||
// an 8 KB task on the iOBC as soon as a frame parse error made it dump one. The output is
|
||||
// unchanged, it is just flushed as it is built.
|
||||
constexpr size_t CHUNK_LEN = 128;
|
||||
// An entry appends at most two hex digits, a separator and a line break.
|
||||
constexpr size_t MAX_ENTRY_LEN = 4;
|
||||
char printBuffer[CHUNK_LEN] = {};
|
||||
size_t currentPos = 0;
|
||||
printf("hex [");
|
||||
for (size_t i = 0; i < size; i++) {
|
||||
// To avoid buffer overflows.
|
||||
if (sizeof(printBuffer) - currentPos <= 7) {
|
||||
break;
|
||||
if (currentPos + MAX_ENTRY_LEN >= CHUNK_LEN) {
|
||||
printf("%s", printBuffer);
|
||||
printBuffer[0] = '\0';
|
||||
currentPos = 0;
|
||||
}
|
||||
|
||||
currentPos += snprintf(printBuffer + currentPos, 6, "%02x", data[i]);
|
||||
currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, "%02x", data[i]);
|
||||
if (i < size - 1) {
|
||||
currentPos += sprintf(printBuffer + currentPos, ",");
|
||||
currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, ",");
|
||||
if ((i + 1) % maxCharPerLine == 0) {
|
||||
currentPos += sprintf(printBuffer + currentPos, "\n");
|
||||
currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, "\n");
|
||||
}
|
||||
}
|
||||
}
|
||||
#if FSFW_DISABLE_PRINTOUT == 0
|
||||
printf("hex [%s]\n", printBuffer);
|
||||
printf("%s]\n", printBuffer);
|
||||
#endif /* FSFW_DISABLE_PRINTOUT == 0 */
|
||||
#endif
|
||||
}
|
||||
@@ -98,27 +105,30 @@ void arrayprinter::printDec(const uint8_t *data, size_t size, size_t maxCharPerL
|
||||
}
|
||||
std::cout << "]" << std::endl;
|
||||
#else
|
||||
// General format: 32,243,-12 so it is number of chars times 4
|
||||
// plus line break plus small safety margin.
|
||||
uint16_t expectedLines = ceil((double)size / maxCharPerLine);
|
||||
char printBuffer[size * 4 + 1 + expectedLines] = {};
|
||||
#if FSFW_DISABLE_PRINTOUT == 0
|
||||
// Chunked for the same reason as printHex above: the buffer used to be a variable length array
|
||||
// sized from the input.
|
||||
constexpr size_t CHUNK_LEN = 128;
|
||||
// An entry appends at most three digits, a separator and a line break.
|
||||
constexpr size_t MAX_ENTRY_LEN = 5;
|
||||
char printBuffer[CHUNK_LEN] = {};
|
||||
size_t currentPos = 0;
|
||||
printf("dec [");
|
||||
for (size_t i = 0; i < size; i++) {
|
||||
// To avoid buffer overflows.
|
||||
if (sizeof(printBuffer) - currentPos <= 4) {
|
||||
break;
|
||||
if (currentPos + MAX_ENTRY_LEN >= CHUNK_LEN) {
|
||||
printf("%s", printBuffer);
|
||||
printBuffer[0] = '\0';
|
||||
currentPos = 0;
|
||||
}
|
||||
|
||||
currentPos += snprintf(printBuffer + currentPos, 4, "%d", data[i]);
|
||||
currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, "%d", data[i]);
|
||||
if (i < size - 1) {
|
||||
currentPos += sprintf(printBuffer + currentPos, ",");
|
||||
currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, ",");
|
||||
if ((i + 1) % maxCharPerLine == 0) {
|
||||
currentPos += sprintf(printBuffer + currentPos, "\n");
|
||||
currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, "\n");
|
||||
}
|
||||
}
|
||||
}
|
||||
#if FSFW_DISABLE_PRINTOUT == 0
|
||||
printf("dec [%s]\n", printBuffer);
|
||||
printf("%s]\n", printBuffer);
|
||||
#endif /* FSFW_DISABLE_PRINTOUT == 0 */
|
||||
#endif
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user