config: Fix crash when a reload fails twice

Have string values stored in char arrays in totem_config
so we don't get into a mess with the pointers.

Also remove vsftype (which hasn't been used since corosync 1)

Use strncpy even though we know the string is fine. Keep covscan happy

Signed-off-by: Christine Caulfield <ccaulfie@redhat.com>
Reviewed-by: Jan Friesse <jfriesse@redhat.com>

exec/cfg.c

@@ -593,7 +593,6 @@ static void remove_ro_entries(icmap_map_t temp_map)
 	delete_and_notify_if_changed(temp_map, "totem.interface.broadcast");
 	delete_and_notify_if_changed(temp_map, "totem.interface.mcastport");
 	delete_and_notify_if_changed(temp_map, "totem.interface.ttl");
-	delete_and_notify_if_changed(temp_map, "totem.vsftype");
 	delete_and_notify_if_changed(temp_map, "totem.transport");
 	delete_and_notify_if_changed(temp_map, "totem.cluster_name");
 	delete_and_notify_if_changed(temp_map, "quorum.provider");

exec/totemconfig.c

@@ -139,7 +139,7 @@ static void *totem_get_param_by_name(struct totem_config *totem_config, const ch
 	if (strcmp(param_name, "totem.knet_compression_level") == 0)
 		return &totem_config->knet_compression_level;
 	if (strcmp(param_name, "totem.knet_compression_model") == 0)
-		return &totem_config->knet_compression_model;
+		return totem_config->knet_compression_model;
 	if (strcmp(param_name, "totem.block_unlisted_ips") == 0)
 		return &totem_config->block_unlisted_ips;
 
@@ -210,18 +210,24 @@ static void totem_volatile_config_set_string_value (struct totem_config *totem_c
 	const char *key_name, const char *deleted_key, const char *default_value)
 {
 	char runtime_key_name[ICMAP_KEYNAME_MAXLEN];
-	void **config_value = NULL; /* compiler placation */
-	void *old_config_ptr;
+	int res;
+	char *new_config_value;
+	const void *config_value;
 
 	config_value = totem_get_param_by_name(totem_config, key_name);
-	old_config_ptr = *config_value;
-	if (icmap_get_string(key_name, (char **)config_value) != CS_OK ||
+
+	res = icmap_get_string_r(map, key_name, (char **)&new_config_value);
+	if (res != CS_OK ||
 	    (deleted_key != NULL && strcmp(deleted_key, key_name) == 0)) {
 
-		/* Need to strdup() here so that the free() below works for a default and a configured value */
-		*config_value = strdup(default_value);
+		/* Slightly pointless use of strncpy but it keeps coverity happy */
+		strncpy((char *)config_value, default_value, CONFIG_STRING_LEN_MAX);
+	} else {
+		strncpy((char *)config_value, new_config_value, CONFIG_STRING_LEN_MAX);
+	}
+	if (res == CS_OK) {
+		free(new_config_value);
 	}
-	free(old_config_ptr);
 
 	/*
 	 * Store totem_config value to cmap runtime section
@@ -236,7 +242,7 @@ static void totem_volatile_config_set_string_value (struct totem_config *totem_c
 	strcpy(runtime_key_name, "runtime.config.");
 	strcat(runtime_key_name, key_name);
 
-	(void)icmap_set_string_r(map, runtime_key_name, (char *)*config_value);
+	(void)icmap_set_string_r(map, runtime_key_name, (char *)config_value);
 }
 
 /*
@@ -601,13 +607,9 @@ static int totem_get_crypto(struct totem_config *totem_config, const char **erro
 		return -1;
 	}
 
-	free(totem_config->crypto_cipher_type);
-	free(totem_config->crypto_hash_type);
-	free(totem_config->crypto_model);
-
-	totem_config->crypto_cipher_type = strdup(tmp_cipher);
-	totem_config->crypto_hash_type = strdup(tmp_hash);
-	totem_config->crypto_model = strdup(tmp_model);
+	strncpy(totem_config->crypto_cipher_type, tmp_cipher, CONFIG_STRING_LEN_MAX);
+	strncpy(totem_config->crypto_hash_type, tmp_hash, CONFIG_STRING_LEN_MAX);
+	strncpy(totem_config->crypto_model, tmp_model, CONFIG_STRING_LEN_MAX);
 
 	return 0;
 }

exec/totemknet.c

@@ -1477,17 +1477,15 @@ int totemknet_reconfigure (
 	struct knet_handle_compress_cfg compress_cfg;
 	int res = 0;
 
-	if (totem_config->knet_compression_model) {
-		assert(strlen(totem_config->knet_compression_model) < sizeof(compress_cfg.compress_model));
-		strcpy(compress_cfg.compress_model, totem_config->knet_compression_model);
+	assert(strlen(totem_config->knet_compression_model) < sizeof(compress_cfg.compress_model));
+	strcpy(compress_cfg.compress_model, totem_config->knet_compression_model);
 
-		compress_cfg.compress_threshold = totem_config->knet_compression_threshold;
-		compress_cfg.compress_level = totem_config->knet_compression_level;
+	compress_cfg.compress_threshold = totem_config->knet_compression_threshold;
+	compress_cfg.compress_level = totem_config->knet_compression_level;
 
-		res = knet_handle_compress(instance->knet_handle, &compress_cfg);
-		if (res) {
-			KNET_LOGSYS_PERROR(errno, LOGSYS_LEVEL_ERROR, "knet_handle_compress failed");
-		}
+	res = knet_handle_compress(instance->knet_handle, &compress_cfg);
+	if (res) {
+		KNET_LOGSYS_PERROR(errno, LOGSYS_LEVEL_ERROR, "knet_handle_compress failed");
 	}
 
 #ifdef HAVE_LIBNOZZLE

include/corosync/totem/totem.h

@@ -51,6 +51,7 @@
 
 #define FRAME_SIZE_MAX		KNET_MAX_PACKET_SIZE
 
+#define CONFIG_STRING_LEN_MAX   128
 /*
  * Estimation of required buffer size for totemudp and totemudpu - it should be at least
  *   sizeof(memb_join) + PROCESSOR_MAX * 2 * sizeof(srp_addr))
@@ -212,17 +213,15 @@ struct totem_config {
 
 	unsigned int max_messages;
 
-	const char *vsf_type;
-
 	unsigned int broadcast_use;
 
-	char *crypto_model;
+	char crypto_model[CONFIG_STRING_LEN_MAX];
 
-	char *crypto_cipher_type;
+	char crypto_cipher_type[CONFIG_STRING_LEN_MAX];
 
-	char *crypto_hash_type;
+	char crypto_hash_type[CONFIG_STRING_LEN_MAX];
 
-	char *knet_compression_model;
+	char knet_compression_model[CONFIG_STRING_LEN_MAX];
 
 	uint32_t knet_compression_threshold;