Skip to contentSkip to conversation

Add regression test for CVE-2017-17052

PATCH Superseded Michael Moese 2 participants 5 messages Jan 12, 2018 – Jan 22, 2018

v3 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 Add regression test for CVE-2017-17052

original reproducer can be found here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2b7e8665b4ff51c034c55df3cff76518d1a9ee3a

---
 runtest/cve                    |   1 +
 testcases/cve/.gitignore       |   1 +
 testcases/cve/Makefile         |   2 +
 testcases/cve/cve-2017-17052.c | 129 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 133 insertions(+)
 create mode 100644 testcases/cve/cve-2017-17052.c
runtest/cve +1 -0
index 5d124083e..529d832a9 100644
@@ -27,3 +27,4 @@ cve-2017-15537 ptrace07
 cve-2017-15951 request_key03 -b cve-2017-15951
 cve-2017-1000364 stack_clash
 cve-2017-5754 meltdown
+cve-2017-17052 cve-2017-17052
testcases/cve/.gitignore +1 -0
index 2566dbd18..42f32e825 100644
@@ -10,3 +10,4 @@ cve-2017-6951
 cve-2017-5669
 meltdown
 stack_clash
+cve-2017-17052
testcases/cve/Makefile +2 -0
index a7df1e43c..38ce27c93 100644
@@ -36,4 +36,6 @@ ifneq (,$(filter $(HOST_CPU),x86 x86_64))
 meltdown: CFLAGS += -msse2
 endif
 
+cve-2017-17052:	CFLAGS += -pthread
+
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
testcases/cve/cve-2017-17052.c new file +129 -0

129 lines not on this page · raw message

Signature · 1 line
2.13.6
#1 lore raw

PATCH v3 1/1 Add regression test for CVE-2017-17052

Hi!

Quoted 7 lines
testcases/cve/cve-2017-17052.c new file +7 -0
+#include <unistd.h>
+#include <pthread.h>
+#include <sys/wait.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+
+#include "tst_test.h"

testcases/cve/cve-2017-17052.c line 43

Quoted 10 lines
testcases/cve/cve-2017-17052.c new file +10 -0
+#include "tst_safe_pthread.h"
+#include "lapi/syscalls.h"
+
+#define RUNS	   4
+#define EXEC_USEC  400000
+
+struct my_shm_data {
+	int exit;
+};
+static struct my_shm_data *shm;

There is no need to pack the the exit into a structure like that, we can simply do:

static volatile int *do_exit;

...

	do_exit = SAFE_MMAP(...);

And it should be volatile as well, so that it's not optimized-out of the loops by the compiler.

testcases/cve/cve-2017-17052.c line 47

Quoted 3 lines
testcases/cve/cve-2017-17052.c new file +3 -0
+static void setup(void)
+{
+	shm = SAFE_MMAP(NULL, sizeof(struct my_shm_data), PROT_READ|PROT_WRITE,

^
The system aligns the length to be a multiple of pagesize, so we may as well pass result of getpagesize() here.

testcases/cve/cve-2017-17052.c line 55

Quoted 8 lines
testcases/cve/cve-2017-17052.c new file +8 -0
+		    MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+
+	shm->exit = 0;
+}
+
+static void cleanup(void)
+{
+	SAFE_MUNMAP(shm, sizeof(struct my_shm_data));

^
Here we must pass length that is multiple of pagesize, at least manual pages says so.

testcases/cve/cve-2017-17052.c line 58

Quoted 3 lines
testcases/cve/cve-2017-17052.c new file +3 -0
+}
+
+static void *mmap_thread(void *_arg)

Identifiers starting with underscore are reserved for system i.e. libc we should avoid using these here.

testcases/cve/cve-2017-17052.c line 65

Quoted 7 lines
testcases/cve/cve-2017-17052.c new file +7 -0
+{
+	for (;;) {
+		SAFE_MMAP(NULL, 0x1000000, PROT_READ,
+				MAP_POPULATE|MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
+		if (shm->exit)
+			exit(0);
+	}

We may as well do:

	return arg;

Which is a nice trick to avoid unused warnings.

Also you are supposed to include stdlib.h for exit(3).

testcases/cve/cve-2017-17052.c line 75

Quoted 10 lines
testcases/cve/cve-2017-17052.c new file +10 -0
+}
+
+static void *fork_thread(void *_arg)
+{
+	if (shm->exit)
+		exit(0);
+
+	usleep(rand() % 10000);
+	SAFE_FORK();
+}

Here as well, the arg should not start with underscore and we should add return to avoid the warnings as well.

Sorry for not pointing these in the previous review, also no need to respin the patch, I can fix the minor problems before commiting.

Signature · 2 lines
Cyril Hrubis
[email protected]
#2 lore raw

PATCH v3 1/1 Add regression test for CVE-2017-17052

Hi!

Quoted 7 lines

testcases/cve/cve-2017-17052.c line 47

Quoted 3 lines
testcases/cve/cve-2017-17052.c new file +3 -0
+static void setup(void)
+{
+	shm = SAFE_MMAP(NULL, sizeof(struct my_shm_data), PROT_READ|PROT_WRITE,

^
The system aligns the length to be a multiple of pagesize, so we may as well pass result of getpagesize() here.

Quoted 11 lines

testcases/cve/cve-2017-17052.c line 55

Quoted 8 lines
testcases/cve/cve-2017-17052.c new file +8 -0
+		    MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+
+	shm->exit = 0;
+}
+
+static void cleanup(void)
+{
+	SAFE_MUNMAP(shm, sizeof(struct my_shm_data));

^
Here we must pass length that is multiple of pagesize, at least manual pages says so.

Sorry, I've misread the manual page, that applies only for huge page mappings, so passing sizeof(*shm) to mmap() and munmap() should work just fine, it would still allocate whole page though.

Signature · 2 lines
Cyril Hrubis
[email protected]
#3 lore raw

PATCH v3 1/1 Add regression test for CVE-2017-17052

In reply to Cyril Hrubis

Hi again,
I totally agree with your remarks.

Quoted 2 lines

Sorry for not pointing these in the previous review, also no need to respin the patch, I can fix the minor problems before commiting.

If you want me to resend the patch, just tell me - I'll do that on monday.

Have a nice weekend,

Michael

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

PATCH v3 1/1 Add regression test for CVE-2017-17052

Hi!

Quoted 3 lines
Quoted 2 lines

Sorry for not pointing these in the previous review, also no need to respin the patch, I can fix the minor problems before commiting.

If you want me to resend the patch, just tell me - I'll do that on monday.

I've amended the changes locally, along with removing newline from one of the tst_res() messages and pushed, thanks.

Signature · 2 lines
Cyril Hrubis
[email protected]