[PATCH 2/2] Don't call sync_* funcs for unloaded services

[Jan Friesse <jfriesse@xxxxxxxxxx>]

When service is unloaded, sync shouldn't call sync_init|process|activate
and abort functions. It happens very rare, but in process of unloading
all services, totem can recreate membership and bad things can happen
(service is unloaded, so there may be access to already freed memory,
 ...)

Solution is to fetch services sync handlers in every time when we are
building service list instead of using precreated one.

Signed-off-by: Jan Friesse <jfriesse@xxxxxxxxxx>
---
 exec/sync.c |   64 +++++++++++++++++++++++++++++-----------------------------
 1 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/exec/sync.c b/exec/sync.c
index f9f8cb4..b09ea53 100644
--- a/exec/sync.c
+++ b/exec/sync.c
@@ -149,10 +149,6 @@ static int my_service_list_entries = 0;
 
 static const struct memb_ring_id sync_ring_id;
 
-static struct service_entry my_initial_service_list[SERVICES_COUNT_MAX];
-
-static int my_initial_service_list_entries;
-
 static void (*sync_synchronization_completed) (void);
 
 static void sync_deliver_fn (
@@ -172,6 +168,10 @@ static struct totempg_group sync_group = {
 
 static void *sync_group_handle;
 
+int (*my_sync_callbacks_retrieve) (
+		int service_id,
+                struct sync_callbacks *callbacks);
+
 int sync_init (
         int (*sync_callbacks_retrieve) (
                 int service_id,
@@ -179,8 +179,6 @@ int sync_init (
         void (*synchronization_completed) (void))
 {
 	unsigned int res;
-	int i;
-	struct sync_callbacks sync_callbacks;
 
 	res = totempg_groups_initialize (
 		&sync_group_handle,
@@ -202,25 +200,8 @@ int sync_init (
 	}
 
 	sync_synchronization_completed = synchronization_completed;
-	for (i = 0; i < SERVICES_COUNT_MAX; i++) {
-		res = sync_callbacks_retrieve (i, &sync_callbacks);
-		if (res == -1) {
-			continue;
-		}
-		if (sync_callbacks.sync_init == NULL) {
-			continue;
-		}
-		my_initial_service_list[my_initial_service_list_entries].state =
-			INIT;
-		my_initial_service_list[my_initial_service_list_entries].service_id = i;
-		strcpy (my_initial_service_list[my_initial_service_list_entries].name,
-			sync_callbacks.name);
-		my_initial_service_list[my_initial_service_list_entries].sync_init = sync_callbacks.sync_init;
-		my_initial_service_list[my_initial_service_list_entries].sync_process = sync_callbacks.sync_process;
-		my_initial_service_list[my_initial_service_list_entries].sync_abort = sync_callbacks.sync_abort;
-		my_initial_service_list[my_initial_service_list_entries].sync_activate = sync_callbacks.sync_activate;
-		my_initial_service_list_entries += 1;
-	}
+	my_sync_callbacks_retrieve = sync_callbacks_retrieve;
+
 	return (0);
 }
 
@@ -346,7 +327,7 @@ static void sync_service_build_handler (unsigned int nodeid, const void *msg)
 			my_service_list[my_service_list_entries].service_id =
 				req_exec_service_build_message->service_list[i];
 			sprintf (my_service_list[my_service_list_entries].name,
-				"External Service (id = %d)\n",
+				"Uknown External Service (id = %d)\n",
 				req_exec_service_build_message->service_list[i]);
 			my_service_list[my_service_list_entries].sync_init =
 				dummy_sync_init;
@@ -491,6 +472,8 @@ static void sync_servicelist_build_enter (
 {
 	struct req_exec_service_build_message service_build;
 	int i;
+	int res;
+	struct sync_callbacks sync_callbacks;
 
 	my_state = SYNC_SERVICELIST_BUILD;
 	for (i = 0; i < member_list_entries; i++) {
@@ -505,14 +488,31 @@ static void sync_servicelist_build_enter (
 
 	my_processing_idx = 0;
 
-	memcpy (my_service_list, my_initial_service_list,
-		sizeof (struct service_entry) *
-			my_initial_service_list_entries);
-	my_service_list_entries = my_initial_service_list_entries;
+	memset(my_service_list, 0, sizeof (struct service_entry) * SERVICES_COUNT_MAX);
+	my_service_list_entries = 0;
+
+	for (i = 0; i < SERVICES_COUNT_MAX; i++) {
+		res = my_sync_callbacks_retrieve (i, &sync_callbacks);
+		if (res == -1) {
+			continue;
+		}
+		if (sync_callbacks.sync_init == NULL) {
+			continue;
+		}
+		my_service_list[my_service_list_entries].state = INIT;
+		my_service_list[my_service_list_entries].service_id = i;
+		strcpy (my_service_list[my_service_list_entries].name,
+			sync_callbacks.name);
+		my_service_list[my_service_list_entries].sync_init = sync_callbacks.sync_init;
+		my_service_list[my_service_list_entries].sync_process = sync_callbacks.sync_process;
+		my_service_list[my_service_list_entries].sync_abort = sync_callbacks.sync_abort;
+		my_service_list[my_service_list_entries].sync_activate = sync_callbacks.sync_activate;
+		my_service_list_entries += 1;
+	}
 
-	for (i = 0; i < my_initial_service_list[i].service_id; i++) {
+	for (i = 0; i < my_service_list[i].service_id; i++) {
 		service_build.service_list[i] =
-			my_initial_service_list[i].service_id;
+			my_service_list[i].service_id;
 	}
 	service_build.service_list_entries = i;
 
-- 
1.7.1

_______________________________________________
discuss mailing list
discuss@xxxxxxxxxxxx
http://lists.corosync.org/mailman/listinfo/discuss