Skip to content

config: handle allocation failures in config_list - #7429

Open
cristianchiriac wants to merge 1 commit into
libgit2:mainfrom
cristianchiriac:config-list-oom
Open

cristianchiriac wants to merge 1 commit into
libgit2:mainfrom
cristianchiriac:config-list-oom

Conversation

@cristianchiriac

Copy link
Copy Markdown

Fixes #7416. Fixes #7417.

Both issues are in src/libgit2/config_list.c, and an allocation-failure sweep over opening and snapshotting a config hits both, so they're fixed together:

  • git_config_list_append: the git__calloc of a new map head wasn't checked. On failure, NULL went into the map and was then written through. The list entry is now allocated first. If the map head allocation or the headmap_put fails, both are freed and the map is left unchanged. Before, a failed list allocation also left the map pointing at an entry that the caller then freed.
  • git_config_list_dup: the result of git_config_list_dup_entry wasn't assigned to error. On failure, the partial list was freed and the function returned 0 without setting *out.

Test: config::oom::open_and_snapshot_fail_or_succeed writes a small config with a multivar and a subsection. It then opens the config, snapshots it and reads two values, with an allocator that fails the Nth allocation. It does this for every N until no allocation fails. Each run must either report an error or return the values in the file. This is the same pattern as blame::oom in #7425.

  • before: segfault (also with only one of the two fixes applied)
  • after: passes

All tests in -sconfig pass (MinGW gcc 16, Windows 11).

git_config_list_append did not check the allocation of a new map head, so
it stored NULL in the map and then wrote through it. It now allocates the
list entry first and frees both on failure, which leaves the map
unchanged. git_config_list_dup did not keep the error from duplicating an
entry, so it freed the partial list and returned 0 without setting *out.

Fixes libgit2#7416
Fixes libgit2#7417

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Lost error in git_config_list_dup on allocation failure [BUG] NULL deref in git_config_list_append on allocation failure

1 participant