[LTP] [PATCH 1/3] request_key03: Refactor child process code

[LTP] [PATCH 1/3] request_key03: Refactor child process code

[Martin Doucha]

Split off child process code into separate functions.

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---

This is the final patchset of my max_runtime fixes. I've tried to keep
functional changes to a minimum. We can redesign the child processes
to run for a fixed period of time later.

I've also tested the patchset on a vulnerable kernel and it successfully
triggers the kernel bug.

 .../syscalls/request_key/request_key03.c      | 101 ++++++++++--------
 1 file changed, 58 insertions(+), 43 deletions(-)

diff --git a/testcases/kernel/syscalls/request_key/request_key03.c b/testcases/kernel/syscalls/request_key/request_key03.c
index 3f0c093f3..2780532f3 100644
--- a/testcases/kernel/syscalls/request_key/request_key03.c
+++ b/testcases/kernel/syscalls/request_key/request_key03.c
@@ -39,10 +39,65 @@
 
 static char *opt_bug;
 
+static void run_child_add(const char *type, const char *payload, int effort)
+{
+	int i;
+
+	/*
+	 * Depending on the state of the key, add_key() should either succeed or
+	 * fail with one of several errors:
+	 *
+	 * (1) key didn't exist at all: either add_key() should succeed (if the
+	 *     payload is valid), or it should fail with EINVAL (if the payload
+	 *     is invalid; this is needed for the "encrypted" and "trusted" key
+	 *     types because they have a quirk where the payload syntax differs
+	 *     for creating new keys vs. updating existing keys)
+	 *
+	 * (2) key was negative: add_key() should succeed
+	 *
+	 * (3) key was uninstantiated: add_key() should wait for the key to be
+	 *     negated, then fail with ENOKEY
+	 *
+	 * For now we also accept EDQUOT because the kernel frees up the keys
+	 * quota asynchronously after keys are unlinked.  So it may be hit.
+	 */
+	for (i = 0; i < 100 * effort; i++) {
+		usleep(rand() % 1024);
+		TEST(add_key(type, "desc", payload, strlen(payload),
+			KEY_SPEC_SESSION_KEYRING));
+		if (TST_RET < 0 && TST_ERR != EINVAL && TST_ERR != ENOKEY &&
+			TST_ERR != EDQUOT) {
+			tst_brk(TBROK | TTERRNO,
+				"unexpected error adding key of type '%s'",
+				type);
+		}
+
+		TEST(keyctl(KEYCTL_CLEAR, KEY_SPEC_SESSION_KEYRING));
+
+		if (TST_RET < 0)
+			tst_brk(TBROK | TTERRNO, "unable to clear keyring");
+	}
+}
+
+static void run_child_request(const char *type, int effort)
+{
+	int i;
+
+	for (i = 0; i < 5000 * effort; i++) {
+		TEST(request_key(type, "desc", "callout_info",
+			KEY_SPEC_SESSION_KEYRING));
+		if (TST_RET < 0 && TST_ERR != ENOKEY && TST_ERR != ENOENT &&
+			TST_ERR != EDQUOT) {
+			tst_brk(TBROK | TTERRNO,
+				"unexpected error requesting key of type '%s'",
+				type);
+		}
+	}
+}
+
 static void test_with_key_type(const char *type, const char *payload,
 			       int effort)
 {
-	int i;
 	int status;
 	pid_t add_key_pid;
 	pid_t request_key_pid;
@@ -68,56 +123,16 @@ static void test_with_key_type(const char *type, const char *payload,
 	/*
 	 * Fork a subprocess which repeatedly tries to "add" a key of the given
 	 * type.  This actually will try to update the key if it already exists.
-	 * Depending on the state of the key, add_key() should either succeed or
-	 * fail with one of several errors:
-	 *
-	 * (1) key didn't exist at all: either add_key() should succeed (if the
-	 *     payload is valid), or it should fail with EINVAL (if the payload
-	 *     is invalid; this is needed for the "encrypted" and "trusted" key
-	 *     types because they have a quirk where the payload syntax differs
-	 *     for creating new keys vs. updating existing keys)
-	 *
-	 * (2) key was negative: add_key() should succeed
-	 *
-	 * (3) key was uninstantiated: add_key() should wait for the key to be
-	 *     negated, then fail with ENOKEY
-	 *
-	 * For now we also accept EDQUOT because the kernel frees up the keys
-	 * quota asynchronously after keys are unlinked.  So it may be hit.
 	 */
 	add_key_pid = SAFE_FORK();
 	if (add_key_pid == 0) {
-		for (i = 0; i < 100 * effort; i++) {
-			usleep(rand() % 1024);
-			TEST(add_key(type, "desc", payload, strlen(payload),
-				     KEY_SPEC_SESSION_KEYRING));
-			if (TST_RET < 0 && TST_ERR != EINVAL &&
-			    TST_ERR != ENOKEY && TST_ERR != EDQUOT) {
-				tst_brk(TBROK | TTERRNO,
-					"unexpected error adding key of type '%s'",
-					type);
-			}
-			TEST(keyctl(KEYCTL_CLEAR, KEY_SPEC_SESSION_KEYRING));
-			if (TST_RET < 0) {
-				tst_brk(TBROK | TTERRNO,
-					"unable to clear keyring");
-			}
-		}
+		run_child_add(type, payload, effort);
 		exit(0);
 	}
 
 	request_key_pid = SAFE_FORK();
 	if (request_key_pid == 0) {
-		for (i = 0; i < 5000 * effort; i++) {
-			TEST(request_key(type, "desc", "callout_info",
-					 KEY_SPEC_SESSION_KEYRING));
-			if (TST_RET < 0 && TST_ERR != ENOKEY &&
-			    TST_ERR != ENOENT && TST_ERR != EDQUOT) {
-				tst_brk(TBROK | TTERRNO,
-					"unexpected error requesting key of type '%s'",
-					type);
-			}
-		}
+		run_child_request(type, effort);
 		exit(0);
 	}
 
-- 
2.37.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/3] request_key03: Split test into 3 testcases

[Martin Doucha]

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 .../syscalls/request_key/request_key03.c      | 70 ++++++++++---------
 1 file changed, 37 insertions(+), 33 deletions(-)

diff --git a/testcases/kernel/syscalls/request_key/request_key03.c b/testcases/kernel/syscalls/request_key/request_key03.c
index 2780532f3..cb256f41e 100644
--- a/testcases/kernel/syscalls/request_key/request_key03.c
+++ b/testcases/kernel/syscalls/request_key/request_key03.c
@@ -37,6 +37,30 @@
 #include "tst_test.h"
 #include "lapi/keyctl.h"
 
+static struct test_case {
+	const char *type;
+	const char *payload;
+	int effort;
+} testcase_list[] = {
+	/*
+	 * Briefly test the "encrypted" and/or "trusted" key types when
+	 * availaible, mainly to reproduce CVE-2017-15299.
+	 */
+	{"encrypted", "update user:foo 32", 2},
+	{"trusted", "update", 2},
+
+	/*
+	 * Test the "user" key type for longer, mainly in order to reproduce
+	 * CVE-2017-15951.  However, without the fix for CVE-2017-15299 as well,
+	 * WARNs may show up in the kernel log.
+	 *
+	 * Note: the precise iteration count is arbitrary; it's just intended to
+	 * be enough to give a decent chance of reproducing the bug, without
+	 * wasting too much time.
+	 */
+	{"user", "payload", 20},
+};
+
 static char *opt_bug;
 
 static void run_child_add(const char *type, const char *payload, int effort)
@@ -95,29 +119,29 @@ static void run_child_request(const char *type, int effort)
 	}
 }
 
