On 06/05/16 at 07:51pm, Tobias Stoeckmann wrote:

Some resources (memory or file descriptors) are not released on all error paths.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> --- Yes it's rather ironic to send this patch after forgetting one on my own just now. ;) --- lib/libalpm/add.c | 5 ++++- lib/libalpm/backup.c | 5 +++-- lib/libalpm/be_local.c | 18 +++++++++++++++--- lib/libalpm/be_package.c | 1 + 4 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index f5c9a95..d132e52 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -466,7 +466,7 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg, } }

- /* prepare directory for database entries so permission are correct after + /* prepare directory for database entries so permissions are correct after changelog/install script installation */ if(_alpm_local_db_prepare(db, newpkg)) { alpm_logaction(handle, ALPM_CALLER_PREFIX, @@ -503,6 +503,9 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg, _alpm_log(handle, ALPM_LOG_ERROR, _("could not change directory to %s (%s)\n"), handle->root, strerror(errno)); _alpm_archive_read_free(archive); + if(cwdfd >= 0) { + close(cwdfd); + } close(fd); ret = -1; goto cleanup; diff --git a/lib/libalpm/backup.c b/lib/libalpm/backup.c index f622589..50bad5e 100644 --- a/lib/libalpm/backup.c +++ b/lib/libalpm/backup.c @@ -48,9 +48,10 @@ int _alpm_split_backup(const char *string, alpm_backup_t **backup) ptr++; /* now str points to the filename and ptr points to the hash */ STRDUP((*backup)->name, str, FREE(str); return -1); - STRDUP((*backup)->hash, ptr, FREE(str); return -1); + STRDUP((*backup)->hash, ptr, FREE((*backup)->name); FREE(str); return -1); FREE(str); - return 0;} + return 0; +}

/* Look for a filename in a alpm_pkg_t.backup list. If we find it, * then we return the full backup entry. diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index f817822..0b351f9 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -794,7 +794,7 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) _alpm_strip_newline(line, 0); if(strcmp(line, "%FILES%") == 0) { size_t files_count = 0, files_size = 0, len; - alpm_file_t *files = NULL; + alpm_file_t *files = NULL, *newfiles;

while(safe_fgets(line, sizeof(line), fp) && (len = _alpm_strip_newline(line, 0))) { @@ -805,13 +805,18 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq)

Immediately above this is a call to _alpm_greedy_grow which needs memory released on failure as well.

/* since we know the length of the file string already, * we can do malloc + memcpy rather than strdup */ len += 1; - MALLOC(files[files_count].name, len, goto error); + MALLOC(files[files_count].name, len, goto nomem); memcpy(files[files_count].name, line, len); files_count++; } /* attempt to hand back any memory we don't need */ if(files_count > 0) { - files = realloc(files, sizeof(alpm_file_t) * files_count); + newfiles = realloc(files, sizeof(alpm_file_t) * files_count);

newfiles should be declared here; we limit variables to the smallest scope possible.

+ if(newfiles == NULL) { + goto nomem;

I don't think we need to treat this as an error. The realloc is just releasing memory; failure should not be significant.

+ } + files = newfiles; + /* make sure the list is sorted */ qsort(files, files_count, sizeof(alpm_file_t), _alpm_files_cmp); } else { @@ -819,6 +824,13 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) } info->files.count = files_count; info->files.files = files; + continue; +nomem: + while(files_count > 0) { + FREE(files[--files_count].name); + } + FREE(files); + goto error; } else if(strcmp(line, "%BACKUP%") == 0) { while(safe_fgets(line, sizeof(line), fp) && _alpm_strip_newline(line, 0)) { alpm_backup_t *backup; diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index c9ed770..a79c0c5 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -762,6 +762,7 @@ int SYMEXPORT alpm_pkg_load(alpm_handle_t *handle, const char *filename, int ful

if(fail) { _alpm_log(handle, ALPM_LOG_ERROR, _("required key missing from keyring\n")); + free(sigpath); return -1; } } -- 2.8.3