Attachment 629240 Details for Bug 863940 – 2012-10-18-0001-Don-t-call-sync_-funcs-for-unloaded-services

[patch] 2012-10-18-0001-Don-t-call-sync_-funcs-for-unloaded-services

2012-10-18-0001-Don-t-call-sync_-funcs-for-unloaded-services.patch (text/plain), 4.82 KB, created by Jan Friesse on 2012-10-18 08:39:41 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jan Friesse

Created: 2012-10-18 08:39:41 UTC

Size: 4.82 KB

patch

obsolete

>From 87284e53febb538bbd6e31648accb22b76f46937 Mon Sep 17 00:00:00 2001
>From: Jan Friesse <jfriesse@redhat.com>
>Date: Mon, 15 Oct 2012 16:43:27 +0200
>Subject: [PATCH 1/3] 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>
>(backported from commit fed7fc23e14e098dbb52842a4c79879a376f6ded)
>---
> exec/syncv2.c |   65 +++++++++++++++++++++++++++++----------------------------
> 1 files changed, 33 insertions(+), 32 deletions(-)
>
>diff --git a/exec/syncv2.c b/exec/syncv2.c
>index 344d7d0..d39e057 100644
>--- a/exec/syncv2.c
>+++ b/exec/syncv2.c
>@@ -146,10 +146,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[PROCESSOR_COUNT_MAX];
>-
>-static int my_initial_service_list_entries;
>-
> static void (*sync_synchronization_completed) (void);
> 
> static void sync_deliver_fn (
>@@ -169,6 +165,10 @@ static struct totempg_group sync_group = {
> 
> static hdb_handle_t sync_group_handle;
> 
>+int (*my_sync_callbacks_retrieve) (
>+		int service_id,
>+                struct sync_callbacks *callbacks);
>+
> int sync_v2_init (
>         int (*sync_callbacks_retrieve) (
>                 int service_id,
>@@ -176,8 +176,6 @@ int sync_v2_init (
>         void (*synchronization_completed) (void))
> {
> 	unsigned int res;
>-	int i;
>-	struct sync_callbacks sync_callbacks;
> 
> 	res = totempg_groups_initialize (
> 		&sync_group_handle,
>@@ -199,26 +197,8 @@ int sync_v2_init (
> 	}
> 
> 	sync_synchronization_completed = synchronization_completed;
>-	for (i = 0; i < 64; i++) {
>-		res = sync_callbacks_retrieve (i, &sync_callbacks);
>-		if (res == -1) {
>-			continue;
>-		}
>-		if (sync_callbacks.sync_init_api.sync_init_v1 == 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].api_version = sync_callbacks.api_version;
>-		my_initial_service_list[my_initial_service_list_entries].sync_init_api = sync_callbacks.sync_init_api;
>-		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);
> }
> 
>@@ -489,6 +469,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++) {
>@@ -503,14 +485,33 @@ 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) * 128);
>+	my_service_list_entries = 0;
>+
>+	for (i = 0; i < 64; i++) {
>+		res = my_sync_callbacks_retrieve (i, &sync_callbacks);
>+		if (res == -1) {
>+			continue;
>+		}
>+		if (sync_callbacks.sync_init_api.sync_init_v1 == 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].api_version = sync_callbacks.api_version;
>+		my_service_list[my_service_list_entries].sync_init_api = sync_callbacks.sync_init_api;
>+		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
>

Actions: View | Diff

Attachments on bug 863940: 629238 | 629239 | 629240