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

[Allan McRae]

On 06/04/16 04:23, Will Miles wrote:
> 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?
> 

Just as you wrote it above is fine.

>>
>>>           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.
> 

Thanks.

A