Merge pull request #2449 from pqarmitage/updates

vrrp_ipsets and vrrp_iptables fixes and new sanitize configure options

INSTALL

@@ -44,6 +44,9 @@ For setting VMAC interfaces unmanaged by NetworkManager. This is not needed for
    NetworkManager >= 1.18, and possibly some earlier versions, but is needed for
    NetworkManager 1.0.6.
 	NetworkManager-libnm-devel
+For building with sanitizers
+	libasan libubsan libhwasan liblsan (gcc)
+	compiler-rt (clang)
 
 For building the documentation, the following packages need to be installed:
 	Fedora: python-sphinx (will pull in: python2-sphinx_rtd_theme)

configure.ac

@@ -265,6 +265,28 @@ AC_ARG_ENABLE(perf,
   [AS_HELP_STRING([--enable-perf], [compile with perf performance data recording support for vrrp process])])
 AC_ARG_ENABLE(sanitize-address,
   [AS_HELP_STRING([--enable-sanitize-address], [compile with sanitize=address (ASAN) support])])
+AC_ARG_ENABLE(sanitize-address-options,
+  [AS_HELP_STRING([--enable-sanitize-address-options], [compile with sanitize=address (ASAN) default options])])
+AC_ARG_ENABLE(sanitize-hwaddress,
+  [AS_HELP_STRING([--enable-sanitize-hwaddress], [compile with sanitize=hwaddress (HWASAN) support])])
+AC_ARG_ENABLE(sanitize-hwaddress-options,
+  [AS_HELP_STRING([--enable-sanitize-hwaddress-options], [compile with sanitize=hwaddress (HWASAN) default options])])
+AC_ARG_ENABLE(sanitize-undefined,
+  [AS_HELP_STRING([--enable-sanitize-undefined], [compile with sanitize=undefined (UBSAN) support])])
+AC_ARG_ENABLE(sanitize-undefined-options,
+  [AS_HELP_STRING([--enable-sanitize-undefined-options], [compile with sanitize=undefined (UBSAN) default options])])
+AC_ARG_ENABLE(sanitize-memory,
+  [AS_HELP_STRING([--enable-sanitize-memory], [compile with sanitize=memory (MSAN) support])])
+AC_ARG_ENABLE(sanitize-memory-options,
+  [AS_HELP_STRING([--enable-sanitize-memory-options], [compile with sanitize=memory (MSAN) default options])])
+AC_ARG_ENABLE(sanitize-leak,
+  [AS_HELP_STRING([--enable-sanitize-leak], [compile with sanitize=leak (LSAN) support])])
+AC_ARG_ENABLE(sanitize-leak-options,
+  [AS_HELP_STRING([--enable-sanitize-leak-options], [compile with sanitize=leak (LSAN) default options])])
+AC_ARG_ENABLE(sanitize-scudo,
+  [AS_HELP_STRING([--enable-sanitize-scudo], [compile with sanitize=scudo support])])
+AC_ARG_ENABLE(sanitize-scudo-options,
+  [AS_HELP_STRING([--enable-sanitize-scudo-options], [compile with sanitize=scudo default options])])
 AC_ARG_ENABLE(log-file,
   [AS_HELP_STRING([--enable-log-file], [enable logging to file (-g)])])
 AC_ARG_ENABLE(dump-threads,
@@ -333,6 +355,13 @@ AC_ARG_WITH([systemdsystemunitdir],
   [with_systemdsystemunitdir=auto])
 AC_ARG_WITH([dbus-data-dir],
   [AS_HELP_STRING([--with-dbus-data-dir=DIR], [Directory for Dbus interface files])])
+AC_ARG_ENABLE([cflags],
+  [AS_HELP_STRING([--enable-cflags=flags], [additional CFLAGS])])
+AC_ARG_ENABLE([cppflags],
+  [AS_HELP_STRING([--enable-cppflags=flags], [additional CPPFLAGS])])
+AC_ARG_ENABLE([ldflags],
+  [AS_HELP_STRING([--enable-ldflags=flags], [additional LDFLAGS])])
+
 
 # Set the kernel headers path
 if test -n "$kernel_src_path"; then
@@ -412,6 +441,11 @@ KA_CFLAGS="-g $CFLAGS"
 KA_LDFLAGS=$LDFLAGS
 KA_LIBS=$LDLIBS
 
+# Set any additional compiler flags
+AS_IF([test -n "$enable_cflags" -a "$enable_cflags" != no], [add_to_var([KA_CFLAGS], ["$enable_cflags"])])
+AS_IF([test -n "$enable_cppflags" -a "$enable_cppflags" != no], [add_to_var([KA_CPPFLAGS], ["$enable_cppflags"])])
+AS_IF([test -n "$enable_ldflags" -a "$enable_ldflags" != no], [add_to_var([KA_LDFLAGS], ["$enable_ldflags"])])
+
 # For reporting GCC bugs, uncomment the next two lines
 #KA_CPPFLAGS="$KA_CPPFLAGS -v -save-temps"
 #KA_CFLAGS="$KA_CFLAGS -v -save-temps"
@@ -2850,16 +2884,92 @@ else
   ENABLE_PERF=No
 fi
 
+# consider -fsanitize-recover=all or -fsanitize-recover=address -fsanitize=pointer-compare -fsanitize=pointer-subtract (pointer-subtract needs option detect_invalid_pointer_pairs=2)
+SANITIZERS=
 dnl ----[ sanitize=address testing or not? ]----
-if test "${enable_sanitize_address}" = yes; then
-#  AC_DEFINE([_WITH_SANITIZE_ADDRESS_], [ 1 ], [Define to 1 to build with sanitize=address support])
-  ENABLE_SANITIZE_ADDRESS=Yes
-  add_config_opt([SANITIZE_ADDRESS])
-  add_to_var([KA_CFLAGS], [-fsanitize=address -g])
+AS_IF([test "${enable_sanitize_address}" = yes],
+  [
+    AC_DEFINE([_WITH_SANITIZE_ADDRESS_], [ 1 ], [Define to 1 to build with sanitize=address support])
+    ENABLE_SANITIZE_ADDRESS=Yes
+    add_config_opt([SANITIZE_ADDRESS])
+    SANITIZERS="$SANITIZERS,address"
+    AS_IF([test "${enable_sanitize_address_options}" != no -a -n "${enable_sanitize_address_options}"],
+      [AC_DEFINE_UNQUOTED([_ASAN_DEFAULT_OPTIONS_], ["${enable_sanitize_address_options}"], [default options for ASAN])])
+  ],[
+    ENABLE_SANITIZE_ADDRESS=No
+  ]
+)
+
+dnl ----[ sanitize=hwaddress testing or not? ]----
+if test "${enable_sanitize_hwaddress}" = yes; then
+  AC_DEFINE([_WITH_SANITIZE_HWADDRESS_], [ 1 ], [Define to 1 to build with sanitize=hwaddress support])
+  ENABLE_SANITIZE_HWADDRESS=Yes
+  add_config_opt([SANITIZE_HWADDRESS])
+  SANITIZERS="$SANITIZERS,hwaddress"
+    AS_IF([test "${enable_sanitize_hwaddress_options}" != no -a -n "${enable_sanitize_hwaddress_options}"],
+      [AC_DEFINE_UNQUOTED([_HWASAN_DEFAULT_OPTIONS_], ["${enable_sanitize_hwaddress_options}"], [default options for HWASAN])])
 else
-  ENABLE_SANITIZE_ADDRESS=No
+  ENABLE_SANITIZE_HWADDRESS=No
+fi
+
+dnl ----[ sanitize=undefined testing or not? ]----
+echo enable_Sanitize_undefined = $enable_sanitize_undefined
+if test "${enable_sanitize_undefined}" = yes; then
+  AC_DEFINE([_WITH_SANITIZE_UNDEFINED_], [ 1 ], [Define to 1 to build with sanitize=undefined support])
+  ENABLE_SANITIZE_UNDEFINED=Yes
+  add_config_opt([SANITIZE_UNDEFINED])
+  add_to_var([KA_CFLAGS], ["--param=max-inline-insns-single=100000"])
+  add_to_var([KA_CFLAGS], ["--param=large-function-growth=500"])
+  SANITIZERS="$SANITIZERS,undefined"
+    AS_IF([test "${enable_sanitize_undefined_options}" != no -a -n "${enable_sanitize_undefined_options}"],
+      [AC_DEFINE_UNQUOTED([_UBSAN_DEFAULT_OPTIONS_], ["${enable_sanitize_undefined_options}"], [default options for UBSAN])])
+else
+  ENABLE_SANITIZE_UNDEFINED=No
+fi
+
+dnl ----[ sanitize=memory testing or not? ]----
+if test "${enable_sanitize_memory}" = yes; then
+  AC_DEFINE([_WITH_SANITIZE_MEMORY_], [ 1 ], [Define to 1 to build with sanitize=memory support])
+  ENABLE_SANITIZE_MEMORY=Yes
+  add_config_opt([SANITIZE_MEMORY])
+  SANITIZERS="$SANITIZERS,memory"
+    AS_IF([test "${enable_sanitize_memory_options}" != no -a -n "${enable_sanitize_memory_options}"],
+      [AC_DEFINE_UNQUOTED([_MSAN_DEFAULT_OPTIONS_], ["${enable_sanitize_memory_options}"], [default options for MSAN])])
+else
+  ENABLE_SANITIZE_MEMORY=No
 fi
 
+dnl ----[ sanitize=leak testing or not? ]----
+if test "${enable_sanitize_leak}" = yes; then
+  AC_DEFINE([_WITH_SANITIZE_LEAK_], [ 1 ], [Define to 1 to build with sanitize=leak support])
+  ENABLE_SANITIZE_LEAK=Yes
+  add_config_opt([SANITIZE_LEAK])
+  SANITIZERS="$SANITIZERS,leak"
+    AS_IF([test "${enable_sanitize_leak_options}" != no -a -n "${enable_sanitize_leak_options}"],
+      [AC_DEFINE_UNQUOTED([_LSAN_DEFAULT_OPTIONS_], ["${enable_sanitize_leak_options}"], [default options for LSAN])])
+else
+  ENABLE_SANITIZE_LEAK=No
+fi
+
+dnl ----[ sanitize=scudo testing or not? ]----
+if test "${enable_sanitize_scudo}" = yes; then
+  AC_DEFINE([_WITH_SANITIZE_SCUDO_], [ 1 ], [Define to 1 to build with sanitize=scudo support])
+  ENABLE_SANITIZE_SCUDO=Yes
+  add_config_opt([SANITIZE_SCUDO])
+  SANITIZERS="$SANITIZERS,scudo"
+    AS_IF([test "${enable_sanitize_scudo_options}" != no -a -n "${enable_sanitize_scudo_options}"],
+      [AC_DEFINE_UNQUOTED([_SCUDO_DEFAULT_OPTIONS_], ["${enable_sanitize_scudo_options}"], [default options for SCUDO])])
+else
+  ENABLE_SANITIZE_SCUDO=No
+fi
+AS_IF([test -n "$SANITIZERS"],
+  [
+    add_to_var([KA_CFLAGS], ["-fsanitize=${SANITIZERS:1}"])	# Skip leading ,
+    add_to_var([KA_CFLAGS], ["-g"])
+    AC_DEFINE([_WITH_SANITIZER_], [ 1 ], [Define to 1 if any sanitizer is enabled])
+  ])
+AM_CONDITIONAL([WITH_SANITIZER], [test -n "$SANITIZERS"])
+
 if test "${enable_log_file}" = yes; then
   AC_DEFINE([ENABLE_LOG_TO_FILE], [ 1 ], [Define if enabling logging to files])
   ENABLE_LOG_FILE_APPEND=Yes
@@ -3284,7 +3394,22 @@ if test ${ENABLE_PERF} = Yes; then
   echo "Perf support             : Yes"
 fi
 if test ${ENABLE_SANITIZE_ADDRESS} = Yes; then
-  echo "sanitize=address testing : Yes"
+  echo "sanitize=address         : Yes"
+fi
+if test ${ENABLE_SANITIZE_HWADDRESS} = Yes; then
+  echo "sanitize=hwaddress       : Yes"
+fi
+if test ${ENABLE_SANITIZE_UNDEFINED} = Yes; then
+  echo "sanitize=undefined       : Yes"
+fi
+if test ${ENABLE_SANITIZE_MEMORY} = Yes; then
+  echo "sanitize=memory          : Yes"
+fi
+if test ${ENABLE_SANITIZE_LEAK} = Yes; then
+  echo "sanitize=leak            : Yes"
+fi
+if test ${ENABLE_SANITIZE_SCUDO} = Yes; then
+  echo "sanitize=scudo           : Yes"
 fi
 if test ${MEM_CHECK} = Yes; then
   echo "Memory alloc check       : Yes"

keepalived/check/check_parser.c

@@ -925,13 +925,16 @@ vs_weight_handler(const vector_t *strvec)
 	current_vs->weight = weight;
 }
 
