From 4b384b7f0b0e840e09e3bffd2dbb59b88bdd4864 Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Wed, 29 Feb 2012 16:33:21 -0600 Subject: Fix a memory leak when loading an invalid package This is easily triggered via a `pacman -Sc` operation when it attempts to open a delta file as a package- we end up leaking loads of memory due to us never freeing the archive object. When you have upwards of 1200 delta files in your sync database directory, this results in a memory leak of nearly 1.5 MiB. Also fix another memory leak noticed at the same time- we need to call the internal _alpm_pkg_free() function, as without the origin data being set the public free function will do nothing. Signed-off-by: Dan McGee --- lib/libalpm/be_package.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'lib/libalpm/be_package.c') diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 4d9d0e82..ad34640a 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -382,7 +382,7 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, /* try to create an archive object to read in the package */ if((archive = archive_read_new()) == NULL) { - alpm_pkg_free(newpkg); + _alpm_pkg_free(newpkg); RET_ERR(handle, ALPM_ERR_LIBARCHIVE, NULL); } @@ -391,8 +391,8 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, if(archive_read_open_filename(archive, pkgfile, ALPM_BUFFER_SIZE) != ARCHIVE_OK) { - alpm_pkg_free(newpkg); - RET_ERR(handle, ALPM_ERR_PKG_OPEN, NULL); + handle->pm_errno = ALPM_ERR_PKG_OPEN; + goto error; } _alpm_log(handle, ALPM_LOG_DEBUG, "starting package load for %s\n", pkgfile); -- cgit v1.2.3-24-g4f1b From 986e99a613605985f64f0e3e4c2635717931f77d Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Wed, 29 Feb 2012 16:47:39 -0600 Subject: Fix a potential memory leak in filelist creation If we begin to create a file list when loading a package, but abort because of an error to one of our goto labels, the memory used to create the file list will leak. This is because we use a set of local variables to hold the data, and thus _alpm_pkg_free() cannot clean up for us. Use the file list struct on the package object as much as possible to keep state when building the file list, thus allowing _alpm_pkg_free() to clean up any partially built data. Signed-off-by: Dan McGee --- lib/libalpm/be_package.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) (limited to 'lib/libalpm/be_package.c') diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index ad34640a..93b762a1 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -360,8 +360,7 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, struct archive_entry *entry; alpm_pkg_t *newpkg = NULL; struct stat st; - size_t files_count = 0, files_size = 0; - alpm_file_t *files = NULL; + size_t files_size = 0; if(pkgfile == NULL || strlen(pkgfile) == 0) { RET_ERR(handle, ALPM_ERR_WRONG_ARGS, NULL); @@ -426,28 +425,34 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, /* for now, ignore all files starting with '.' that haven't * already been handled (for future possibilities) */ } else if(full) { + const size_t files_count = newpkg->files.count; + alpm_file_t *current_file; /* Keep track of all files for filelist generation */ if(files_count >= files_size) { size_t old_size = files_size; + alpm_file_t *newfiles; if(files_size == 0) { files_size = 4; } else { files_size *= 2; } - files = realloc(files, sizeof(alpm_file_t) * files_size); - if(!files) { + newfiles = realloc(newpkg->files.files, + sizeof(alpm_file_t) * files_size); + if(!newfiles) { ALLOC_FAIL(sizeof(alpm_file_t) * files_size); goto error; } /* ensure all new memory is zeroed out, in both the initial * allocation and later reallocs */ - memset(files + old_size, 0, + memset(newfiles + old_size, 0, sizeof(alpm_file_t) * (files_size - old_size)); + newpkg->files.files = newfiles; } - STRDUP(files[files_count].name, entry_name, goto error); - files[files_count].size = archive_entry_size(entry); - files[files_count].mode = archive_entry_mode(entry); - files_count++; + current_file = newpkg->files.files + files_count; + STRDUP(current_file->name, entry_name, goto error); + current_file->size = archive_entry_size(entry); + current_file->mode = archive_entry_mode(entry); + newpkg->files.count++; } if(archive_read_data_skip(archive)) { @@ -485,15 +490,16 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, newpkg->infolevel = INFRQ_BASE | INFRQ_DESC | INFRQ_SCRIPTLET; if(full) { - if(files) { + if(newpkg->files.files) { /* attempt to hand back any memory we don't need */ - files = realloc(files, sizeof(alpm_file_t) * files_count); + newpkg->files.files = realloc(newpkg->files.files, + sizeof(alpm_file_t) * newpkg->files.count); /* "checking for conflicts" requires a sorted list, ensure that here */ _alpm_log(handle, ALPM_LOG_DEBUG, "sorting package filelist for %s\n", pkgfile); - newpkg->files.files = files_msort(files, files_count); + newpkg->files.files = files_msort(newpkg->files.files, + newpkg->files.count); } - newpkg->files.count = files_count; newpkg->infolevel |= INFRQ_FILES; } -- cgit v1.2.3-24-g4f1b