diff mbox

[pacman-dev] detect pkghash allocation failure

Message ID 20180108003042.22980-2-andrew.gregory.8@gmail.com
State Accepted, archived
Headers show

Commit Message

Andrew Gregory Jan. 8, 2018, 12:30 a.m. UTC 
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 <andrew.gregory.8@gmail.com>
---

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

Comments

Allan McRae Jan. 10, 2018, 12:52 a.m. UTC | #1 
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 <andrew.gregory.8@gmail.com>
> ---
> 
> 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...
diff mbox

Patch

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