+#ifdef _WITH_SANITIZE_UNDEFINED_
+__attribute__((no_sanitize("null")))
+#endif
 void
 init_check_keywords(bool active)
 {
 	vpp_t check_ptr;
 
 	/* SSL mapping */
-	install_keyword_root("SSL", &ssl_handler, active, VPP &check_data->ssl);
+	install_keyword_root("SSL", &ssl_handler, active, VPP &check_data->ssl);	// Causes sanitizer error: member access within null pointer of type 'struct check_data_t'
 	install_keyword("password", &sslpass_handler);
 	install_keyword("ca", &sslca_handler);
 	install_keyword("certificate", &sslcert_handler);

keepalived/core/Makefile.am

@@ -45,4 +45,9 @@ if !ONE_PROCESS_DEBUG
   EXTRA_libcore_a_SOURCES += reload_monitor.c config_notify.c
 endif
 
+if WITH_SANITIZER
+  libcore_a_LIBADD	+= sanitizer.o
+  EXTRA_libcore_a_SOURCES += sanitizer.c
+endif
+
 MAINTAINERCLEANFILES	= @MAINTAINERCLEANFILES@

keepalived/core/global_parser.c

@@ -1112,13 +1112,23 @@ vrrp_iptables_handler(const vector_t *strvec)
 		if (vector_size(strvec) >= 3) {
 			if (!check_valid_iptables_chain_name(strvec, 2, "out chain"))
 				return;
+
+			if (!strcmp(global_data->vrrp_iptables_inchain, strvec_slot(strvec, 2))) {
+				log_message(LOG_INFO, "vrrp_iptables: chain names cannot be the same");
+				FREE_CONST_PTR(global_data->vrrp_iptables_inchain);
+
+				return;
+			}
 			global_data->vrrp_iptables_outchain = STRDUP(strvec_slot(strvec,2));
 		}
-	} else {
-		global_data->vrrp_iptables_inchain = STRDUP(DEFAULT_IPTABLES_CHAIN_IN);
-		global_data->vrrp_iptables_outchain = STRDUP(DEFAULT_IPTABLES_CHAIN_OUT);
+
+		return;
 	}
