Re: [pacman-dev] [PATCH v2] Fix handling of unavailable backup hashes

2016-04-11 Thread Andrew Gregory
On 04/11/16 at 10:51am, Will Miles wrote:
> This patch fixes several places where an unavailable (NULL) backup hash may be
> passed to a printf() call, which segfaults on some libcs.  This includes
> local database writes where unavailable hashes are represented by the file
> name only.  This condition may occur when an updated package changes the
> contents of its backup=() list.
> 
> Signed-off-by: Will Miles 
> ---
>  lib/libalpm/add.c  | 6 +++---
>  lib/libalpm/be_local.c | 6 +-
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
> index f5c9a95..629e2a5 100644
> --- a/lib/libalpm/add.c
> +++ b/lib/libalpm/add.c
> @@ -339,9 +339,9 @@ static int extract_single_file(alpm_handle_t *handle, 
> struct archive *archive,
>   hash_pkg = backup ? backup->hash : 
> alpm_compute_md5sum(filename);
>  
>   _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, "current:  %s\n", (hash_local 
> ? hash_local : "NULL") );
> + _alpm_log(handle, ALPM_LOG_DEBUG, "new:  %s\n", (hash_pkg ? 
> hash_pkg : "NULL") );
> + _alpm_log(handle, ALPM_LOG_DEBUG, "original: %s\n", (hash_orig 
> ? hash_orig : "NULL") );
>  
>   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..21166cb 100644
> --- a/lib/libalpm/be_local.c
> +++ b/lib/libalpm/be_local.c
> @@ -1037,7 +1037,11 @@ 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);
> + } else {
> + fprintf(fp, "%s\n", backup->name);

This will break the parser if the file name contains a tab.  Just do
something similar to what you did above:

 fprintf(fp, "%s\t%s\n", backup->name, backup->hash ? backup->hash : "");

The parser also needs to be updated to use strrchr instead of strchr
for the same reason, but that's another patch.

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


[pacman-dev] [PATCH v2] Fix handling of unavailable backup hashes

2016-04-11 Thread Will Miles
This patch fixes several places where an unavailable (NULL) backup hash may be
passed to a printf() call, which segfaults on some libcs.  This includes
local database writes where unavailable hashes are represented by the file
name only.  This condition may occur when an updated package changes the
contents of its backup=() list.

Signed-off-by: Will Miles 
---
 lib/libalpm/add.c  | 6 +++---
 lib/libalpm/be_local.c | 6 +-
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
index f5c9a95..629e2a5 100644
--- a/lib/libalpm/add.c
+++ b/lib/libalpm/add.c
@@ -339,9 +339,9 @@ static int extract_single_file(alpm_handle_t *handle, 
struct archive *archive,
hash_pkg = backup ? backup->hash : 
alpm_compute_md5sum(filename);
 
_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, "current:  %s\n", (hash_local 
? hash_local : "NULL") );
+   _alpm_log(handle, ALPM_LOG_DEBUG, "new:  %s\n", (hash_pkg ? 
hash_pkg : "NULL") );
+   _alpm_log(handle, ALPM_LOG_DEBUG, "original: %s\n", (hash_orig 
? hash_orig : "NULL") );
 
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..21166cb 100644
--- a/lib/libalpm/be_local.c
+++ b/lib/libalpm/be_local.c
@@ -1037,7 +1037,11 @@ 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);
+   } else {
+   fprintf(fp, "%s\n", backup->name);
+   }
}
fputc('\n', fp);
}
-- 
2.6.3