Skip to content

Commit 0563af5

Browse files
committed
tasks/task_save: NULL-check temp_data malloc in content_undo_load_state
/* We need to make a temporary copy of the buffer, to allow the swap below */ temp_data = malloc(undo_load_buf.size); temp_data_size = undo_load_buf.size; memcpy(temp_data, undo_load_buf.data, undo_load_buf.size); Unchecked malloc; the memcpy on the next line NULL-derefs on OOM. This is the undo-load-state path triggered from the menu ('Undo Load State' action) - takes the previous savestate that content_load_state overwrote and swaps it back in. Fix: NULL-check temp_data. On OOM tear down the 'blocks' SRAM- backup array that was built earlier in this same function (lines 190-212 pre-patch - it mirrors current SRAM into a scratch buffer across the deserialize). Mirrors the normal-return cleanup path at the bottom of the function. Not using goto-cleanup because the existing function structure doesn't have a single exit point and retrofitting one would churn unrelated code. The inline cleanup at the OOM exit reproduces the same two-loop teardown verbatim. === Not a bug === The pre-existing 'blocks[i].data = malloc(blocks[i].size)' at line 212 and the matching one in content_load_state at line 1087 are both unchecked, but they don't crash: every downstream use of blocks[i].data is gated on 'if (blocks[i].data)' (lines 217 and 1092 in the respective functions). On OOM the corresponding SRAM block just isn't backed up / restored, which is a graceful degrade. === Thread-safety === content_undo_load_state runs on the main/runloop thread. No shared-state mutations; no lock discipline changes. === Reachability === User-triggered via menu 'Undo Load State' or the configured hotkey. undo_load_buf.size is the size of the previous save- state, typically tens of KB to a few MB depending on core. Realistic OOM site on memory-tight embedded/handheld targets.
1 parent b36e9f4 commit 0563af5

1 file changed

Lines changed: 16 additions & 0 deletions

File tree

tasks/task_save.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,22 @@ bool content_undo_load_state(void)
230230

231231
/* We need to make a temporary copy of the buffer, to allow the swap below */
232232
temp_data = malloc(undo_load_buf.size);
233+
/* NULL-check the malloc before the memcpy on the next line
234+
* NULL-derefs. On OOM we also need to tear down the 'blocks'
235+
* array built above to match the normal-return cleanup path
236+
* at the end of this function. Not using goto-cleanup because
237+
* the existing function structure doesn't have a single exit
238+
* point and retrofitting one would churn unrelated code. */
239+
if (!temp_data)
240+
{
241+
for (i = 0; i < num_blocks; i++)
242+
{
243+
free(blocks[i].data);
244+
blocks[i].data = NULL;
245+
}
246+
free(blocks);
247+
return false;
248+
}
233249
temp_data_size = undo_load_buf.size;
234250
memcpy(temp_data, undo_load_buf.data, undo_load_buf.size);
235251

0 commit comments

Comments
 (0)