+
+	global_data->vrrp_iptables_inchain = STRDUP(DEFAULT_IPTABLES_CHAIN_IN);
+	global_data->vrrp_iptables_outchain = STRDUP(DEFAULT_IPTABLES_CHAIN_OUT);
 }
+
 #ifdef _HAVE_LIBIPSET_
 static bool
 check_valid_ipset_name(const vector_t *strvec, unsigned entry, const char *log_name)
@@ -1131,6 +1141,17 @@ vrrp_ipsets_handler(const vector_t *strvec)
 {
 	size_t len;
 	char set_name[IPSET_MAXNAMELEN];
+	unsigned sn0, sn1;
+	const char **set_names[] = {
+		&global_data->vrrp_ipset_address,
+		&global_data->vrrp_ipset_address6,
+		&global_data->vrrp_ipset_address_iface6,
+		&global_data->vrrp_ipset_igmp,
+		&global_data->vrrp_ipset_mld,
+#ifdef _HAVE_VRRP_VMAC_
+		&global_data->vrrp_ipset_vmac_nd
+#endif
+						};
 
 	FREE_CONST_PTR(global_data->vrrp_ipset_address);
 	FREE_CONST_PTR(global_data->vrrp_ipset_address6);
@@ -1140,6 +1161,7 @@ vrrp_ipsets_handler(const vector_t *strvec)
 #ifdef _HAVE_VRRP_VMAC_
 	FREE_CONST_PTR(global_data->vrrp_ipset_vmac_nd);
 #endif
+	global_data->using_ipsets = PARAMETER_UNSET;
 
 	if (vector_size(strvec) < 2) {
 		global_data->using_ipsets = false;
@@ -1152,7 +1174,7 @@ vrrp_ipsets_handler(const vector_t *strvec)
 
 	if (vector_size(strvec) >= 3) {
 		if (!check_valid_ipset_name(strvec, 2, "IPv6 address"))
-			return;
+			goto ipset_error;
 		global_data->vrrp_ipset_address6 = STRDUP(strvec_slot(strvec,2));
 	} else {
 		/* No second set specified, copy first name and add "6" */
@@ -1164,7 +1186,7 @@ vrrp_ipsets_handler(const vector_t *strvec)
 
 	if (vector_size(strvec) >= 4) {
 		if (!check_valid_ipset_name(strvec, 3, "IPv6 address_iface"))
-			return;
+			goto ipset_error;
 		global_data->vrrp_ipset_address_iface6 = STRDUP(strvec_slot(strvec,3));
 	} else {
 		/* No third set specified, copy second name and add "_if6" */
@@ -1179,7 +1201,7 @@ vrrp_ipsets_handler(const vector_t *strvec)
 
 	if (vector_size(strvec) >= 5) {
 		if (!check_valid_ipset_name(strvec, 4, "IGMP"))
-			return;
+			goto ipset_error;
 		global_data->vrrp_ipset_igmp = STRDUP(strvec_slot(strvec,4));
 	} else {
 		/* No second set specified, copy first name and add "_igmp" */
@@ -1191,7 +1213,7 @@ vrrp_ipsets_handler(const vector_t *strvec)
 
 	if (vector_size(strvec) >= 6) {
 		if (!check_valid_ipset_name(strvec, 5, "MLD"))
-			return;
+			goto ipset_error;
 		global_data->vrrp_ipset_mld = STRDUP(strvec_slot(strvec,5));
 	} else {
 		/* No second set specified, copy first name and add "_mld" */
@@ -1204,7 +1226,7 @@ vrrp_ipsets_handler(const vector_t *strvec)
 #ifdef _HAVE_VRRP_VMAC_
 	if (vector_size(strvec) >= 7) {
 		if (!check_valid_ipset_name(strvec, 6, "ND"))
-			return;
+			goto ipset_error;
 		global_data->vrrp_ipset_vmac_nd = STRDUP(strvec_slot(strvec,6));
 	} else {
 		/* No second set specified, copy first name and add "_nd" */
@@ -1214,6 +1236,30 @@ vrrp_ipsets_handler(const vector_t *strvec)
 		global_data->vrrp_ipset_vmac_nd = STRDUP(set_name);
 	}
 #endif
+
+	/* Ensure all the set names are different */
+	for (sn0 = 0; sn0 < sizeof(set_names) / sizeof(set_names[0]) - 1; sn0++) {
+		for (sn1 = sn0 + 1; sn1 < sizeof(set_names) / sizeof(set_names[0]); sn1++) {
+			if (!strcmp(*set_names[sn0], *set_names[sn1])) {
+				report_config_error(CONFIG_GENERAL_ERROR, "vrrp_ipsets: set name %s used more than once", *set_names[sn0]);
+				goto ipset_error;
+			}
+		}
+	}
+
+	global_data->using_ipsets = true;
+
+	return;
+
+ipset_error:
+	FREE_CONST_PTR(global_data->vrrp_ipset_address);
+	FREE_CONST_PTR(global_data->vrrp_ipset_address6);
+	FREE_CONST_PTR(global_data->vrrp_ipset_address_iface6);
+	FREE_CONST_PTR(global_data->vrrp_ipset_igmp);
+	FREE_CONST_PTR(global_data->vrrp_ipset_mld);
+#ifdef _HAVE_VRRP_VMAC_
+	FREE_CONST_PTR(global_data->vrrp_ipset_vmac_nd);
+#endif
 }
 #endif
 #elif defined _WITH_NFTABLES_
@@ -1252,8 +1298,7 @@ vrrp_nftables_handler(__attribute__((unused)) const vector_t *strvec)
 		if (!check_valid_nftables_chain_name(strvec, 1, "chain"))
 			return;
 		name = strvec_slot(strvec, 1);
-	}
-	else {
+	} else {
 		/* Table name defaults to "keepalived" */
 		name = DEFAULT_NFTABLES_TABLE;
 	}

keepalived/core/main.c

@@ -43,7 +43,6 @@
 #include "main.h"
 #include "global_data.h"
 #include "daemon.h"
-#include "config.h"
 #ifndef _ONE_PROCESS_DEBUG_
 #include "config_notify.h"
 #endif
@@ -83,7 +82,6 @@
 #include "namespaces.h"
 #include "scheduler.h"
 #include "keepalived_netlink.h"
-#include "git-commit.h"
 #if defined THREAD_DUMP || defined _EPOLL_DEBUG_ || defined _EPOLL_THREAD_DUMP_ || defined _SCRIPT_DEBUG_
 #include "scheduler.h"
 #endif
@@ -118,6 +116,9 @@
 #ifdef _USE_SYSTEMD_NOTIFY_
 #include "systemd.h"
 #endif
+#ifdef _WITH_SANITIZER_
+#include "sanitizer.h"
+#endif
 #include "warnings.h"
 
 #define CHILD_WAIT_SECS	5
@@ -2470,6 +2471,10 @@ keepalived_main(int argc, char **argv)
 	}
 #endif
 
+#ifdef _WITH_SANITIZER_
+	sanitizer_init();
+#endif
+
 #ifdef _REPRODUCIBLE_BUILD_
 	char *config_opts_read;