Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remote: fix memory leak in git_remote_download() #6651

Merged
merged 1 commit into from Nov 20, 2023

Conversation

7Ji
Copy link
Contributor

@7Ji 7Ji commented Oct 31, 2023

connect_opts is created with its custom_headers and proxy_opts->url possibly allocated on heap, just like in git_remote_fetch(). But unlike in _fetch(), it is not disposed at the end of the function, thus causing memory leak.


A backtrace demostrating the issue (from the demo below):

=================================================================
==24905==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 22 byte(s) in 1 object(s) allocated from:
    #0 0x7f4cb4ce1359 in __interceptor_malloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7f4cb52e942e in stdalloc__malloc /tmp/libgit2/src/util/allocators/stdalloc.c:20
    #2 0x7f4cb52e9040 in git__malloc /tmp/libgit2/src/util/alloc.h:19
    #3 0x7f4cb52e9229 in git__strdup /tmp/libgit2/src/util/alloc.c:54
    #4 0x7f4cb53a4605 in git_proxy_options_dup /tmp/libgit2/src/libgit2/proxy.c:35
    #5 0x7f4cb53ba916 in git_remote_connect_options_dup /tmp/libgit2/src/libgit2/remote.c:777
    #6 0x7f4cb53badc8 in git_remote_connect_options_normalize /tmp/libgit2/src/libgit2/remote.c:917
    #7 0x7f4cb53b8426 in git_remote_connect_options__from_fetch_opts /tmp/libgit2/src/libgit2/remote.h:79
    #8 0x7f4cb53bc038 in git_remote_download /tmp/libgit2/src/libgit2/remote.c:1335
    #9 0x55a04d5bc99d in main (/tmp/demo+0x199d) (BuildId: 44836a470bc5c2b8ad0414dcda4639c66e0af9f2)
    #10 0x7f4cb4a45ccf  (/usr/lib/libc.so.6+0x27ccf) (BuildId: 8bfe03f6bf9b6a6e2591babd0bbc266837d8f658)

SUMMARY: AddressSanitizer: 22 byte(s) leaked in 1 allocation(s).

The demo:

#include <stdio.h>
#include <stdlib.h>
#include <git2.h>

int main() {
    git_repository *repo;
    git_remote *remote;
    git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT;
    git_fetch_options fetch_opts = GIT_FETCH_OPTIONS_INIT;
    char const *refspecs_strings[] = {"+refs/*:refs/*", NULL};
    git_strarray const refspecs = { .count = 1, .strings = (char **)refspecs_strings };
    int r;

    fetch_opts.proxy_opts.url = "http://xray.lan:11092";
    fetch_opts.proxy_opts.type = GIT_PROXY_SPECIFIED;

    git_libgit2_init();
    
    if (git_repository_open_bare(&repo, "/tmp/demo.git")) {
        printf("Failed to open repo\n");
        return -1;
    }
    if (git_remote_create_anonymous(&remote, repo, "https://github.com/7Ji/buildos.git")) {
        printf("Failed to create remote\n");
        r = -1;
        goto free_repo;
    }
#ifdef DEMO_MANUAL_DOWNLOAD
    if (git_remote_connect(remote, GIT_DIRECTION_FETCH, &callbacks, &fetch_opts.proxy_opts, NULL)) {
        printf("Failed to connect\n");
        r = -1;
        goto free_remote;
    }
    if (git_remote_download(remote, &refspecs, &fetch_opts)) {
        printf("Failed to download\n");
        r = -1;
        goto free_remote;
    }
    if (git_remote_disconnect(remote)) {
        printf("Failed to disconnect\n");
        r = -1;
        goto free_remote;
    }
#else
    if (git_remote_fetch(remote, &refspecs, &fetch_opts, NULL)) {
        printf("Failed to fetch\n");
        r = -1;
        goto free_remote;
    }
#endif
    r = 0;
free_remote:
    git_remote_free(remote);
free_repo:
    git_repository_free(repo);
    git_libgit2_shutdown();
    return r;
}

Compile with (where /tmp/libgit2_debug stores a libgit2 build with address sanitizer enabled)

gcc demo.c -fsanitize=address -L/tmp/libgit2_debug -lgit2 -DDEMO_MANUAL_DOWNLOAD=yes -o demo_download
gcc demo.c -fsanitize=address -L/tmp/libgit2_debug -lgit2 -o demo_fetch

Run with

LD_LIBRARY_PATH=/tmp/libgit2_debug ./demo_fetch
LD_LIBRARY_PATH=/tmp/libgit2_debug ./demo_download

connect_opts is created with its custom_headers and proxy_opts->url possibly
allocated on heap, just like in git_remote_fetch(). But unlike in
_fetch(), it is not disposed at the end of the function, thus causing
memory leak.
@ethomson
Copy link
Member

Thanks! Good catch. 😁

@ethomson ethomson merged commit d947561 into libgit2:main Nov 20, 2023
26 checks passed
@ethomson ethomson added the bug label Nov 20, 2023
@7Ji 7Ji deleted the download_leak_fix branch November 20, 2023 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants