[pacman-dev] [PATCH] Fix segfault during install when a package update adds a new file to 'backup'

[Will Miles]

On 2016-04-05 12:13 AM, Andrew Gregory wrote:
> On 04/01/16 at 10:52am, Will Miles wrote:
>> When a new package release adds a new entry to the backup list, no hash exists
>> for that file in the previously installed package version.  Fix two places
>> where that nonexistent hash may be dereferenced.
>>
>> Signed-off-by: Will Miles <wmiles at sgl.com>
>> ---
>>   lib/libalpm/add.c      | 2 +-
>>   lib/libalpm/be_local.c | 4 +++-
>>   2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
>> index f5c9a95..ff0b81d 100644
>> --- a/lib/libalpm/add.c
>> +++ b/lib/libalpm/add.c
>> @@ -341,7 +341,7 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive,
>>   		_alpm_log(handle, ALPM_LOG_DEBUG, "checking hashes for %s\n", origfile);
>>   		_alpm_log(handle, ALPM_LOG_DEBUG, "current:  %s\n", hash_local);
>>   		_alpm_log(handle, ALPM_LOG_DEBUG, "new:      %s\n", hash_pkg);
>> -		_alpm_log(handle, ALPM_LOG_DEBUG, "original: %s\n", hash_orig);
>> +		_alpm_log(handle, ALPM_LOG_DEBUG, "original: %s\n", (hash_orig ? hash_orig : "NULL") );
> All of these hashed can potentially be NULL if we encounter an error
> when reading the file.  This check should be applied to all of them.

OK.   For n=3, would you prefer an inline function or a macro?

>
>>   		if(hash_local && hash_pkg && strcmp(hash_local, hash_pkg) == 0) {
>>   			/* local and new files are the same, updating anyway to get
>> diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c
>> index f817822..5e39199 100644
>> --- a/lib/libalpm/be_local.c
>> +++ b/lib/libalpm/be_local.c
>> @@ -1037,7 +1037,9 @@ int _alpm_local_db_write(alpm_db_t *db, alpm_pkg_t *info, alpm_dbinfrq_t inforeq
>>   			fputs("%BACKUP%\n", fp);
>>   			for(lp = info->backup; lp; lp = lp->next) {
>>   				const alpm_backup_t *backup = lp->data;
>> -				fprintf(fp, "%s\t%s\n", backup->name, backup->hash);
>> +				if (backup->hash) {
>> +					fprintf(fp, "%s\t%s\n", backup->name, backup->hash);
>> +				}
> We should still add these entries to the database even if we don't
> have hashes for them.  Just skip printing the hash itself (or replace
> it with an empty string) if it's NULL.

OK.  I'll review the read hash operation to make sure it handles reading 
an empty string as a non-error as well.

>
>>   			}
>>   			fputc('\n', fp);
>>   		}
>> -- 
>> 2.6.3