Просмотр исходного кода

Don't call sync_* funcs for unloaded services

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@redhat.com>
Reviewed-by: Steven Dake <sdake@redhat.com>
Jan Friesse 13 лет назад
Родитель
Сommit
fed7fc23e1
1 измененных файлов с 32 добавлено и 32 удалено
  1. 32 32
      exec/sync.c

+ 32 - 32
exec/sync.c

@@ -149,10 +149,6 @@ static int my_service_list_entries = 0;
 
 
 static const struct memb_ring_id sync_ring_id;
 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_synchronization_completed) (void);
 
 
 static void sync_deliver_fn (
 static void sync_deliver_fn (
@@ -172,6 +168,10 @@ static struct totempg_group sync_group = {
 
 
 static void *sync_group_handle;
 static void *sync_group_handle;
 
 
+int (*my_sync_callbacks_retrieve) (
+		int service_id,
+                struct sync_callbacks *callbacks);
+
 int sync_init (
 int sync_init (
         int (*sync_callbacks_retrieve) (
         int (*sync_callbacks_retrieve) (
                 int service_id,
                 int service_id,
@@ -179,8 +179,6 @@ int sync_init (
         void (*synchronization_completed) (void))
         void (*synchronization_completed) (void))
 {
 {
 	unsigned int res;
 	unsigned int res;
-	int i;
-	struct sync_callbacks sync_callbacks;
 
 
 	res = totempg_groups_initialize (
 	res = totempg_groups_initialize (
 		&sync_group_handle,
 		&sync_group_handle,
@@ -202,25 +200,8 @@ int sync_init (
 	}
 	}
 
 
 	sync_synchronization_completed = synchronization_completed;
 	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);
 	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 =
 			my_service_list[my_service_list_entries].service_id =
 				req_exec_service_build_message->service_list[i];
 				req_exec_service_build_message->service_list[i];
 			sprintf (my_service_list[my_service_list_entries].name,
 			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]);
 				req_exec_service_build_message->service_list[i]);
 			my_service_list[my_service_list_entries].sync_init =
 			my_service_list[my_service_list_entries].sync_init =
 				dummy_sync_init;
 				dummy_sync_init;
@@ -491,6 +472,8 @@ static void sync_servicelist_build_enter (
 {
 {
 	struct req_exec_service_build_message service_build;
 	struct req_exec_service_build_message service_build;
 	int i;
 	int i;
+	int res;
+	struct sync_callbacks sync_callbacks;
 
 
 	my_state = SYNC_SERVICELIST_BUILD;
 	my_state = SYNC_SERVICELIST_BUILD;
 	for (i = 0; i < member_list_entries; i++) {
 	for (i = 0; i < member_list_entries; i++) {
@@ -505,14 +488,31 @@ static void sync_servicelist_build_enter (
 
 
 	my_processing_idx = 0;
 	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] =
 		service_build.service_list[i] =
-			my_initial_service_list[i].service_id;
+			my_service_list[i].service_id;
 	}
 	}
 	service_build.service_list_entries = i;
 	service_build.service_list_entries = i;