linux-kernel - [PATCH] lkdtm: Do not use flush_icache

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:   Tue, 1 Nov 2016 14:43:25 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Catalin Marinas <catalin.marinas@....com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Arnd Bergmann <arnd@...db.de>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Michal Hocko <mhocko@...e.com>,
        Jerome Marchand <jmarchan@...hat.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        Lorenzo Stoakes <lstoakes@...il.com>,
        Dan Williams <dan.j.williams@...el.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Jan Kara <jack@...e.cz>, Ingo Molnar <mingo@...nel.org>,
        Andrey Ryabinin <aryabinin@...tuozzo.com>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: [PATCH] lkdtm: Do not use flush_icache_range() on user addresses

From: Catalin Marinas <catalin.marinas@....com>

The flush_icache_range() API is meant to be used on kernel addresses
only as it may not have the infrastructure (exception entries) to handle
user memory faults.

The lkdtm execute_user_location() function tests the kernel execution of
user space addresses by mmap'ing an anonymous page, copying some code
together with cache maintenance and attempting to run it. However, the
cache maintenance step may fail because of the incorrect API usage
described above. The patch changes lkdtm to use access_process_vm() for
copying the code into user space which would take care of the necessary
cache maintenance.

Signed-off-by: Catalin Marinas <catalin.marinas@....com>
[kees: export access_process_vm() for module use]
Signed-off-by: Kees Cook <keescook@...omium.org>
---
Since this now adds an export, we should probably delay this to v4.10.
(And I've fixed my build tests to actually do CONFIG_LKDTM=m instead of
lying to me.)
---
 drivers/misc/lkdtm_perms.c | 7 +++++--
 mm/memory.c                | 1 +
 mm/nommu.c                 | 1 +
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/lkdtm_perms.c b/drivers/misc/lkdtm_perms.c
index 45f1c0f96612..c7635a79341f 100644
--- a/drivers/misc/lkdtm_perms.c
+++ b/drivers/misc/lkdtm_perms.c
@@ -60,15 +60,18 @@ static noinline void execute_location(void *dst, bool write)
 
 static void execute_user_location(void *dst)
 {
+	int copied;
+
 	/* Intentionally crossing kernel/user memory boundary. */
 	void (*func)(void) = dst;
 
 	pr_info("attempting ok execution at %p\n", do_nothing);
 	do_nothing();
 
-	if (copy_to_user((void __user *)dst, do_nothing, EXEC_SIZE))
+	copied = access_process_vm(current, (unsigned long)dst, do_nothing,
+				   EXEC_SIZE, FOLL_WRITE);
+	if (copied < EXEC_SIZE)
 		return;
-	flush_icache_range((unsigned long)dst, (unsigned long)dst + EXEC_SIZE);
 	pr_info("attempting bad execution at %p\n", func);
 	func();
 }
diff --git a/mm/memory.c b/mm/memory.c
index e18c57bdc75c..485f12d8ad5c 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3966,6 +3966,7 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr,
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(access_process_vm);
 
 /*
  * Print the name of a VMA.
diff --git a/mm/nommu.c b/mm/nommu.c
index db5fd1795298..0990145054b5 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -1878,6 +1878,7 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, in
 	mmput(mm);
 	return len;
 }
+EXPORT_SYMBOL_GPL(access_process_vm);
 
 /**
  * nommu_shrink_inode_mappings - Shrink the shared mappings on an inode
-- 
2.7.4


-- 
Kees Cook
Nexus Security