-static void test_with_key_type(const char *type, const char *payload,
-			       int effort)
+static void do_test(unsigned int n)
 {
 	int status;
 	pid_t add_key_pid;
 	pid_t request_key_pid;
 	bool info_only;
+	struct test_case *tc = testcase_list + n;
 
 	TEST(keyctl(KEYCTL_JOIN_SESSION_KEYRING, NULL));
 	if (TST_RET < 0)
 		tst_brk(TBROK | TTERRNO, "failed to join new session keyring");
 
-	TEST(add_key(type, "desc", payload, strlen(payload),
+	TEST(add_key(tc->type, "desc", tc->payload, strlen(tc->payload),
 		     KEY_SPEC_SESSION_KEYRING));
 	if (TST_RET < 0 && TST_ERR != EINVAL) {
 		if (TST_ERR == ENODEV) {
 			tst_res(TCONF, "kernel doesn't support key type '%s'",
-				type);
+				tc->type);
 			return;
 		}
 		tst_brk(TBROK | TTERRNO,
 			"unexpected error checking whether key type '%s' is supported",
-			type);
+			tc->type);
 	}
 
 	/*
@@ -126,13 +150,13 @@ static void test_with_key_type(const char *type, const char *payload,
 	 */
 	add_key_pid = SAFE_FORK();
 	if (add_key_pid == 0) {
-		run_child_add(type, payload, effort);
+		run_child_add(tc->type, tc->payload, tc->effort);
 		exit(0);
 	}
 
 	request_key_pid = SAFE_FORK();
 	if (request_key_pid == 0) {
-		run_child_request(type, effort);
+		run_child_request(tc->type, tc->effort);
 		exit(0);
 	}
 
@@ -149,11 +173,11 @@ static void test_with_key_type(const char *type, const char *payload,
 	if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
 		tst_res(info_only ? TINFO : TPASS,
 			"didn't crash while updating key of type '%s'",
-			type);
+			tc->type);
 	} else if (WIFSIGNALED(status) && WTERMSIG(status) == SIGKILL) {
 		tst_res(info_only ? TINFO : TFAIL,
 			"kernel oops while updating key of type '%s'",
-			type);
+			tc->type);
 	} else {
 		tst_brk(TBROK, "add_key child %s", tst_strstatus(status));
 	}
