[pacman-dev] [PATCH] detect pkghash allocation failure
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(-) 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
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...
participants (2)
-
Allan McRae
-
Andrew Gregory