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 --- 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); }

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