diff options
| author | Dan McGee <dan@archlinux.org> | 2012-02-29 16:47:39 -0600 | 
|---|---|---|
| committer | Dan McGee <dan@archlinux.org> | 2012-03-05 11:44:34 -0600 | 
| commit | 986e99a613605985f64f0e3e4c2635717931f77d (patch) | |
| tree | 074a7197e1bfb2c301687873a6a6dde16ea1e683 | |
| parent | 4b384b7f0b0e840e09e3bffd2dbb59b88bdd4864 (diff) | |
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 <dan@archlinux.org>
| -rw-r--r-- | lib/libalpm/be_package.c | 32 | 
1 files changed, 19 insertions, 13 deletions
| 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;  	} | 
