This changes the signature of _alpm_pkg_dup() to return an integer error code and provide the new package in a passed pointer argument. All callers are now more robust with checking the return value of this function to ensure a fatal error did not occur. We allow load failures to proceed as otherwise we have a chicken and egg problem- if a 'desc' local database entry is missing, the best way of restoring said file is `pacman -Sf --dbonly packagename`. This patch fixes a segfault that was occurring in this case. Fixes the segfault reported in FS#25667. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/add.c | 11 +++++++---- lib/libalpm/db.c | 3 +-- lib/libalpm/deps.c | 12 +++++++++--- lib/libalpm/deps.h | 2 +- lib/libalpm/package.c | 28 ++++++++++++++++++++++++---- lib/libalpm/package.h | 2 +- lib/libalpm/remove.c | 35 ++++++++++++++++++++++++++--------- lib/libalpm/sync.c | 6 +++++- 8 files changed, 74 insertions(+), 25 deletions(-) diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index 8bbf157..cb8551e 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -468,19 +468,22 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg, is_upgrade = 1; /* we'll need to save some record for backup checks later */ - oldpkg = _alpm_pkg_dup(local); + if(_alpm_pkg_dup(local, &oldpkg) == -1) { + ret = -1; + goto cleanup; + } - EVENT(trans, ALPM_TRANS_EVT_UPGRADE_START, newpkg, oldpkg); + EVENT(trans, ALPM_TRANS_EVT_UPGRADE_START, newpkg, local); _alpm_log(handle, ALPM_LOG_DEBUG, "upgrading package %s-%s\n", newpkg->name, newpkg->version); /* copy over the install reason */ - newpkg->reason = alpm_pkg_get_reason(oldpkg); + newpkg->reason = alpm_pkg_get_reason(local); /* pre_upgrade scriptlet */ if(alpm_pkg_has_scriptlet(newpkg) && !(trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) { _alpm_runscriptlet(handle, newpkg->origin_data.file, - "pre_upgrade", newpkg->version, oldpkg->version); + "pre_upgrade", newpkg->version, local->version); } } else { is_upgrade = 0; diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c index 77bdf98..33282b8 100644 --- a/lib/libalpm/db.c +++ b/lib/libalpm/db.c @@ -553,8 +553,7 @@ int _alpm_db_add_pkgincache(alpm_db_t *db, alpm_pkg_t *pkg) return -1; } - newpkg = _alpm_pkg_dup(pkg); - if(newpkg == NULL) { + if(_alpm_pkg_dup(pkg, &newpkg)) { return -1; } diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index 704a904..47b7637 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -512,13 +512,14 @@ static int can_remove_package(alpm_db_t *db, alpm_pkg_t *pkg, * @param db package database to do dependency tracing in * @param *targs pointer to a list of packages * @param include_explicit if 0, explicitly installed packages are not included + * @return 0 on success, -1 on errors */ -void _alpm_recursedeps(alpm_db_t *db, alpm_list_t *targs, int include_explicit) +int _alpm_recursedeps(alpm_db_t *db, alpm_list_t *targs, int include_explicit) { alpm_list_t *i, *j; if(db == NULL || targs == NULL) { - return; + return -1; } for(i = targs; i; i = i->next) { @@ -527,13 +528,18 @@ void _alpm_recursedeps(alpm_db_t *db, alpm_list_t *targs, int include_explicit) alpm_pkg_t *deppkg = j->data; if(_alpm_dep_edge(pkg, deppkg) && can_remove_package(db, deppkg, targs, include_explicit)) { + alpm_pkg_t *copy; _alpm_log(db->handle, ALPM_LOG_DEBUG, "adding '%s' to the targets\n", deppkg->name); /* add it to the target list */ - targs = alpm_list_add(targs, _alpm_pkg_dup(deppkg)); + if(_alpm_pkg_dup(deppkg, ©)) { + return -1; + } + targs = alpm_list_add(targs, copy); } } } + return 0; } /** diff --git a/lib/libalpm/deps.h b/lib/libalpm/deps.h index 29e69eb..ce25bda 100644 --- a/lib/libalpm/deps.h +++ b/lib/libalpm/deps.h @@ -31,7 +31,7 @@ void _alpm_dep_free(alpm_depend_t *dep); alpm_depend_t *_alpm_dep_dup(const alpm_depend_t *dep); void _alpm_depmiss_free(alpm_depmissing_t *miss); alpm_list_t *_alpm_sortbydeps(alpm_handle_t *handle, alpm_list_t *targets, int reverse); -void _alpm_recursedeps(alpm_db_t *db, alpm_list_t *targs, int include_explicit); +int _alpm_recursedeps(alpm_db_t *db, alpm_list_t *targs, int include_explicit); int _alpm_resolvedeps(alpm_handle_t *handle, alpm_list_t *localpkgs, alpm_pkg_t *pkg, alpm_list_t *preferred, alpm_list_t **packages, alpm_list_t *remove, alpm_list_t **data); diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c index d865ac9..3d73a43 100644 --- a/lib/libalpm/package.c +++ b/lib/libalpm/package.c @@ -467,13 +467,32 @@ alpm_pkg_t *_alpm_pkg_new(void) return pkg; } -alpm_pkg_t *_alpm_pkg_dup(alpm_pkg_t *pkg) +/** + * Duplicate a package data struct. + * @param pkg the package to duplicate + * @param new_ptr location to store duplicated package pointer + * @return 0 on success, -1 on fatal error, 1 on non-fatal error + */ +int _alpm_pkg_dup(alpm_pkg_t *pkg, alpm_pkg_t **new_ptr) { alpm_pkg_t *newpkg; alpm_list_t *i; + int ret = 0; + + if(!pkg || !pkg->handle) { + return -1; + } + + if(!new_ptr) { + RET_ERR(pkg->handle, ALPM_ERR_WRONG_ARGS, -1); + } if(pkg->ops->force_load(pkg)) { - return NULL; + _alpm_log(pkg->handle, ALPM_LOG_WARNING, + _("could not fully load metadata for package %s-%s\n"), + pkg->name, pkg->version); + ret = 1; + pkg->handle->pm_errno = ALPM_ERR_PKG_INVALID; } CALLOC(newpkg, 1, sizeof(alpm_pkg_t), goto cleanup); @@ -540,11 +559,12 @@ alpm_pkg_t *_alpm_pkg_dup(alpm_pkg_t *pkg) newpkg->ops = pkg->ops; newpkg->handle = pkg->handle; - return newpkg; + *new_ptr = newpkg; + return ret; cleanup: _alpm_pkg_free(newpkg); - return NULL; + RET_ERR(pkg->handle, ALPM_ERR_MEMORY, -1); } void _alpm_pkg_free(alpm_pkg_t *pkg) diff --git a/lib/libalpm/package.h b/lib/libalpm/package.h index bb92ddc..25d1b1a 100644 --- a/lib/libalpm/package.h +++ b/lib/libalpm/package.h @@ -144,7 +144,7 @@ alpm_file_t *_alpm_file_copy(alpm_file_t *dest, const alpm_file_t *src); int _alpm_files_cmp(const void *f1, const void *f2); alpm_pkg_t* _alpm_pkg_new(void); -alpm_pkg_t *_alpm_pkg_dup(alpm_pkg_t *pkg); +int _alpm_pkg_dup(alpm_pkg_t *pkg, alpm_pkg_t **new_ptr); void _alpm_pkg_free(alpm_pkg_t *pkg); void _alpm_pkg_free_trans(alpm_pkg_t *pkg); diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index c6886c6..7903a0f 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -49,6 +49,7 @@ int SYMEXPORT alpm_remove_pkg(alpm_handle_t *handle, alpm_pkg_t *pkg) { const char *pkgname; alpm_trans_t *trans; + alpm_pkg_t *copy; /* Sanity checks */ CHECK_HANDLE(handle, return -1); @@ -67,11 +68,14 @@ int SYMEXPORT alpm_remove_pkg(alpm_handle_t *handle, alpm_pkg_t *pkg) _alpm_log(handle, ALPM_LOG_DEBUG, "adding package %s to the transaction remove list\n", pkgname); - trans->remove = alpm_list_add(trans->remove, _alpm_pkg_dup(pkg)); + if(_alpm_pkg_dup(pkg, ©) == -1) { + return -1; + } + trans->remove = alpm_list_add(trans->remove, copy); return 0; } -static void remove_prepare_cascade(alpm_handle_t *handle, alpm_list_t *lp) +static int remove_prepare_cascade(alpm_handle_t *handle, alpm_list_t *lp) { alpm_trans_t *trans = handle->trans; @@ -81,14 +85,18 @@ static void remove_prepare_cascade(alpm_handle_t *handle, alpm_list_t *lp) alpm_depmissing_t *miss = i->data; alpm_pkg_t *info = _alpm_db_get_pkgfromcache(handle->db_local, miss->target); if(info) { + alpm_pkg_t *copy; if(!_alpm_pkg_find(trans->remove, info->name)) { _alpm_log(handle, ALPM_LOG_DEBUG, "pulling %s in target list\n", info->name); - trans->remove = alpm_list_add(trans->remove, _alpm_pkg_dup(info)); + if(_alpm_pkg_dup(info, ©) == -1) { + return -1; + } + trans->remove = alpm_list_add(trans->remove, copy); } } else { - _alpm_log(handle, ALPM_LOG_ERROR, _("could not find %s in database -- skipping\n"), - miss->target); + _alpm_log(handle, ALPM_LOG_ERROR, + _("could not find %s in database -- skipping\n"), miss->target); } } alpm_list_free_inner(lp, (alpm_list_fn_free)_alpm_depmiss_free); @@ -96,6 +104,7 @@ static void remove_prepare_cascade(alpm_handle_t *handle, alpm_list_t *lp) lp = alpm_checkdeps(handle, _alpm_db_get_pkgcache(handle->db_local), trans->remove, NULL, 1); } + return 0; } static void remove_prepare_keep_needed(alpm_handle_t *handle, alpm_list_t *lp) @@ -134,6 +143,7 @@ static void remove_prepare_keep_needed(alpm_handle_t *handle, alpm_list_t *lp) * the packages blocking the transaction. * @param handle the context handle * @param data a pointer to an alpm_list_t* to fill + * @return 0 on success, -1 on error */ int _alpm_remove_prepare(alpm_handle_t *handle, alpm_list_t **data) { @@ -144,8 +154,10 @@ int _alpm_remove_prepare(alpm_handle_t *handle, alpm_list_t **data) if((trans->flags & ALPM_TRANS_FLAG_RECURSE) && !(trans->flags & ALPM_TRANS_FLAG_CASCADE)) { _alpm_log(handle, ALPM_LOG_DEBUG, "finding removable dependencies\n"); - _alpm_recursedeps(db, trans->remove, - trans->flags & ALPM_TRANS_FLAG_RECURSEALL); + if(_alpm_recursedeps(db, trans->remove, + trans->flags & ALPM_TRANS_FLAG_RECURSEALL)) { + return -1; + } } if(!(trans->flags & ALPM_TRANS_FLAG_NODEPS)) { @@ -156,7 +168,9 @@ int _alpm_remove_prepare(alpm_handle_t *handle, alpm_list_t **data) if(lp != NULL) { if(trans->flags & ALPM_TRANS_FLAG_CASCADE) { - remove_prepare_cascade(handle, lp); + if(remove_prepare_cascade(handle, lp)) { + return -1; + } } else if(trans->flags & ALPM_TRANS_FLAG_UNNEEDED) { /* Remove needed packages (which would break dependencies) * from target list */ @@ -184,7 +198,10 @@ int _alpm_remove_prepare(alpm_handle_t *handle, alpm_list_t **data) if((trans->flags & ALPM_TRANS_FLAG_CASCADE) && (trans->flags & ALPM_TRANS_FLAG_RECURSE)) { _alpm_log(handle, ALPM_LOG_DEBUG, "finding removable dependencies\n"); - _alpm_recursedeps(db, trans->remove, trans->flags & ALPM_TRANS_FLAG_RECURSEALL); + if(_alpm_recursedeps(db, trans->remove, + trans->flags & ALPM_TRANS_FLAG_RECURSEALL)) { + return -1; + } } if(!(trans->flags & ALPM_TRANS_FLAG_NODEPS)) { diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 6c30c94..e3e7fa9 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -558,8 +558,12 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) for(j = spkg->removes; j; j = j->next) { alpm_pkg_t *rpkg = j->data; if(!_alpm_pkg_find(trans->remove, rpkg->name)) { + alpm_pkg_t *copy; _alpm_log(handle, ALPM_LOG_DEBUG, "adding '%s' to remove list\n", rpkg->name); - trans->remove = alpm_list_add(trans->remove, _alpm_pkg_dup(rpkg)); + if(_alpm_pkg_dup(rpkg, ©) == -1) { + return -1; + } + trans->remove = alpm_list_add(trans->remove, copy); } } } -- 1.7.6