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

[Will Miles]

On 2016-04-02 2:55 AM, Allan McRae wrote:
> On 02/04/16 00:52, 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>
>> ---
> Just as a query, where are you seeing these non-existent backup hashes?
>   Corrupt local databases?


I ran in to it when I had a case where I was upgrading a package that 
had a new file added to the backup list.   Something like these PKGBUILD 
fragments:

pkgname=example
pkgver=3.2.0
backup=( 'etc/example.conf' )

getting upgraded to:

pkgname=example
pkgver=4.0.0
backup=( 'etc/example.conf' 'etc/example_auth.conf' )

When the upgrade install is running, there's no hash for 
'etc/example_auth.conf' in the database yet.   I think that's the 
expected behaviour, though, and it seems to be handled correctly 
everywhere except these two places where passing the NULL into printf() 
broke QNX's libc.   We ran into it while developing PKGBUILDs for some 
of our internal software.

-Will

>
>>   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") );
>>   
> Fine - I guess this is needed for non-glibc systems.
>
>>   		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);
>> +				}
> How can this ever happen?
>
>>   			}
>>   			fputc('\n', fp);
>>   		}
>>