@@ -163,39 +187,19 @@ static void test_with_key_type(const char *type, const char *payload,
 	if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
 		tst_res(info_only ? TINFO : TPASS,
 			"didn't crash while requesting key of type '%s'",
-			type);
+			tc->type);
 	} else if (WIFSIGNALED(status) && WTERMSIG(status) == SIGKILL) {
 		tst_res(info_only ? TINFO : TFAIL,
 			"kernel oops while requesting key of type '%s'",
-			type);
+			tc->type);
 	} else {
 		tst_brk(TBROK, "request_key child %s", tst_strstatus(status));
 	}
 }
 
-static void do_test(void)
-{
-	/*
-	 * Briefly test the "encrypted" and/or "trusted" key types when
-	 * availaible, mainly to reproduce CVE-2017-15299.
-	 */
-	test_with_key_type("encrypted", "update user:foo 32", 2);
-	test_with_key_type("trusted", "update", 2);
-
-	/*
-	 * Test the "user" key type for longer, mainly in order to reproduce
-	 * CVE-2017-15951.  However, without the fix for CVE-2017-15299 as well,
-	 * WARNs may show up in the kernel log.
-	 *
-	 * Note: the precise iteration count is arbitrary; it's just intended to
-	 * be enough to give a decent chance of reproducing the bug, without
-	 * wasting too much time.
-	 */
-	test_with_key_type("user", "payload", 20);
-}
-
 static struct tst_test test = {
-	.test_all = do_test,
+	.test = do_test,
+	.tcnt = ARRAY_SIZE(testcase_list),
 	.forks_child = 1,
 	.options = (struct tst_option[]) {
 		{"b:", &opt_bug,  "Bug to test for (cve-2017-15299 or cve-2017-15951; default is both)"},
-- 
2.37.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 3/3] request_key03: Add max_runtime and make children runtime-aware

[Martin Doucha]

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 .../kernel/syscalls/request_key/request_key03.c      | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/testcases/kernel/syscalls/request_key/request_key03.c b/testcases/kernel/syscalls/request_key/request_key03.c
index cb256f41e..464fcd8a4 100644
--- a/testcases/kernel/syscalls/request_key/request_key03.c
+++ b/testcases/kernel/syscalls/request_key/request_key03.c
@@ -100,6 +100,11 @@ static void run_child_add(const char *type, const char *payload, int effort)
 
 		if (TST_RET < 0)
 			tst_brk(TBROK | TTERRNO, "unable to clear keyring");
+
+		if (!tst_remaining_runtime()) {
+			tst_res(TINFO, "add_key() process runtime exceeded");
+			break;
+		}
 	}
 }
 
@@ -116,6 +121,12 @@ static void run_child_request(const char *type, int effort)
 				"unexpected error requesting key of type '%s'",
 				type);
 		}
+
+		if (!tst_remaining_runtime()) {
+			tst_res(TINFO,
+				"request_key() process runtime exceeded");
+			break;
+		}
 	}
 }
 
@@ -201,6 +212,7 @@ static struct tst_test test = {
 	.test = do_test,
 	.tcnt = ARRAY_SIZE(testcase_list),
 	.forks_child = 1,
+	.max_runtime = 20,
 	.options = (struct tst_option[]) {
 		{"b:", &opt_bug,  "Bug to test for (cve-2017-15299 or cve-2017-15951; default is both)"},
 		{}
-- 
2.37.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] request_key03: Refactor child process code

[Cyril Hrubis]

Hi!
Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp