git: 30cafaa9611d - main - devd tests: Fix client_test

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Thu, 21 Nov 2024 18:55:55 UTC
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=30cafaa9611d82525ab32ec32304b93835a5124a

commit 30cafaa9611d82525ab32ec32304b93835a5124a
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2024-11-21 18:55:13 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2024-11-21 18:55:13 +0000

    devd tests: Fix client_test
    
    The loop doesn't check for overflow of the event buffer, which can
    easily happen if other tests are running in parallel (the bectl tests in
    particular trigger devd events).
    
    When that overflow occurs, a funny thing can happen: the loop ends up
    trying to read 0 bytes from the socket, succeeds, and then prints its
    buffer to stdout.  It does this as fast as possible, eventually timing
    out.  Then, because kyua wants to log the test's output, it slurps the
    output file into memory so that it can insert it into the test db.  This
    output file is quite large, usually around 8GB when I see it happen, and
    is large enough to trigger an OOM kill in my test suite runner VM.
    
    Fix the test: use a larger buffer and fail the test if we fill it before
    both events are observed.  Also don't print the output buffer on every
    loop iteration, since unlike the seqpacket test that will just print the
    same output over and over.
    
    Reviewed by:    imp, asomers
    MFC after:      2 weeks
    Sponsored by:   Klara, Inc.
    Differential Revision:  https://reviews.freebsd.org/D47625
---
 sbin/devd/tests/client_test.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/sbin/devd/tests/client_test.c b/sbin/devd/tests/client_test.c
index 729c7a2f8bad..fdac3c98b3c6 100644
--- a/sbin/devd/tests/client_test.c
+++ b/sbin/devd/tests/client_test.c
@@ -22,15 +22,14 @@
  * SUCH DAMAGE.
  */
 
-#include <sys/cdefs.h>
-#include <stdbool.h>
-#include <stdio.h>
-
 #include <sys/param.h>
-#include <sys/types.h>
 #include <sys/socket.h>
 #include <sys/un.h>
 
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+
 #include <atf-c.h>
 
 const char create_pat[] = "!system=DEVFS subsystem=CDEV type=CREATE cdev=md";
@@ -134,29 +133,37 @@ ATF_TC_BODY(seqpacket, tc)
 ATF_TC_WITHOUT_HEAD(stream);
 ATF_TC_BODY(stream, tc)
 {
+	char *event;
 	int s;
 	bool got_create_event = false;
 	bool got_destroy_event = false;
-	ssize_t len = 0;
+	size_t len = 0, sz;
 
 	s = common_setup(SOCK_STREAM, "/var/run/devd.pipe");
+
+	/*
+	 * Use a large buffer: we're reading from a stream socket so can't rely
+	 * on record boundaries.  Instead, we just keep appending to the buffer.
+	 */
+	sz = 1024 * 1024;
+	event = malloc(sz);
+	ATF_REQUIRE(event != NULL);
+
 	/*
 	 * Loop until both events are detected on the same or different reads.
 	 * There may be extra events due to unrelated system activity.
 	 * If we never get both events, then the test will timeout.
 	 */
-	while (!(got_create_event && got_destroy_event)) {
-		char event[1024];
+	while (!(got_create_event && got_destroy_event) && len < sz - 1) {
 		ssize_t newlen;
 		char *create_pos, *destroy_pos;
 
 		/* Read 1 less than sizeof(event) to allow space for NULL */
-		newlen = read(s, &event[len], sizeof(event) - len - 1);
-		ATF_REQUIRE(newlen != -1);
+		newlen = read(s, &event[len], sz - len - 1);
+		ATF_REQUIRE(newlen > 0);
 		len += newlen;
 		/* NULL terminate the result */
 		event[len] = '\0';
-		printf("%s", event);
 
 		create_pos = strstr(event, create_pat);
 		if (create_pos != NULL)
@@ -166,7 +173,11 @@ ATF_TC_BODY(stream, tc)
 		if (destroy_pos != NULL)
 			got_destroy_event = true;
 	}
+	printf("%s", event);
+	if (len >= sz - 1)
+		atf_tc_fail("Event buffer overflowed");
 
+	free(event);
 	close(s);
 }