From: Thomas Egerer <thomas.egerer@secunet.com>
Date: Fri, 2 Sep 2022 11:54:05 +0000 (+0000)
Subject: unit-tests: Use allocated listener instead of stack object in exchange tests
X-Git-Tag: 5.9.8dr4~11^2~7
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=996f557c409e6b65b47b9c81761535e7fd093117;p=thirdparty%2Fstrongswan.git

unit-tests: Use allocated listener instead of stack object in exchange tests

When using the statement expression and a stack object along with
clang-11 and libasan, we get quite a lot of errors about reading
invalid memory. This is due to clang making the actual listener_t local
to the block, such that the access outside of the macros using
_assert_payload is (correctly) considered an error.
By using a heap allocated object, we can destroy it once the listener
returns FALSE (cleaning up properly), and since bus_t does not touch the
listener after that, we don't get any errors from libasan.

Co-authored-by: Tobias Brunner <tobias@strongswan.org>
---

diff --git a/src/libcharon/tests/suites/test_ike_mid_sync.c b/src/libcharon/tests/suites/test_ike_mid_sync.c
index fa7592a6ac..efe58c23f7 100644
--- a/src/libcharon/tests/suites/test_ike_mid_sync.c
+++ b/src/libcharon/tests/suites/test_ike_mid_sync.c
@@ -22,7 +22,7 @@
 #include <bio/bio_reader.h>
 #include <bio/bio_writer.h>
 
-/**
+/*
  * FIXME: Since we don't have the server side yet, this is kind of a hack!!!
  */
 
@@ -37,15 +37,18 @@ static bool add_notify(listener_t *listener, ike_sa_t *ike_sa,
 	{
 		message->add_notify(message, FALSE, IKEV2_MESSAGE_ID_SYNC_SUPPORTED,
 							chunk_empty);
+		free(listener);
 		return FALSE;
 	}
 	return TRUE;
 }
+
 #define add_notify_to_ike_auth() ({ \
-	listener_t _notify_listener = { \
+	listener_t *_notify_listener; \
+	INIT(_notify_listener, \
 		.message = add_notify, \
-	}; \
-	exchange_test_helper->add_listener(exchange_test_helper, &_notify_listener); \
+	); \
+	exchange_test_helper->add_listener(exchange_test_helper, _notify_listener); \
 })
 
 /**
diff --git a/src/libcharon/tests/utils/exchange_test_asserts.c b/src/libcharon/tests/utils/exchange_test_asserts.c
index 8c39a66432..1a4fdda837 100644
--- a/src/libcharon/tests/utils/exchange_test_asserts.c
+++ b/src/libcharon/tests/utils/exchange_test_asserts.c
@@ -178,6 +178,8 @@ bool exchange_test_asserts_message(listener_t *listener, ike_sa_t *ike_sa,
 				assert_message_rule(this, message, &this->rules[i]);
 			}
 		}
+		free(this->rules);
+		free(this);
 		return FALSE;
 	}
 	return TRUE;
diff --git a/src/libcharon/tests/utils/exchange_test_asserts.h b/src/libcharon/tests/utils/exchange_test_asserts.h
index ac98dd4527..a58832d733 100644
--- a/src/libcharon/tests/utils/exchange_test_asserts.h
+++ b/src/libcharon/tests/utils/exchange_test_asserts.h
@@ -350,16 +350,18 @@ bool exchange_test_asserts_message(listener_t *this, ike_sa_t *ike_sa,
 
 #define _assert_payload(dir, c, ...) ({ \
 	listener_message_rule_t _rules[] = { __VA_ARGS__ }; \
-	listener_message_assert_t _listener = { \
+	listener_message_assert_t *_listener; \
+	INIT(_listener, \
 		.listener = { .message = exchange_test_asserts_message, }, \
 		.file = __FILE__, \
 		.line = __LINE__, \
 		.incoming = streq(dir, "IN") ? TRUE : FALSE, \
 		.count = c, \
-		.rules = _rules, \
+		.rules = malloc(sizeof(_rules)), \
 		.num_rules = countof(_rules), \
-	}; \
-	exchange_test_helper->add_listener(exchange_test_helper, &_listener.listener); \
+	); \
+	memcpy(_listener->rules, _rules, sizeof(_rules)); \
+	exchange_test_helper->add_listener(exchange_test_helper, &_listener->listener); \
 })
 
 /**