Be more robust when copying package data

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

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

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(_alpm_pkg_dup(pkg, &newpkg)) {
-	if(newpkg == NULL) {
 		return -1;
 	}
 

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, &copy)) {
+					return -1;
+				}
+				targs = alpm_list_add(targs, copy);
 			}
 		}
 	}
+	return 0;
 }
 
 /**

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

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)

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

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, &copy) == -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, &copy) == -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"),
+				_alpm_log(handle, ALPM_LOG_ERROR,
-									miss->target);
+						_("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,
+		if(_alpm_recursedeps(db, trans->remove,
-				trans->flags & ALPM_TRANS_FLAG_RECURSEALL);
+				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)) {

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, &copy) == -1) {
+					return -1;
+				}
+				trans->remove = alpm_list_add(trans->remove, copy);
 			}
 		}
 	}