← Watch feed
wire: remove transport specific output buffers
Public commit record
68/100 · Adequate
What the developer wrote
Authored by Jon Griffiths
wire: remove transport specific output buffers
These are only ever used to reject messages which can be done with a
temporary ~128 bytes of stack instead of a permanent 3-9kb of RAM.
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This commit is a small memory-optimization cleanup. It removes several large, permanently allocated output buffers used only to send short 'message rejected' error replies, replacing them with a tiny temporary buffer on the function call stack. There is no direct security bug being fixed here; it is primarily a resource-saving change. However, using stack memory instead of heap memory slightly reduces the attack surface related to heap allocation misuse, and the new code is more careful about buffer sizes.
Recommended action
Treat as a routine cleanup/refactoring commit. Reviewers may want to confirm that `jade_process_reject_message_ex` never writes more than 112 bytes into the new `out` buffer and that stack usage remains safe under the task's stack limits. No urgent security response is indicated by the diff alone.
Security signals we found
01
Reduction of persistent heap-allocated buffers lowers exposure to heap corruption or allocation-failure paths
02
New stack buffer is sized specifically for short rejection messages (112 bytes) rather than the full maximum output message size
03
No change to message parsing logic, RPC dispatch, or cryptographic operations
04
No bounds-check bugs introduced; helper uses `sizeof(out)` when calling the reject-message builder
Evidence from the diff
The patch removes per-transport output buffers (_libjade_serial_data_out, ble_data_out, qemu_tcp_data_out, serial_data_out) sized with MAX_OUTPUT_MSG_SIZE and the data_out parameter from handle_data(). Rejection replies are now built in a local uint8_t out[112] stack buffer inside a new reject_data() helper. The macro SEND_REJECT_MSG is replaced by this helper, which also hardcodes CBOR_RPC_INVALID_REQUEST and uses a 16-byte length string buffer. The change reduces RAM use by 3–9 KB and removes a heap-allocation dependency for these buffers.
Changed components
main/wire.cmain/wire.hmain/serial.cmain/ble/ble.cmain/qemu/qemu_tcp.clibjade/libjade.cInspect captured patch +22 / −39
diff --git a/libjade/libjade.c b/libjade/libjade.c
index 860e8a4..16c42e9 100644
--- a/libjade/libjade.c
+++ b/libjade/libjade.c
@@ -472,7 +472,6 @@ void libjade_stop(void)
}
static uint8_t _libjade_serial_data_in[MAX_INPUT_MSG_SIZE + 1] = { 0 };
-static uint8_t _libjade_serial_data_out[MAX_OUTPUT_MSG_SIZE] = { 0 };
static size_t _libjade_serial_read_ptr = 0;
static TickType_t _libjade_last_processing_time = 0;
@@ -486,8 +485,7 @@ bool libjade_send(const uint8_t* data, const size_t size)
data_with_source[0] = SOURCE_SERIAL;
memcpy(data_with_source + 1 + _libjade_serial_read_ptr, data, size);
const bool reject_incomplete = false;
- handle_data(data_with_source, &_libjade_serial_read_ptr, size, &_libjade_last_processing_time, reject_incomplete,
- _libjade_serial_data_out);
+ handle_data(data_with_source, &_libjade_serial_read_ptr, size, &_libjade_last_processing_time, reject_incomplete);
return true;
}
diff --git a/main/ble/ble.c b/main/ble/ble.c
index 77bc5cb..a902290 100644
--- a/main/ble/ble.c
+++ b/main/ble/ble.c
@@ -57,7 +57,6 @@ static size_t ble_read = 0;
static uint8_t own_addr_type = BLE_OWN_ADDR_RANDOM;
static uint8_t* full_ble_data_in = NULL;
static TickType_t last_processing_time = 0;
-static uint8_t* ble_data_out = NULL;
static uint16_t peer_conn_handle = 0;
static const size_t ATT_OVERHEAD = 3;
static const size_t MAX_BLE_ATTR_SIZE = 512;
@@ -100,8 +99,7 @@ static int gatt_chr_event(
if (ble_read + ble_msg_len >= MAX_INPUT_MSG_SIZE) {
const bool reject_incomplete = true; // Reject current buffer if incomplete
const size_t new_data = 0;
- handle_data(
- full_ble_data_in, &ble_read, new_data, &last_processing_time, reject_incomplete, ble_data_out);
+ handle_data(full_ble_data_in, &ble_read, new_data, &last_processing_time, reject_incomplete);
JADE_ASSERT(ble_read == 0);
}
@@ -113,8 +111,7 @@ static int gatt_chr_event(
JADE_LOGD("Passing %u+%u bytes from ble device to common handler", ble_read, ble_msg_len);
const bool reject_incomplete = false;
- handle_data(
- full_ble_data_in, &ble_read, ble_msg_len, &last_processing_time, reject_incomplete, ble_data_out);
+ handle_data(full_ble_data_in, &ble_read, ble_msg_len, &last_processing_time, reject_incomplete);
return 0;
default:
@@ -449,7 +446,6 @@ bool ble_init(TaskHandle_t* ble_handle)
{
JADE_ASSERT(ble_handle);
JADE_ASSERT(!full_ble_data_in);
- JADE_ASSERT(!ble_data_out);
// Initialise assuming preferred MTU and sanity check value
set_ble_max_write_size_for_mtu(CONFIG_BT_NIMBLE_ATT_PREFERRED_MTU);
@@ -459,7 +455,6 @@ bool ble_init(TaskHandle_t* ble_handle)
// Extra byte at the start for source-id
full_ble_data_in = (uint8_t*)JADE_MALLOC_PREFER_SPIRAM(MAX_INPUT_MSG_SIZE + 1);
full_ble_data_in[0] = SOURCE_BLE;
- ble_data_out = JADE_MALLOC_PREFER_SPIRAM(MAX_OUTPUT_MSG_SIZE);
p_ble_writer_handle = ble_handle;
ble_writer_shutdown_done = xSemaphoreCreateBinary();
diff --git a/main/qemu/qemu_tcp.c b/main/qemu/qemu_tcp.c
index 6275640..d1ab642 100644
--- a/main/qemu/qemu_tcp.c
+++ b/main/qemu/qemu_tcp.c
@@ -25,7 +25,6 @@
#include <lwip/netdb.h>
static uint8_t* full_qemu_tcp_data_in = NULL;
-static uint8_t* qemu_tcp_data_out = NULL;
static esp_eth_handle_t s_eth_handle = NULL;
static esp_eth_mac_t* s_mac = NULL;
@@ -100,7 +99,7 @@ static void qemu_tcp_reader(void* ignore)
// Pass to common handler
JADE_LOGD("Passing %u+%u bytes from tcp stream to common handler", read, len);
const bool reject_incomplete = false;
- handle_data(full_qemu_tcp_data_in, &read, len, &last_processing_time, reject_incomplete, qemu_tcp_data_out);
+ handle_data(full_qemu_tcp_data_in, &read, len, &last_processing_time, reject_incomplete);
}
}
@@ -223,7 +222,6 @@ bool qemu_tcp_init(TaskHandle_t* qemu_tcp_handle)
{
JADE_ASSERT(qemu_tcp_handle);
JADE_ASSERT(!full_qemu_tcp_data_in);
- JADE_ASSERT(!qemu_tcp_data_out);
JADE_ASSERT(!sockmutex);
sockmutex = xSemaphoreCreateMutex();
@@ -234,7 +232,6 @@ bool qemu_tcp_init(TaskHandle_t* qemu_tcp_handle)
// Extra byte at the start for source-id
full_qemu_tcp_data_in = JADE_MALLOC_PREFER_SPIRAM(MAX_INPUT_MSG_SIZE + 1);
full_qemu_tcp_data_in[0] = SOURCE_QEMU_TCP;
- qemu_tcp_data_out = JADE_MALLOC_PREFER_SPIRAM(MAX_OUTPUT_MSG_SIZE);
BaseType_t retval = xTaskCreatePinnedToCore(
&qemu_tcp_reader, "qemu_tcp_reader", 5 * 1024, NULL, JADE_TASK_PRIO_READER, NULL, JADE_CORE_SECONDARY);
diff --git a/main/serial.c b/main/serial.c
index 6f8509b..106183d 100644
--- a/main/serial.c
+++ b/main/serial.c
@@ -39,7 +39,6 @@
#endif // IDF_TARGET_ESP32S3
static uint8_t* full_serial_data_in = NULL;
-static uint8_t* serial_data_out = NULL;
static TaskHandle_t serial_reader_handle = NULL;
static TaskHandle_t* p_serial_writer_handle = NULL;
@@ -128,7 +127,7 @@ static void serial_reader(void* ignore)
JADE_LOGD("Passing %u+%u bytes from serial device to common handler", read, len);
const bool reject_incomplete = false;
- handle_data(full_serial_data_in, &read, len, &last_processing_time, reject_incomplete, serial_data_out);
+ handle_data(full_serial_data_in, &read, len, &last_processing_time, reject_incomplete);
}
serial_post_exit_event_and_await_death(&serial_reader_shutdown_done);
}
@@ -269,7 +268,6 @@ bool serial_init(TaskHandle_t* serial_handle)
{
JADE_ASSERT(serial_handle);
JADE_ASSERT(!full_serial_data_in);
- JADE_ASSERT(!serial_data_out);
JADE_ASSERT(!serial_is_enabled);
JADE_ASSERT(!serial_reader_shutdown_done);
JADE_ASSERT(!serial_writer_shutdown_done);
@@ -281,7 +279,6 @@ bool serial_init(TaskHandle_t* serial_handle)
// Extra byte at the start for source-id
full_serial_data_in = JADE_MALLOC_PREFER_SPIRAM(MAX_INPUT_MSG_SIZE + 1);
full_serial_data_in[0] = SOURCE_SERIAL;
- serial_data_out = JADE_MALLOC_PREFER_SPIRAM(MAX_OUTPUT_MSG_SIZE);
p_serial_writer_handle = serial_handle;
return serial_init_internal();
}
diff --git a/main/wire.c b/main/wire.c
index 42c1f2d..b25b5d2 100644
--- a/main/wire.c
+++ b/main/wire.c
@@ -29,14 +29,14 @@ static const TickType_t TIMEOUT_TICKS = 3000 / portTICK_PERIOD_MS;
static const TickType_t TIMEOUT_TICKS = 2000 / portTICK_PERIOD_MS;
#endif
-// Macros for use in handle_data() as always called with fixed params
-#define SEND_REJECT_MSG(code, msg, rejectedlen) \
- do { \
- char lenstr[8]; \
- const int ret = snprintf(lenstr, sizeof(lenstr), "%u", rejectedlen); \
- JADE_ASSERT(ret > 0 && ret < sizeof(lenstr)); \
- jade_process_reject_message_ex(ctx, code, msg, (uint8_t*)lenstr, ret, data_out, MAX_OUTPUT_MSG_SIZE); \
- } while (false)
+static void reject_data(const cbor_msg_t ctx, const char* msg, size_t rejected_len)
+{
+ uint8_t len_str[16], out[112]; // sufficient
+ JADE_LOGW("%s, length %u", msg, rejected_len);
+ const int ret = snprintf((char*)len_str, sizeof(len_str), "%u", rejected_len);
+ JADE_ASSERT(ret > 0 && ret < sizeof(len_str));
+ jade_process_reject_message_ex(ctx, CBOR_RPC_INVALID_REQUEST, msg, len_str, ret, out, sizeof(out));
+}
// Some messages we handle immediately in this task
static const char PING[] = { 'p', 'i', 'n', 'g' };
@@ -76,13 +76,12 @@ static bool handleImmediateMessage(cbor_msg_t* ctx)
}
// Handle bytes in receive buffer
-// NOTE: assumes sizes of input and output buffers - could be passed sizes if preferred
-static void handle_data_impl(uint8_t* full_data_in, size_t* read_ptr, bool reject_incomplete, uint8_t* data_out)
+// NOTE: assumes sizes of input buffer - could be passed sizes if preferred
+static void handle_data_impl(uint8_t* full_data_in, size_t* read_ptr, bool reject_incomplete)
{
JADE_ASSERT(full_data_in);
JADE_ASSERT(read_ptr);
JADE_ASSERT(*read_ptr <= MAX_INPUT_MSG_SIZE);
- JADE_ASSERT(data_out);
const jade_msg_source_t source = full_data_in[0];
uint8_t* const data_in = full_data_in + 1;
@@ -113,15 +112,13 @@ static void handle_data_impl(uint8_t* full_data_in, size_t* read_ptr, bool rejec
// Not a complete/valid cbor message, and we are not allowed to await more, so reject what we have.
// Break to reset the read-ptr to the start and lose all the data.
- JADE_LOGW("Got incomplete CBOR message, length %u but not awaiting more data - rejecting", read);
- SEND_REJECT_MSG(CBOR_RPC_INVALID_REQUEST, "Invalid RPC Request message", read);
+ reject_data(ctx, "Invalid RPC Request message", read);
break;
}
if (!rpc_request_valid(&ctx.value)) {
// bad message - expect all inputs to be cbor with a root map with an id and a method strings keys values
- JADE_LOGW("Invalid request, length %u", msg_len);
- SEND_REJECT_MSG(CBOR_RPC_INVALID_REQUEST, "Invalid RPC Request message (malformed)", msg_len);
+ reject_data(ctx, "Invalid RPC Request message (malformed)", msg_len);
} else if (handleImmediateMessage(&ctx)) {
JADE_LOGI("Message handled, not passing to main task");
idletimer_register_activity(false);
@@ -132,7 +129,7 @@ static void handle_data_impl(uint8_t* full_data_in, size_t* read_ptr, bool rejec
// (but not as 'UI' activity - ie. keep jade on but do not stop the screen from turning off)
idletimer_register_activity(false);
} else {
- SEND_REJECT_MSG(CBOR_RPC_INVALID_REQUEST, "Input message too large to handle", msg_len);
+ reject_data(ctx, "Input message too large", msg_len);
}
}
@@ -154,15 +151,14 @@ static void handle_data_impl(uint8_t* full_data_in, size_t* read_ptr, bool rejec
}
// Handle new bytes received
-// NOTE: assumes sizes of input and output buffers - could be passed sizes if preferred
+// NOTE: assumes sizes of input buffer - could be passed sizes if preferred
void handle_data(uint8_t* full_data_in, size_t* read_ptr, const size_t new_data_len, TickType_t* last_processing_time,
- bool reject_incomplete, uint8_t* data_out)
+ bool reject_incomplete)
{
JADE_ASSERT(full_data_in);
JADE_ASSERT(read_ptr);
JADE_ASSERT(*read_ptr + new_data_len <= MAX_INPUT_MSG_SIZE);
JADE_ASSERT(last_processing_time);
- JADE_ASSERT(data_out);
// Get current message processing time
const TickType_t time_now = xTaskGetTickCount();
@@ -182,7 +178,7 @@ void handle_data(uint8_t* full_data_in, size_t* read_ptr, const size_t new_data_
*read_ptr += new_data_len;
JADE_LOGD("Passing %u bytes to common handler", *read_ptr);
reject_incomplete |= (*read_ptr == MAX_INPUT_MSG_SIZE);
- handle_data_impl(full_data_in, read_ptr, reject_incomplete, data_out);
+ handle_data_impl(full_data_in, read_ptr, reject_incomplete);
// Update caller's 'last processing time'
*last_processing_time = time_now;
diff --git a/main/wire.h b/main/wire.h
index af5e58a..2fac18f 100644
--- a/main/wire.h
+++ b/main/wire.h
@@ -6,6 +6,6 @@
#include <stdint.h>
void handle_data(uint8_t* full_data_in, size_t* read_ptr, size_t new_data_len, TickType_t* last_processing_time,
- bool reject_incomplete, uint8_t* data_out);
+ bool reject_incomplete);
#endif /* WIRE_H_ */
Risk score
Our methodology →Why this scored 18/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.