Skip to contentSkip to conversation

Add regression test for CVE-2017-17053

PATCH Superseded Michael Moese 2 participants 6 messages Feb 12, 2018 – Mar 6, 2018 Library code

v4 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v4 cover Add regression test for CVE-2017-17053

Add a regression test for CVE-2017-17053. This testcase is depending on some new library functions included in this series.

This patch series consists of reworked patches according to previous review comments, as well as a small new library wrapper function SAFE_SIGACTION() to install a signal handler.

I made this entire series a v4, as the the library support for /proc/sys/kernel/tainted was previously a v3.

Michael Moese (3):
Add library support for /proc/sys/kernel/tainted Add a library wrapper for sigaction() Add regression test for CVE-2017-17053

 doc/test-writing-guidelines.txt |  42 +++++++++++++
 include/tst_safe_macros.h       |  20 ++++++
 include/tst_taint.h             | 104 +++++++++++++++++++++++++++++++
 lib/tst_taint.c                 | 106 ++++++++++++++++++++++++++++++++
 runtest/cve                     |   1 +
 testcases/cve/.gitignore        |   1 +
 testcases/cve/Makefile          |   1 +
 testcases/cve/cve-2017-17053.c  | 132 ++++++++++++++++++++++++++++++++++++++++
 8 files changed, 407 insertions(+)
 create mode 100644 include/tst_taint.h
 create mode 100644 lib/tst_taint.c
 create mode 100644 testcases/cve/cve-2017-17053.c
Signature · 1 line
2.13.6
#1 lore raw

PATCH v4 1/3 Add library support for /proc/sys/kernel/tainted

Sometimes, it is important to detect if the kernel has issued a warning, died, or is tainted in another way. Linux provides this information in /proc/sys/kernel/tainted in the form of a bitfield. This patch provides library functions for testcases to detect, if it has tainted the kernel.

The following functions will be introduced:

- int tst_taint_init(unsigned int mask)
check if the flags supplied as mask are supported by the running kernel, and if so, if they are not yet set.

- int tst_taint_check()
check if one or more of the bits specified in the mask provided to tst_taint_init() before are set.
Returns 0 if those flags are not set, or the bitmask of set flags

These can be used in the following way:

First, during testcase setup:

void setup(void)

{
	...
	tst_taint_init(TST_TAINT_W | TST_TAINT_D);
}

Second, check if the test triggered a bug:

void run(void)

{
	...
	. test code here
	...
	if (tst_taint_check() != 0)
		tst_res(TFAIL, "kernel has issues");
	 else
		tst_res(TPASS, "kernel seems to be fine");
}
---
 doc/test-writing-guidelines.txt |  42 ++++++++++++++++
 include/tst_taint.h             | 104 +++++++++++++++++++++++++++++++++++++++
 lib/tst_taint.c                 | 106 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 252 insertions(+)
 create mode 100644 include/tst_taint.h
 create mode 100644 lib/tst_taint.c
doc/test-writing-guidelines.txt +42 -0
index 739b295b8..f12386485 100644
@@ -1312,6 +1312,48 @@ common.h:9: FAIL: check failed
 test.c:8: INFO: do_action(arg) failed
 -------------------------------------------------------------------------------
 
+2.2.24 Tainted kernels
+^^^^^^^^^^^^^^^^^^^^^^
+
+If you need to detect, if a testcase triggers a kernel warning, bug or oops,
+the following can be used to detect TAINT_W or TAINT_D:
+
+[source,c]
+-------------------------------------------------------------------------------
+#include "tst_test.h"
+#include "tst_taint.h"
+
+void setup(void)
+{
+	...
+	tst_taint_init(TST_TAINT_W | TST_TAINT_D);
+	...
+}
+...
+void run(void)
+{
+	...
+	if (tst_taint_check() == 0)
+		tst_res(TPASS, "kernel is not tainted");
+	else
+		tst_res(TFAIL, "kernel is tainted");
+}
+-------------------------------------------------------------------------------
+
+You have to call tst_taint_init() with non-zero flags first, preferably during
+setup(). The function will generate a TCONF if the requested flags are not
+fully supported on the running kernel, and TBROK if either a zero mask was
+supplied or if the kernel is already tainted before executing the test.
+
+Then you can call tst_taint_check() during run(), which returns 0 or the 
+tainted flags set in /proc/sys/kernel/tainted as specified earlier.
+
+Depending on your kernel version, not all tainted-flags will be supported.
+
+For reference to tainted kernels, see kernel documentation:
+Documentation/admin-guide/tainted-kernels.rst or
+https://www.kernel.org/doc/html/latest/admin-guide/tainted-kernels.html
+
 2.3 Writing a testcase in shell
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
include/tst_taint.h new file +104 -0

