[PATCH] detect pkghash allocation failure

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] detect pkghash allocation failure

Andrew Gregory
If rehash ever failed with a full hash it would return the old hash
that is already full.  get_hash_position would then loop forever
because it would never find an empty bucket.

Signed-off-by: Andrew Gregory <[hidden email]>
---

We still don't actually handle pkghash creation errors anywhere, but
at least it won't loop forever anymore.

 lib/libalpm/be_local.c |  5 ++++-
 lib/libalpm/be_sync.c  |  5 ++++-
 lib/libalpm/db.c       |  5 ++++-
 lib/libalpm/pkghash.c  | 23 ++++++++++++++---------
 lib/libalpm/pkghash.h  |  4 ++--
 5 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c
index 97a49688..1ec6a20f 100644
--- a/lib/libalpm/be_local.c
+++ b/lib/libalpm/be_local.c
@@ -601,7 +601,10 @@ static int local_db_populate(alpm_db_t *db)
  /* add to the collection */
  _alpm_log(db->handle, ALPM_LOG_FUNCTION, "adding '%s' to package cache for db '%s'\n",
  pkg->name, db->treename);
- db->pkgcache = _alpm_pkghash_add(db->pkgcache, pkg);
+ if(_alpm_pkghash_add(&db->pkgcache, pkg) == NULL) {
+ _alpm_pkg_free(pkg);
+ RET_ERR(db->handle, ALPM_ERR_MEMORY, -1);
+ }
  count++;
  }
 
diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index 84a00c0f..35cd8a4d 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -413,7 +413,10 @@ static alpm_pkg_t *load_pkg_for_entry(alpm_db_t *db, const char *entryname,
  /* add to the collection */
  _alpm_log(db->handle, ALPM_LOG_FUNCTION, "adding '%s' to package cache for db '%s'\n",
  pkg->name, db->treename);
- db->pkgcache = _alpm_pkghash_add(db->pkgcache, pkg);
+ if(_alpm_pkghash_add(&db->pkgcache, pkg) == NULL) {
+ _alpm_pkg_free(pkg);
+ RET_ERR(db->handle, ALPM_ERR_MEMORY, NULL);
+ }
  } else {
  free(pkgname);
  free(pkgver);
diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c
index 789478e8..308fb9c7 100644
--- a/lib/libalpm/db.c
+++ b/lib/libalpm/db.c
@@ -589,7 +589,10 @@ int _alpm_db_add_pkgincache(alpm_db_t *db, alpm_pkg_t *pkg)
  ? ALPM_PKG_FROM_LOCALDB
  : ALPM_PKG_FROM_SYNCDB;
  newpkg->origin_data.db = db;
- db->pkgcache = _alpm_pkghash_add_sorted(db->pkgcache, newpkg);
+ if(_alpm_pkghash_add_sorted(&db->pkgcache, newpkg) == NULL) {
+ _alpm_pkg_free(newpkg);
+ RET_ERR(db->handle, ALPM_ERR_MEMORY, -1);
+ }
 
  free_groupcache(db);
 
diff --git a/lib/libalpm/pkghash.c b/lib/libalpm/pkghash.c
index 45e63082..a0e81f28 100644
--- a/lib/libalpm/pkghash.c
+++ b/lib/libalpm/pkghash.c
@@ -132,8 +132,7 @@ static alpm_pkghash_t *rehash(alpm_pkghash_t *oldhash)
 
  newhash = _alpm_pkghash_create(newsize);
  if(newhash == NULL) {
- /* creation of newhash failed, stick with old one... */
- return oldhash;
+ return NULL;
  }
 
  newhash->list = oldhash->list;
@@ -156,23 +155,29 @@ static alpm_pkghash_t *rehash(alpm_pkghash_t *oldhash)
  return newhash;
 }
 
-static alpm_pkghash_t *pkghash_add_pkg(alpm_pkghash_t *hash, alpm_pkg_t *pkg,
+static alpm_pkghash_t *pkghash_add_pkg(alpm_pkghash_t **hashref, alpm_pkg_t *pkg,
  int sorted)
 {
  alpm_list_t *ptr;
  unsigned int position;
+ alpm_pkghash_t *hash;
 
- if(pkg == NULL || hash == NULL) {
- return hash;
+ if(pkg == NULL || hashref == NULL || *hashref == NULL) {
+ return NULL;
  }
+ hash = *hashref;
 
  if(hash->entries >= hash->limit) {
- hash = rehash(hash);
+ if((hash = rehash(hash)) == NULL) {
+ /* resizing failed and there are no more open buckets */
+ return NULL;
+ }
+ *hashref = hash;
  }
 
  position = get_hash_position(pkg->name_hash, hash);
 
- MALLOC(ptr, sizeof(alpm_list_t), return hash);
+ MALLOC(ptr, sizeof(alpm_list_t), return NULL);
 
  ptr->data = pkg;
  ptr->prev = ptr;
@@ -189,12 +194,12 @@ static alpm_pkghash_t *pkghash_add_pkg(alpm_pkghash_t *hash, alpm_pkg_t *pkg,
  return hash;
 }
 
-alpm_pkghash_t *_alpm_pkghash_add(alpm_pkghash_t *hash, alpm_pkg_t *pkg)
+alpm_pkghash_t *_alpm_pkghash_add(alpm_pkghash_t **hash, alpm_pkg_t *pkg)
 {
  return pkghash_add_pkg(hash, pkg, 0);
 }
 
-alpm_pkghash_t *_alpm_pkghash_add_sorted(alpm_pkghash_t *hash, alpm_pkg_t *pkg)
+alpm_pkghash_t *_alpm_pkghash_add_sorted(alpm_pkghash_t **hash, alpm_pkg_t *pkg)
 {
  return pkghash_add_pkg(hash, pkg, 1);
 }
diff --git a/lib/libalpm/pkghash.h b/lib/libalpm/pkghash.h
index dd163346..a5c02c19 100644
--- a/lib/libalpm/pkghash.h
+++ b/lib/libalpm/pkghash.h
@@ -49,8 +49,8 @@ typedef struct __alpm_pkghash_t alpm_pkghash_t;
 
 alpm_pkghash_t *_alpm_pkghash_create(unsigned int size);
 
-alpm_pkghash_t *_alpm_pkghash_add(alpm_pkghash_t *hash, alpm_pkg_t *pkg);
-alpm_pkghash_t *_alpm_pkghash_add_sorted(alpm_pkghash_t *hash, alpm_pkg_t *pkg);
+alpm_pkghash_t *_alpm_pkghash_add(alpm_pkghash_t **hash, alpm_pkg_t *pkg);
+alpm_pkghash_t *_alpm_pkghash_add_sorted(alpm_pkghash_t **hash, alpm_pkg_t *pkg);
 alpm_pkghash_t *_alpm_pkghash_remove(alpm_pkghash_t *hash, alpm_pkg_t *pkg, alpm_pkg_t **data);
 
 void _alpm_pkghash_free(alpm_pkghash_t *hash);
--
2.15.1
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] detect pkghash allocation failure

Allan McRae
On 08/01/18 10:30, Andrew Gregory wrote:

> If rehash ever failed with a full hash it would return the old hash
> that is already full.  get_hash_position would then loop forever
> because it would never find an empty bucket.
>
> Signed-off-by: Andrew Gregory <[hidden email]>
> ---
>
> We still don't actually handle pkghash creation errors anywhere, but
> at least it won't loop forever anymore.
>

Ack.

We don't really handle failure well throughout the codebase...