All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russell Currey <ruscur@russell.cc>
To: keescook@chromium.org
Cc: linux-kernel@vger.kernel.org, dja@axtens.net, mpe@ellerman.id.au,
	kernel-hardening@lists.openwall.com,
	linuxppc-dev@lists.ozlabs.org, christophe.leroy@c-s.fr,
	Russell Currey <ruscur@russell.cc>
Subject: [PATCH] lkdtm: Test KUAP directional user access unlocks on powerpc
Date: Fri, 31 Jan 2020 16:31:57 +1100	[thread overview]
Message-ID: <20200131053157.22463-1-ruscur@russell.cc> (raw)

Kernel Userspace Access Prevention (KUAP) on powerpc supports
allowing only one access direction (Read or Write) when allowing access
to or from user memory.

A bug was recently found that showed that these one-way unlocks never
worked, and allowing Read *or* Write would actually unlock Read *and*
Write.  We should have a test case for this so we can make sure this
doesn't happen again.

Like ACCESS_USERSPACE, the correct result is for the test to fault.

At the time of writing this, the upstream kernel still has this bug
present, so the test will allow both accesses whereas ACCESS_USERSPACE
will correctly fault.

Signed-off-by: Russell Currey <ruscur@russell.cc>
---
 drivers/misc/lkdtm/core.c  |  3 +++
 drivers/misc/lkdtm/lkdtm.h |  3 +++
 drivers/misc/lkdtm/perms.c | 43 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+)

diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index ee0d6e721441..baef3c6f48d6 100644
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c
@@ -137,6 +137,9 @@ static const struct crashtype crashtypes[] = {
 	CRASHTYPE(EXEC_USERSPACE),
 	CRASHTYPE(EXEC_NULL),
 	CRASHTYPE(ACCESS_USERSPACE),
+#ifdef CONFIG_PPC_KUAP
+	CRASHTYPE(ACCESS_USERSPACE_KUAP),
+#endif
 	CRASHTYPE(ACCESS_NULL),
 	CRASHTYPE(WRITE_RO),
 	CRASHTYPE(WRITE_RO_AFTER_INIT),
diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index c56d23e37643..406a3fb32e6f 100644
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h
@@ -57,6 +57,9 @@ void lkdtm_EXEC_RODATA(void);
 void lkdtm_EXEC_USERSPACE(void);
 void lkdtm_EXEC_NULL(void);
 void lkdtm_ACCESS_USERSPACE(void);
+#ifdef CONFIG_PPC_KUAP
+void lkdtm_ACCESS_USERSPACE_KUAP(void);
+#endif
 void lkdtm_ACCESS_NULL(void);
 
 /* lkdtm_refcount.c */
diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 62f76d506f04..2c9aa0114333 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -10,6 +10,9 @@
 #include <linux/mman.h>
 #include <linux/uaccess.h>
 #include <asm/cacheflush.h>
+#ifdef CONFIG_PPC_KUAP
+#include <asm/uaccess.h>
+#endif
 
 /* Whether or not to fill the target memory area with do_nothing(). */
 #define CODE_WRITE	true
@@ -200,6 +203,46 @@ void lkdtm_ACCESS_USERSPACE(void)
 	vm_munmap(user_addr, PAGE_SIZE);
 }
 
+/* Test that KUAP's directional user access unlocks work as intended */
+#ifdef CONFIG_PPC_KUAP
+void lkdtm_ACCESS_USERSPACE_KUAP(void)
+{
+	unsigned long user_addr, tmp = 0;
+	unsigned long *ptr;
+
+	user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
+			    PROT_READ | PROT_WRITE | PROT_EXEC,
+			    MAP_ANONYMOUS | MAP_PRIVATE, 0);
+	if (user_addr >= TASK_SIZE) {
+		pr_warn("Failed to allocate user memory\n");
+		return;
+	}
+
+	if (copy_to_user((void __user *)user_addr, &tmp, sizeof(tmp))) {
+		pr_warn("copy_to_user failed\n");
+		vm_munmap(user_addr, PAGE_SIZE);
+		return;
+	}
+
+	ptr = (unsigned long *)user_addr;
+
+	/* Allowing "write to" should not allow "read from" */
+	allow_write_to_user(ptr, sizeof(unsigned long));
+	pr_info("attempting bad read at %px with write allowed\n", ptr);
+	tmp = *ptr;
+	tmp += 0xc0dec0de;
+	prevent_write_to_user(ptr, sizeof(unsigned long));
+
+	/* Allowing "read from" should not allow "write to" */
+	allow_read_from_user(ptr, sizeof(unsigned long));
+	pr_info("attempting bad write at %px with read allowed\n", ptr);
+	*ptr = tmp;
+	prevent_read_from_user(ptr, sizeof(unsigned long));
+
+	vm_munmap(user_addr, PAGE_SIZE);
+}
+#endif
+
 void lkdtm_ACCESS_NULL(void)
 {
 	unsigned long tmp;
-- 
2.25.0


WARNING: multiple messages have this Message-ID (diff)
From: Russell Currey <ruscur@russell.cc>
To: keescook@chromium.org
Cc: kernel-hardening@lists.openwall.com,
	linux-kernel@vger.kernel.org, Russell Currey <ruscur@russell.cc>,
	linuxppc-dev@lists.ozlabs.org, dja@axtens.net
Subject: [PATCH] lkdtm: Test KUAP directional user access unlocks on powerpc
Date: Fri, 31 Jan 2020 16:31:57 +1100	[thread overview]
Message-ID: <20200131053157.22463-1-ruscur@russell.cc> (raw)

Kernel Userspace Access Prevention (KUAP) on powerpc supports
allowing only one access direction (Read or Write) when allowing access
to or from user memory.

A bug was recently found that showed that these one-way unlocks never
worked, and allowing Read *or* Write would actually unlock Read *and*
Write.  We should have a test case for this so we can make sure this
doesn't happen again.

Like ACCESS_USERSPACE, the correct result is for the test to fault.

At the time of writing this, the upstream kernel still has this bug
present, so the test will allow both accesses whereas ACCESS_USERSPACE
will correctly fault.

Signed-off-by: Russell Currey <ruscur@russell.cc>
---
 drivers/misc/lkdtm/core.c  |  3 +++
 drivers/misc/lkdtm/lkdtm.h |  3 +++
 drivers/misc/lkdtm/perms.c | 43 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+)

diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index ee0d6e721441..baef3c6f48d6 100644
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c
@@ -137,6 +137,9 @@ static const struct crashtype crashtypes[] = {
 	CRASHTYPE(EXEC_USERSPACE),
 	CRASHTYPE(EXEC_NULL),
 	CRASHTYPE(ACCESS_USERSPACE),
+#ifdef CONFIG_PPC_KUAP
+	CRASHTYPE(ACCESS_USERSPACE_KUAP),
+#endif
 	CRASHTYPE(ACCESS_NULL),
 	CRASHTYPE(WRITE_RO),
 	CRASHTYPE(WRITE_RO_AFTER_INIT),
diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index c56d23e37643..406a3fb32e6f 100644
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h
@@ -57,6 +57,9 @@ void lkdtm_EXEC_RODATA(void);
 void lkdtm_EXEC_USERSPACE(void);
 void lkdtm_EXEC_NULL(void);
 void lkdtm_ACCESS_USERSPACE(void);
+#ifdef CONFIG_PPC_KUAP
+void lkdtm_ACCESS_USERSPACE_KUAP(void);
+#endif
 void lkdtm_ACCESS_NULL(void);
 
 /* lkdtm_refcount.c */
diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 62f76d506f04..2c9aa0114333 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -10,6 +10,9 @@
 #include <linux/mman.h>
 #include <linux/uaccess.h>
 #include <asm/cacheflush.h>
+#ifdef CONFIG_PPC_KUAP
+#include <asm/uaccess.h>
+#endif
 
 /* Whether or not to fill the target memory area with do_nothing(). */
 #define CODE_WRITE	true
@@ -200,6 +203,46 @@ void lkdtm_ACCESS_USERSPACE(void)
 	vm_munmap(user_addr, PAGE_SIZE);
 }
 
+/* Test that KUAP's directional user access unlocks work as intended */
+#ifdef CONFIG_PPC_KUAP
+void lkdtm_ACCESS_USERSPACE_KUAP(void)
+{
+	unsigned long user_addr, tmp = 0;
+	unsigned long *ptr;
+
+	user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
+			    PROT_READ | PROT_WRITE | PROT_EXEC,
+			    MAP_ANONYMOUS | MAP_PRIVATE, 0);
+	if (user_addr >= TASK_SIZE) {
+		pr_warn("Failed to allocate user memory\n");
+		return;
+	}
+
+	if (copy_to_user((void __user *)user_addr, &tmp, sizeof(tmp))) {
+		pr_warn("copy_to_user failed\n");
+		vm_munmap(user_addr, PAGE_SIZE);
+		return;
+	}
+
+	ptr = (unsigned long *)user_addr;
+
+	/* Allowing "write to" should not allow "read from" */
+	allow_write_to_user(ptr, sizeof(unsigned long));
+	pr_info("attempting bad read at %px with write allowed\n", ptr);
+	tmp = *ptr;
+	tmp += 0xc0dec0de;
+	prevent_write_to_user(ptr, sizeof(unsigned long));
+
+	/* Allowing "read from" should not allow "write to" */
+	allow_read_from_user(ptr, sizeof(unsigned long));
+	pr_info("attempting bad write at %px with read allowed\n", ptr);
+	*ptr = tmp;
+	prevent_read_from_user(ptr, sizeof(unsigned long));
+
+	vm_munmap(user_addr, PAGE_SIZE);
+}
+#endif
+
 void lkdtm_ACCESS_NULL(void)
 {
 	unsigned long tmp;
-- 
2.25.0


             reply	other threads:[~2020-01-31  5:32 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-31  5:31 Russell Currey [this message]
2020-01-31  5:31 ` [PATCH] lkdtm: Test KUAP directional user access unlocks on powerpc Russell Currey
2020-01-31  6:44 ` Christophe Leroy
2020-01-31  6:44   ` Christophe Leroy
2020-01-31  6:53   ` Russell Currey
2020-01-31  6:53     ` Russell Currey
2020-01-31  6:53     ` Russell Currey
2020-01-31  6:58     ` Christophe Leroy
2020-01-31  6:58       ` Christophe Leroy
2020-01-31  7:01       ` Russell Currey
2020-01-31  7:01         ` Russell Currey
2020-01-31  7:01         ` Russell Currey
2020-02-01 16:40     ` Kees Cook
2020-02-01 16:40       ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200131053157.22463-1-ruscur@russell.cc \
    --to=ruscur@russell.cc \
    --cc=christophe.leroy@c-s.fr \
    --cc=dja@axtens.net \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.