Commit d5865759f8698f1c75339451a26fa3ae00276a51

Guillem Jover 2022-08-25T00:52:43

test: Fix explicit_bzero() test on the Hurd On the Hurd a small read(3) might end up (indirectly) copying the data on the stack, which we will end up finding even when we have cleared the buffer. To avoid these side effects, we add a new function, that we force not to be inlined, so that we can reuse the same stack space, that will blank any possible stack side effects. This should be portable regardless of stack growing up or down. Diagnosis-by: Samuel Thibault <sthibault@debian.org>

diff --git a/COPYING b/COPYING
index 67223d4..cf43edd 100644
--- a/COPYING
+++ b/COPYING
@@ -369,7 +369,7 @@ Copyright:
  Copyright © 2014 Theo de Raadt <deraadt@openbsd.org>
  Copyright © 2014 Google Inc.
  Copyright © 2015 Michael Felt <aixtools@gmail.com>
- Copyright © 2015 Guillem Jover <guillem@hadrons.org>
+ Copyright © 2015, 2022 Guillem Jover <guillem@hadrons.org>
 License: ISC
  Permission to use, copy, modify, and distribute this software for any
  purpose with or without fee is hereby granted, provided that the above
diff --git a/test/explicit_bzero.c b/test/explicit_bzero.c
index 74993c2..bee29de 100644
--- a/test/explicit_bzero.c
+++ b/test/explicit_bzero.c
@@ -1,6 +1,7 @@
 /*	$OpenBSD: explicit_bzero.c,v 1.7 2021/03/27 11:17:58 bcook Exp $	*/
 /*
  * Copyright (c) 2014 Google Inc.
+ * Copyright (c) 2022 Guillem Jover <guillem@hadrons.org>
  *
  * Permission to use, copy, modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -123,6 +124,18 @@ populate_secret(char *buf, ssize_t len)
 	ASSERT_EQ(0, close(fds[0]));
 }
 
+static void __attribute__((__noinline__))
+blank_stack_side_effects(char *buf, size_t len)
+{
+	char scratch[SECRETBYTES * 4];
+
+	/* If the read(3) in populate_secret() wrote into the stack, as it
+	 * might happen on the Hurd for small data, then we might incorrectly
+	 * detect the wrong secret on the stack. */
+	memset(scratch, 0xFF, sizeof(scratch));
+	ASSERT_EQ(NULL, memmem(scratch, sizeof(scratch), buf, len));
+}
+
 static int
 count_secrets(const char *buf)
 {
@@ -143,6 +156,7 @@ test_without_bzero(void)
 	char *res;
 	assert_on_stack();
 	populate_secret(buf, sizeof(buf));
+	blank_stack_side_effects(buf, sizeof(buf));
 	res = memmem(altstack, ALTSTACK_SIZE, buf, sizeof(buf));
 	ASSERT_NE(NULL, res);
 	return (res);
@@ -155,6 +169,7 @@ test_with_bzero(void)
 	char *res;
 	assert_on_stack();
 	populate_secret(buf, sizeof(buf));
+	blank_stack_side_effects(buf, sizeof(buf));
 	res = memmem(altstack, ALTSTACK_SIZE, buf, sizeof(buf));
 	ASSERT_NE(NULL, res);
 	explicit_bzero(buf, sizeof(buf));