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

Fix a memory leak in glob_for_cachedir() #1671

Conversation

ppisar
Copy link
Contributor

@ppisar ppisar commented Jul 17, 2024

Covscan complains:

Error: RESOURCE_LEAK (CWE-772): [#def1] [important]
libdnf-0.73.1/libdnf/hy-iutil.cpp:100:5: alloc_arg: "wordexp" allocates memory that is stored into "word_vector.we_wordv".
libdnf-0.73.1/libdnf/hy-iutil.cpp:102:9: leaked_storage: Variable "word_vector" going out of scope leaks the storage "word_vector.we_wordv" points to.
#  100|       if (wordexp(p, &word_vector, 0)) {
#  101|           g_free(p);
#  102|->         return ret;
#  103|       }
#  104|       for (guint i = 0; i < word_vector.we_wordc; ++i) {

The issue is that Covscan model thinks that word_vector should be freed after failing wordexp(). glibc's manual does not explain whether it is or isn't necessary. However, POSIX manual mentions that the memory is valid on WRDE_NOSPACE (not enough memory) error. Reading glibc sources confirms that wordexp() on any error except of WRDE_NOSPACE cleans up and returns original, intact word_vector.

Therefore I recognize the missing wordfree() call as an error and this patch fixed it.

Covscan complains:

    Error: RESOURCE_LEAK (CWE-772): [#def1] [important]
    libdnf-0.73.1/libdnf/hy-iutil.cpp:100:5: alloc_arg: "wordexp" allocates memory that is stored into "word_vector.we_wordv".
    libdnf-0.73.1/libdnf/hy-iutil.cpp:102:9: leaked_storage: Variable "word_vector" going out of scope leaks the storage "word_vector.we_wordv" points to.
    #  100|       if (wordexp(p, &word_vector, 0)) {
    #  101|           g_free(p);
    #  102|->         return ret;
    #  103|       }
    #  104|       for (guint i = 0; i < word_vector.we_wordc; ++i) {

The issue is that Covscan model thinks that word_vector should be
freed after failing wordexp(). glibc's manual does not explain whether
it is or isn't necessary. However, POSIX manual mentions that the
memory is valid on WRDE_NOSPACE (not enough memory) error. Reading
glibc sources confirms that wordexp() on any error except of
WRDE_NOSPACE cleans up and returns original, intact word_vector.

Therefore I recognize the missing wordfree() call as an error and
this patch fixed it.
@kontura kontura merged commit b245193 into rpm-software-management:dnf-4-master Jul 18, 2024
6 of 9 checks passed
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.

2 participants