104 lines not on this page · raw message

lib/tst_taint.c new file +106 -0

106 lines not on this page · raw message

Signature · 1 line
2.13.6
#2 lore raw

PATCH v4 2/3 Add a library wrapper for sigaction()

In reply to Michael Moese

In a multithreaded program, using signal() results in unspecified behavior. In this case, sigaction() has to be used to install a signal handler.
Therefore, SAFE_SIGACTION() is added.

---
 include/tst_safe_macros.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
include/tst_safe_macros.h +20 -0
index 06bff13c7..bf12e0719 100644
@@ -397,6 +397,26 @@ static inline sighandler_t safe_signal(const char *file, const int lineno,
 #define SAFE_SIGNAL(signum, handler) \
 	safe_signal(__FILE__, __LINE__, (signum), (handler))
 
+
+
+static inline int safe_sigaction(const char *file, const int lineno,
+				 int signum, const struct sigaction *act,
+				 struct sigaction *oldact)
+{
+	int rval;
+
+	rval = sigaction(signum, act, oldact);
+
+	if (rval == -1) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"sigaction(%d, %p, %p) failed", signum, act, oldact);
+	}
+
+	return rval;
+}
+#define SAFE_SIGACTION(signum, act, oldact) \
+	safe_sigaction(__FILE__, __LINE__, (signum), (act), (oldact))
+
 #define SAFE_EXECLP(file, arg, ...) do {                   \
 	execlp((file), (arg), ##__VA_ARGS__);              \
 	tst_brk_(__FILE__, __LINE__, TBROK | TERRNO,       \
Signature · 1 line
2.13.6
#3 lore raw

PATCH v4 3/3 Add regression test for CVE-2017-17053

In reply to Michael Moese

This patch adds a regression test for CVE-2017-17053, based on the reproducer in the message of this commit:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ccd5b3235180eef3cfec337df1c8554ab151b5cc

Be warned, if the running kernel is vulnerable to this CVE, it will die in most cases.

---
 runtest/cve                    |   1 +
 testcases/cve/.gitignore       |   1 +
 testcases/cve/Makefile         |   1 +
 testcases/cve/cve-2017-17053.c | 132 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 135 insertions(+)
 create mode 100644 testcases/cve/cve-2017-17053.c
runtest/cve +1 -0
index 6de2ed0ac..6efffc668 100644
@@ -29,3 +29,4 @@ cve-2017-17807 request_key04
 cve-2017-1000364 stack_clash
 cve-2017-5754 meltdown
 cve-2017-17052 cve-2017-17052
+cve-2017-17053 cve-2017-17053
testcases/cve/.gitignore +1 -0
index 42f32e825..5ecaaeb76 100644
@@ -11,3 +11,4 @@ cve-2017-5669
 meltdown
 stack_clash
 cve-2017-17052
+cve-2017-17053
testcases/cve/Makefile +1 -0
index 38ce27c93..00fbc1050 100644
@@ -37,5 +37,6 @@ meltdown: CFLAGS += -msse2
 endif
 
 cve-2017-17052:	CFLAGS += -pthread
+cve-2017-17053:	CFLAGS += -pthread
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
testcases/cve/cve-2017-17053.c new file +132 -0

132 lines not on this page · raw message

Signature · 1 line
2.13.6
#4 lore raw

PATCH v4 3/3 Add regression test for CVE-2017-17053

Hi!

Quoted 23 lines
testcases/cve/cve-2017-17053.c new file +23 -0
+#include <asm/ldt.h>
+#include <pthread.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <sys/syscall.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include "tst_test.h"
+#include "tst_taint.h"
+#include "tst_safe_pthread.h"
+#include "lapi/syscalls.h"
+
+#define EXEC_USEC   5000000
+
+static volatile sig_atomic_t *do_exit;
+
+static void handler(int sig, siginfo_t *si, void *unused)
+{
+	(void)(sig);
+	(void)(si);
+	(void)(unused);
+

testcases/cve/cve-2017-17053.c line 56

Quoted 10 lines
testcases/cve/cve-2017-17053.c new file +10 -0
+	*do_exit = -1;
+}
+
+static void install_sighandler(void)
+{
+	struct sigaction sa;
+
+	sa.sa_flags = SA_SIGINFO;
+	sigemptyset(&sa.sa_mask);
+	sa.sa_sigaction = handler;

Haven't I told to use sa.sa_handler instead? Since we are not using the extra two arguments anyway.

testcases/cve/cve-2017-17053.c line 68

Quoted 11 lines
testcases/cve/cve-2017-17053.c new file +11 -0
+	SAFE_SIGACTION(SIGSEGV, &sa, NULL);
+}
+
+static void setup(void)
+{
+	tst_taint_init(TST_TAINT_W | TST_TAINT_D);
+
+	do_exit = SAFE_MMAP(NULL, sizeof(*do_exit), PROT_READ | PROT_WRITE,
+			    MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+
+	*do_exit = 0;

There is no need to zero it here as we have to reset the flag in the run() function as well.

Quoted 46 lines
testcases/cve/cve-2017-17053.c new file +46 -0
+}
+
+static void cleanup(void)
+{
+	SAFE_MUNMAP(do_exit, sizeof(*do_exit));
+}
+
+static void *fork_thread(void *arg)
+{
+	SAFE_FORK();
+	return arg;
+}
+
+
+void run_test(void)
+{
+	struct user_desc desc = { .entry_number = 8191 };
+	install_sighandler();
+
+	syscall(__NR_modify_ldt, 1, &desc, sizeof(desc));
+
+	for (;;) {
+		if (*do_exit)
+			exit(0);
+
+		if (SAFE_FORK() == 0) {
+			pthread_t t;
+
+			srand(getpid());
+			SAFE_PTHREAD_CREATE(&t, NULL, fork_thread, NULL);
+			usleep(rand() % 10000);
+			syscall(__NR_exit_group, 0);
+		}
+	}
+}
+
+void run(void)
+{
+	int status;
+	pid_t pid;
+
+	*do_exit = 0;
+	pid = SAFE_FORK();
+
+	if (pid == 0) {
+		run_test();

testcases/cve/cve-2017-17053.c line 124

Quoted 10 lines
testcases/cve/cve-2017-17053.c new file +10 -0
+	} else {
+		usleep(EXEC_USEC);
+		*do_exit = 1;
+	}
+
+	SAFE_WAIT(&status);
+	if ((*do_exit == -1) || !WIFEXITED(status) || (tst_taint_check() != 0))
+		tst_res(TFAIL, "kernel is vulnerable");
+	else
+		tst_res(TPASS, "kernel survived");

We do overwrite the do_exit in the parent process unconditionally, hence it will always end up with 1 after we got to the SAFE_WAIT() here, or did I overlooked something?

We mmap() a page of shared memory in the test setup anyways so I suppose that adding a second int pointer called segfaulted or something similar that would be set from the signal handler would be cleanest solution.

Quoted 14 lines
testcases/cve/cve-2017-17053.c new file +8 -0
+}
+
+static struct tst_test test = {
+	.forks_child = 1,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run,
+};
Signature · 5 lines
2.13.6


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp
Signature · 2 lines
Cyril Hrubis
[email protected]
#5 lore raw

PATCH v4 3/3 Add regression test for CVE-2017-17053

Hi,

Quoted 29 lines · Cyril Hrubis

Hi!

Quoted 28 lines
testcases/cve/cve-2017-17053.c new file +28 -0
+#include <asm/ldt.h>
+#include <pthread.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <sys/syscall.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include "tst_test.h"
+#include "tst_taint.h"
+#include "tst_safe_pthread.h"
+#include "lapi/syscalls.h"
+
+#define EXEC_USEC   5000000
+
+static volatile sig_atomic_t *do_exit;
+
+static void handler(int sig, siginfo_t *si, void *unused)
+{
+	(void)(sig);
+	(void)(si);
+	(void)(unused);
+
+	*do_exit = -1;
+}
+
+static void install_sighandler(void)
+{
Quoted 9 lines

testcases/cve/cve-2017-17053.c line 56

Quoted 5 lines
testcases/cve/cve-2017-17053.c new file +5 -0
+	struct sigaction sa;
+
+	sa.sa_flags = SA_SIGINFO;
+	sigemptyset(&sa.sa_mask);
+	sa.sa_sigaction = handler;

Haven't I told to use sa.sa_handler instead? Since we are not using the extra two arguments anyway.

I possibly forgot to change that - but corrected this.

Quoted 76 lines

testcases/cve/cve-2017-17053.c line 68

Quoted 11 lines
testcases/cve/cve-2017-17053.c new file +11 -0
+	SAFE_SIGACTION(SIGSEGV, &sa, NULL);
+}
+
+static void setup(void)
+{
+	tst_taint_init(TST_TAINT_W | TST_TAINT_D);
+
+	do_exit = SAFE_MMAP(NULL, sizeof(*do_exit), PROT_READ | PROT_WRITE,
+			    MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+
+	*do_exit = 0;

There is no need to zero it here as we have to reset the flag in the run() function as well.

Quoted 46 lines
testcases/cve/cve-2017-17053.c new file +46 -0
+}
+
+static void cleanup(void)
+{
+	SAFE_MUNMAP(do_exit, sizeof(*do_exit));
+}
+
+static void *fork_thread(void *arg)
+{
+	SAFE_FORK();
+	return arg;
+}
+
+
+void run_test(void)
+{
+	struct user_desc desc = { .entry_number = 8191 };
+	install_sighandler();
+
+	syscall(__NR_modify_ldt, 1, &desc, sizeof(desc));
+
+	for (;;) {
+		if (*do_exit)
+			exit(0);
+
+		if (SAFE_FORK() == 0) {
+			pthread_t t;
+
+			srand(getpid());
+			SAFE_PTHREAD_CREATE(&t, NULL, fork_thread, NULL);
+			usleep(rand() % 10000);
+			syscall(__NR_exit_group, 0);
+		}
+	}
+}
+
+void run(void)
+{
+	int status;
+	pid_t pid;
+
+	*do_exit = 0;
+	pid = SAFE_FORK();
+
+	if (pid == 0) {
+		run_test();

testcases/cve/cve-2017-17053.c line 124

Quoted 10 lines
testcases/cve/cve-2017-17053.c new file +10 -0
+	} else {
+		usleep(EXEC_USEC);
+		*do_exit = 1;
+	}
+
+	SAFE_WAIT(&status);
+	if ((*do_exit == -1) || !WIFEXITED(status) || (tst_taint_check() != 0))
+		tst_res(TFAIL, "kernel is vulnerable");
+	else
+		tst_res(TPASS, "kernel survived");

We do overwrite the do_exit in the parent process unconditionally, hence it will always end up with 1 after we got to the SAFE_WAIT() here, or did I overlooked something?

Quoted 3 lines

We mmap() a page of shared memory in the test setup anyways so I suppose that adding a second int pointer called segfaulted or something similar that would be set from the signal handler would be cleanest solution.

Agreed. I changed the shared memory to a struct and added a second member to it for this. In this version, a value of -1 for do_exit was used to signal the segfault. However, having a second field is much cleaner. I will do some testing that I did not break anything and then resend.

Quoted 12 lines
Quoted 12 lines
testcases/cve/cve-2017-17053.c new file +8 -0
+}
+
+static struct tst_test test = {
+	.forks_child = 1,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run,
+};
Signature · 1 line
2.13.6
Quoted 6 lines
Quoted 2 lines
Signature · 1 line
Mailing list info: https://lists.linux.it/listinfo/ltp
Signature · 2 lines
Cyril Hrubis
[email protected]

Michael

Signature · 1 line
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)