[pacman-dev] [PATCH] _alpm_backup_dup: fix memory leak in error case
Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/backup.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/libalpm/backup.c b/lib/libalpm/backup.c index c2989b6..aeb4131 100644 --- a/lib/libalpm/backup.c +++ b/lib/libalpm/backup.c @@ -87,10 +87,15 @@ alpm_backup_t *_alpm_backup_dup(const alpm_backup_t *backup) alpm_backup_t *newbackup; CALLOC(newbackup, 1, sizeof(alpm_backup_t), return NULL); - STRDUP(newbackup->name, backup->name, return NULL); - STRDUP(newbackup->hash, backup->hash, return NULL); + STRDUP(newbackup->name, backup->name, goto error); + STRDUP(newbackup->hash, backup->hash, goto error); return newbackup; + +error: + free(newbackup->name); + free(newbackup); + return NULL; } /* vim: set noet: */ -- 2.1.3
A corrupt local db filelist could result in a realloc of size zero. Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/be_local.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 9376396..b8840d2 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -791,7 +791,7 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) files_count++; } /* attempt to hand back any memory we don't need */ - files = realloc(files, sizeof(alpm_file_t) * files_count); + files = realloc(files, files_count ? sizeof(alpm_file_t) * files_count : 1); /* make sure the list is sorted */ qsort(files, files_count, sizeof(alpm_file_t), _alpm_files_cmp); info->files.count = files_count; -- 2.1.3
On 11/18/14 at 12:51am, Allan McRae wrote:
A corrupt local db filelist could result in a realloc of size zero.
Maybe I'm missing something... Calling realloc with size zero should free the memory and return NULL, which seems like the right thing to do if there are no files.
Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/be_local.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 9376396..b8840d2 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -791,7 +791,7 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) files_count++; } /* attempt to hand back any memory we don't need */ - files = realloc(files, sizeof(alpm_file_t) * files_count); + files = realloc(files, files_count ? sizeof(alpm_file_t) * files_count : 1); /* make sure the list is sorted */ qsort(files, files_count, sizeof(alpm_file_t), _alpm_files_cmp); info->files.count = files_count; -- 2.1.3
On 18/11/14 02:20, Andrew Gregory wrote:
On 11/18/14 at 12:51am, Allan McRae wrote:
A corrupt local db filelist could result in a realloc of size zero.
Maybe I'm missing something... Calling realloc with size zero should free the memory and return NULL, which seems like the right thing to do if there are no files.
You are correct. I thought a realloc(foo, 0) was undefined like malloc(0) is. I guess this is a false positive in the clang analyser. Allan
The memory assigned to checkfile was leaked in the error condition. Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/add.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index ee2b7ec..20f5139 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -317,7 +317,7 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, if(!backup->name || strcmp(backup->name, entryname_orig) != 0) { continue; } - STRDUP(newhash, hash_pkg, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); + STRDUP(newhash, hash_pkg, errors++; handle->pm_errno = ALPM_ERR_MEMORY; goto needbackup_cleanup); FREE(backup->hash); backup->hash = newhash; } -- 2.1.3
participants (2)
-
Allan McRae
-
Andrew Gregory