View Issue Details

IDProjectCategoryView StatusLast Update
0000596Pgpool-IIBugpublic2020-05-21 11:29
Reportergregn123Assigned Tot-ishii 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionopen 
Product Version4.1.0 
Target Version4.1.2Fixed in Version4.1.2 
Summary0000596: Problems in watchdog processing json data
DescriptionIn the watchdog source code (src/watchdog/wd_json_data.c), there are some instances of bad handling of values read from json data.
For example:
1) The boolean pool configuration settings "load_balance_mode" and "master_slave_mode" are read using json_get_int_value_for_key(), resulting in 4-bytes being written into their location within the POOL_CONFIG, yet (being bool) they are only 1-byte long. This corrupts the values of the structure members following them.
2) Similarly, when parsing node function json data, "Flags" is read using json_get_int_value_for_key(), resulting in 4-bytes being written into an "unsigned char flags" variable on the stack, overwriting 3-bytes of stack memory following it. On a big-endian system (e.g. Solaris-sparc or Linux for IBM Z), this causes regression test "013.watchdog_failover_require_consensus" to fail, since 0 is written into Flags, rather than the intended value which is in the least significant byte of the int value written.

I have attached a patch to correct these issues (gpool2_watchdog_json_handling_issues.patch).
Please review.

Steps To ReproduceRun the pgpool2 regression tests on a big-endian system (e.g. Solaris-sparc or Linux on IBM Z), and test 013 will fail.
(To build pgpool2 properly on these systems, I found it necessary to specify -DWORDS_BIGENDIAN in CFLAGS when running the configure script, as it's otherwise not accounting for it. My other fix for SCRAM authentication may also be needed - see issue 0000595).
The corruptions will occur on all systems, but these may or may not cause obvious issues.
Tagswatchdog

Activities

gregn123

2020-03-12 14:17

reporter  

pgpool2_watchdog_json_handling_issues.patch (3,237 bytes)
diff --git a/src/watchdog/wd_json_data.c b/src/watchdog/wd_json_data.c
index 7f3cb66..1923131 100644
--- a/src/watchdog/wd_json_data.c
+++ b/src/watchdog/wd_json_data.c
@@ -66,7 +66,7 @@ get_pool_config_from_json(char *json_data, int data_len)
 		goto ERROR_EXIT;
 	if (json_get_bool_value_for_key(root, "enable_pool_hba", &config->enable_pool_hba))
 		goto ERROR_EXIT;
-	if (json_get_int_value_for_key(root, "load_balance_mode", (int *) &config->load_balance_mode))
+	if (json_get_bool_value_for_key(root, "load_balance_mode", &config->load_balance_mode))
 		goto ERROR_EXIT;
 	if (json_get_bool_value_for_key(root, "replication_stop_on_mismatch", &config->replication_stop_on_mismatch))
 		goto ERROR_EXIT;
@@ -76,7 +76,7 @@ get_pool_config_from_json(char *json_data, int data_len)
 		goto ERROR_EXIT;
 	if (json_get_bool_value_for_key(root, "replicate_select", &config->replicate_select))
 		goto ERROR_EXIT;
-	if (json_get_int_value_for_key(root, "master_slave_mode", (int *) &config->master_slave_mode))
+	if (json_get_bool_value_for_key(root, "master_slave_mode", &config->master_slave_mode))
 		goto ERROR_EXIT;
 	if (json_get_bool_value_for_key(root, "connection_cache", &config->connection_cache))
 		goto ERROR_EXIT;
@@ -187,12 +187,12 @@ get_pool_config_json(void)
 	jw_put_int(jNode, "max_pool", pool_config->max_pool);
 	jw_put_bool(jNode, "replication_mode", pool_config->replication_mode);
 	jw_put_bool(jNode, "enable_pool_hba", pool_config->enable_pool_hba);
-	jw_put_int(jNode, "load_balance_mode", pool_config->load_balance_mode);
+	jw_put_bool(jNode, "load_balance_mode", pool_config->load_balance_mode);
 	jw_put_bool(jNode, "allow_clear_text_frontend_auth", pool_config->allow_clear_text_frontend_auth);
 	jw_put_bool(jNode, "replication_stop_on_mismatch", pool_config->replication_stop_on_mismatch);
 	jw_put_bool(jNode, "failover_if_affected_tuples_mismatch", pool_config->failover_if_affected_tuples_mismatch);
 	jw_put_bool(jNode, "replicate_select", pool_config->replicate_select);
-	jw_put_int(jNode, "master_slave_mode", pool_config->master_slave_mode);
+	jw_put_bool(jNode, "master_slave_mode", pool_config->master_slave_mode);
 	jw_put_bool(jNode, "connection_cache", pool_config->connection_cache);
 	jw_put_int(jNode, "health_check_timeout", pool_config->health_check_timeout);
 	jw_put_int(jNode, "health_check_period", pool_config->health_check_period);
@@ -769,7 +769,8 @@ parse_wd_node_function_json(char *json_data, int data_len, char **func_name, int
 	char	   *ptr;
 	int			node_count = 0;
 	int			i;
-
+	int			tmpflags = 0;
+
 	*node_id_set = NULL;
 	*func_name = NULL;
 	*count = 0;
@@ -796,12 +797,17 @@ parse_wd_node_function_json(char *json_data, int data_len, char **func_name, int
 	}
 	*func_name = pstrdup(ptr);
 	/* If it is a node function ? */
-	if (json_get_int_value_for_key(root, "Flags", (int *)flags))
+	if (json_get_int_value_for_key(root, "Flags", &tmpflags))
 	{
 		/* node count not found, But we don't care much about this */
 		*flags = 0;
 		/* it may be from the old version */
 	}
+	else
+	{
+		*flags = (unsigned char)tmpflags;
+	}
+
 	if (json_get_int_value_for_key(root, "NodeCount", &node_count))
 	{
 		/* node count not found, But we don't care much about this */

t-ishii

2020-03-12 20:56

developer   ~0003264

Thank you! Patch looks good. I am going to commit it. (please let me know your name so that I can credit your name in a commit message).

gregn123

2020-03-13 09:23

reporter   ~0003265

Greg Nancarrow (Fujitsu Australia)

t-ishii

2020-03-13 10:50

developer   ~0003268

Patch pushed. Thanks!

Issue History

Date Modified Username Field Change
2020-03-12 14:17 gregn123 New Issue
2020-03-12 14:17 gregn123 File Added: pgpool2_watchdog_json_handling_issues.patch
2020-03-12 14:17 gregn123 Tag Attached: watchdog
2020-03-12 16:32 t-ishii Assigned To => t-ishii
2020-03-12 16:32 t-ishii Status new => assigned
2020-03-12 20:56 t-ishii Note Added: 0003264
2020-03-13 08:51 t-ishii Status assigned => feedback
2020-03-13 08:51 t-ishii Description Updated View Revisions
2020-03-13 08:51 t-ishii Steps to Reproduce Updated View Revisions
2020-03-13 09:23 gregn123 Note Added: 0003265
2020-03-13 09:23 gregn123 Status feedback => assigned
2020-03-13 10:50 t-ishii Note Added: 0003268
2020-03-13 10:51 t-ishii Target Version => 4.1.2
2020-03-13 17:27 t-ishii Status assigned => feedback
2020-05-21 11:29 administrator Status feedback => closed
2020-05-21 11:29 administrator Fixed in Version => 4.1.2