Skip to content

Commit ed41ed5

Browse files
committed
frontend/unix: NULL-check every alloc in inotify path watcher setup
frontend_unix_watch_path_for_changes (the HAS_INOTIFY setup branch) called four allocators back-to-back with no NULL checks: inotify_data = (inotify_data_t*)calloc(1, sizeof(*inotify_data)); inotify_data->fd = fd; inotify_data->wd_list = int_vector_list_new(); inotify_data->path_list = string_list_new(); ... *change_data = (path_change_data_t*)calloc(1, sizeof(path_change_data_t)); (*change_data)->data = inotify_data; Two concrete failure modes: 1. NULL-deref: if calloc(1, sizeof(inotify_data_t)) returned NULL, the 'inotify_data->fd = fd' on the very next line segfaulted. Same problem with the trailing '(*change_data)->data = ...' if the second calloc failed. 2. Silent partial-success leaks: if inotify_data's calloc succeeded but int_vector_list_new or string_list_new returned NULL, the subsequent for-loop still called int_vector_list_append and string_list_append on a NULL list; the latter dereferences list->size (libretro-common/lists/string_list.c:145) for a second NULL-deref. If, conversely, all three list allocations succeeded but the second calloc (path_change_data_t) failed, the function returned with inotify_data + its two sub-lists + the already-established inotify_add_watch kernel watches all orphaned (inotify_data is not reachable from anywhere since *change_data is still NULL). Fix: NULL-check each allocation in turn. On any failure, unwind what was built up - closing the inotify fd automatically releases all its inotify_add_watch entries in the kernel (inotify(7): 'all associated watches are automatically freed' on fd close), so no manual inotify_rm_watch loop is needed on the final failure path. Leave *change_data untouched on every failure path, which is what callers already expect as 'no watcher is set up' (the tear-down branch at the top of this same function uses 'change_data && *change_data' as its presence check). Reachability: this runs when the user enables playlist auto-scan, history tracking, or one of the other 'watch this folder for changes' features. Not a hot path. But the allocations here are for small structs (tens of bytes each), so OOM on them indicates genuine memory pressure where segfaulting rather than degrading gracefully is the wrong outcome. Thread-safety: unchanged. Watcher setup/teardown runs on the main thread; inotify_data and the change_data indirection belong to the caller's state.
1 parent 747a8ab commit ed41ed5

1 file changed

Lines changed: 42 additions & 4 deletions

File tree

frontend/drivers/platform_unix.c

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2961,10 +2961,37 @@ static void frontend_unix_watch_path_for_changes(struct string_list *list,
29612961
return;
29622962
}
29632963

2964-
inotify_data = (inotify_data_t*)calloc(1, sizeof(*inotify_data));
2964+
/* NULL-check every allocation step. Previously this function
2965+
* called calloc + int_vector_list_new + string_list_new + the
2966+
* second calloc with no guards, then dereferenced each result
2967+
* immediately - an OOM on any of them segfaulted, and a partial
2968+
* success (e.g. inotify_data allocated but wd_list alloc failed)
2969+
* leaked the earlier allocations together with the already-
2970+
* established inotify_add_watch kernel watches. On any failure
2971+
* below, unwind what we did manage to set up and return with
2972+
* *change_data left untouched (callers treat NULL *change_data
2973+
* as 'no watcher is set up'). */
2974+
if (!(inotify_data = (inotify_data_t*)calloc(1, sizeof(*inotify_data))))
2975+
{
2976+
close(fd);
2977+
return;
2978+
}
29652979
inotify_data->fd = fd;
2966-
inotify_data->wd_list = int_vector_list_new();
2967-
inotify_data->path_list = string_list_new();
2980+
2981+
if (!(inotify_data->wd_list = int_vector_list_new()))
2982+
{
2983+
free(inotify_data);
2984+
close(fd);
2985+
return;
2986+
}
2987+
2988+
if (!(inotify_data->path_list = string_list_new()))
2989+
{
2990+
int_vector_list_free(inotify_data->wd_list);
2991+
free(inotify_data);
2992+
close(fd);
2993+
return;
2994+
}
29682995

29692996
if (flags & PATH_CHANGE_TYPE_MODIFIED)
29702997
inotify_mask |= IN_MODIFY;
@@ -2992,7 +3019,18 @@ static void frontend_unix_watch_path_for_changes(struct string_list *list,
29923019
string_list_append(inotify_data->path_list, list->elems[i].data, attr);
29933020
}
29943021

2995-
*change_data = (path_change_data_t*)calloc(1, sizeof(path_change_data_t));
3022+
if (!(*change_data = (path_change_data_t*)calloc(1, sizeof(path_change_data_t))))
3023+
{
3024+
/* Rip down everything we built up: the fd, the kernel watches
3025+
* it owns, both lists, and inotify_data itself. Closing fd
3026+
* auto-removes all its inotify_add_watch entries, so we do not
3027+
* need to iterate wd_list and inotify_rm_watch by hand. */
3028+
string_list_free(inotify_data->path_list);
3029+
int_vector_list_free(inotify_data->wd_list);
3030+
close(inotify_data->fd);
3031+
free(inotify_data);
3032+
return;
3033+
}
29963034
(*change_data)->data = inotify_data;
29973035
#endif
29983036
}

0 commit comments

Comments
 (0)