Skip to content

Commit 93449d3

Browse files
committed
menu/materialui: fix leak + null-deref + needless strdup in update_savestate_thumbnail_path
The function had three problems stacked in a handful of lines: 1. Null-deref: mui->savestate_thumbnail_file_path was dereferenced (via strdup) BEFORE the 'if (!mui) return;' guard. If data was ever NULL, the guard would never be reached. 2. Leak: the strdup result was assigned to a const char * and never freed on any exit path - not when savestate_thumbnail was disabled, not when the entry label check failed, not on the success path either. Called frequently (on every selection change in the savestate list) so the leak was quickly user- visible. 3. Needless strdup: savestate_thumbnail_file_path is a fixed-size char[PATH_MAX_LENGTH] embedded in the struct, not a pointer. Heap allocation is unnecessary; a stack buffer of the same size does the job and sidesteps both the leak and the OOM failure mode (where strdup returning NULL silently flipped the string_is_equal comparison below). Fix: declare a local 'old_path[PATH_MAX_LENGTH]' AFTER the NULL guard, strlcpy the current path into it before the field is cleared, and compare against it further down. No heap allocation, no leak, no dereference before NULL check. Thread-safety: unchanged. Menu driver callbacks run on the main thread; materialui_handle_t state is not shared across threads. The only writes touch mui->savestate_thumbnail_file_path and mui->thumbnails.savestate, both already single-thread owned.
1 parent 91eba66 commit 93449d3

1 file changed

Lines changed: 13 additions & 3 deletions

File tree

menu/drivers/materialui.c

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2484,12 +2484,22 @@ static void materialui_update_savestate_thumbnail_path(void *data, unsigned i)
24842484
{
24852485
settings_t *settings = config_get_ptr();
24862486
materialui_handle_t *mui = (materialui_handle_t*)data;
2487-
bool savestate_thumbnail = settings->bools.savestate_thumbnail_enable;
2488-
const char *current_path = strdup(mui->savestate_thumbnail_file_path);
2487+
bool savestate_thumbnail;
2488+
char old_path[PATH_MAX_LENGTH];
24892489

24902490
if (!mui)
24912491
return;
24922492

2493+
savestate_thumbnail = settings->bools.savestate_thumbnail_enable;
2494+
2495+
/* Snapshot the current path before clearing it, so we can detect
2496+
* whether the resolved path actually changed and only then reset
2497+
* the thumbnail. A stack buffer is sufficient (the field it
2498+
* mirrors is itself a fixed-size char array) and avoids the leak
2499+
* + potential NULL-deref of the previous strdup-before-NULL-check
2500+
* construction. */
2501+
strlcpy(old_path, mui->savestate_thumbnail_file_path, sizeof(old_path));
2502+
24932503
mui->savestate_thumbnail_file_path[0] = '\0';
24942504

24952505
if (savestate_thumbnail)
@@ -2525,7 +2535,7 @@ static void materialui_update_savestate_thumbnail_path(void *data, unsigned i)
25252535
strlcpy(mui->savestate_thumbnail_file_path, path,
25262536
sizeof(mui->savestate_thumbnail_file_path));
25272537

2528-
if (!string_is_equal(current_path, mui->savestate_thumbnail_file_path))
2538+
if (!string_is_equal(old_path, mui->savestate_thumbnail_file_path))
25292539
gfx_thumbnail_reset(&mui->thumbnails.savestate);
25302540

25312541
materialui_update_fullscreen_thumbnail_label(mui);

0 commit comments

Comments
 (0)