From: pcarana <pc.moreno2099@gmail.com>
Date: Tue, 3 Dec 2019 00:14:26 +0000 (-0600)
Subject: Fix bug due to bad error handling on multithreading.
X-Git-Tag: v1.1.3~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=8f5d21c9462ee5fb3f8928ce8cc1f21eb8a77777;p=thirdparty%2FFORT-validator.git

Fix bug due to bad error handling on multithreading.

+Don't consider validation results if at least one TAL has an error fetching it's root certificate. The bug was: on TAL 'hard error' (whenever the root certificate couldn't be fetched), the error was minimized and the rest of the TAL validation results were considered to update DB; this can lead to a considerable number of withdrawal PDUs for the routers.
---

diff --git a/src/object/tal.c b/src/object/tal.c
index f35fa489..48edaf99 100644
--- a/src/object/tal.c
+++ b/src/object/tal.c
@@ -42,6 +42,7 @@ struct tal {
 };
 
 struct fv_param {
+	int *exit_status; /* Return status of the file validation */
 	char *tal_file;
 	void *arg;
 };
@@ -49,6 +50,7 @@ struct fv_param {
 struct thread {
 	pthread_t pid;
 	char *file;
+	int *exit_status;
 	SLIST_ENTRY(thread) next;
 };
 
@@ -544,6 +546,8 @@ do_file_validation(void *thread_arg)
 end:
 	fnstack_cleanup();
 	free(param.tal_file);
+	/* param.exit_error isn't released since it's from parent thread */
+	*param.exit_status = error;
 	return NULL;
 }
 
@@ -551,6 +555,7 @@ static void
 thread_destroy(struct thread *thread)
 {
 	free(thread->file);
+	free(thread->exit_status);
 	free(thread);
 }
 
@@ -561,12 +566,20 @@ __do_file_validation(char const *tal_file, void *arg)
 	struct thread *thread;
 	struct fv_param *param;
 	static pthread_t pid;
+	int *exit_status;
 	int error;
 
-	param = malloc(sizeof(struct fv_param));
-	if (param == NULL)
+	exit_status = malloc(sizeof(int));
+	if (exit_status == NULL)
 		return pr_enomem();
 
+	param = malloc(sizeof(struct fv_param));
+	if (param == NULL) {
+		error = pr_enomem();
+		goto free_status;
+	}
+
+	param->exit_status = exit_status;
 	param->tal_file = strdup(tal_file);
 	param->arg = arg;
 
@@ -574,22 +587,24 @@ __do_file_validation(char const *tal_file, void *arg)
 	if (errno) {
 		error = -pr_errno(errno,
 		    "Could not spawn the file validation thread");
-		goto free_param;
+		goto free_status;
 	}
 
 	thread = malloc(sizeof(struct thread));
 	if (thread == NULL) {
 		close_thread(pid, tal_file);
 		error = -EINVAL;
-		goto free_param;
+		goto free_status;
 	}
 
 	thread->pid = pid;
 	thread->file = strdup(tal_file);
+	thread->exit_status = exit_status;
 	SLIST_INSERT_HEAD(&threads, thread, next);
 
 	return 0;
-free_param:
+free_status:
+	free(exit_status);
 	free(param->tal_file);
 	free(param);
 	return error;
@@ -599,7 +614,7 @@ int
 perform_standalone_validation(struct db_table *table)
 {
 	struct thread *thread;
-	int error;
+	int error, t_error;
 
 	SLIST_INIT(&threads);
 	error = process_file_or_dir(config_get_tal(), TAL_FILE_EXTENSION,
@@ -608,6 +623,7 @@ perform_standalone_validation(struct db_table *table)
 		return error;
 
 	/* Wait for all */
+	t_error = 0;
 	while (!SLIST_EMPTY(&threads)) {
 		thread = threads.slh_first;
 		error = pthread_join(thread->pid, NULL);
@@ -615,9 +631,18 @@ perform_standalone_validation(struct db_table *table)
 			pr_crit("pthread_join() threw %d on the '%s' thread.",
 			    error, thread->file);
 		SLIST_REMOVE_HEAD(&threads, next);
+		if (*thread->exit_status) {
+			t_error = *thread->exit_status;
+			pr_warn("Validation from TAL '%s' yielded error, discarding any other validation results.",
+			    thread->file);
+		}
 		thread_destroy(thread);
 	}
 
+	/* One thread has errors, validation can't keep the resulting table */
+	if (t_error)
+		return t_error;
+
 	return error;
 }