diff mbox

[v5,10/10] lkdtm: Add test for XPFO

Message ID 20170809200755.11234-11-tycho@docker.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tycho Andersen Aug. 9, 2017, 8:07 p.m. UTC 
From: Juerg Haefliger <juerg.haefliger@hpe.com>

This test simply reads from userspace memory via the kernel's linear
map.

hugepages is only supported on x86 right now, hence the ifdef.

Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com>
Signed-off-by: Tycho Andersen <tycho@docker.com>
Tested-by: Marco Benatto <marco.antonio.780@gmail.com>
---
 drivers/misc/Makefile     |  1 +
 drivers/misc/lkdtm.h      |  4 +++
 drivers/misc/lkdtm_core.c |  4 +++
 drivers/misc/lkdtm_xpfo.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 71 insertions(+)

Comments

kernel test robot Aug. 12, 2017, 8:24 p.m. UTC | #1 
Hi Juerg,

[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on v4.13-rc4]
[cannot apply to next-20170811]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Tycho-Andersen/Add-support-for-eXclusive-Page-Frame-Ownership/20170813-035705
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: x86_64-randconfig-x016-201733 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/misc/lkdtm_xpfo.c: In function 'read_user_with_flags':
>> drivers/misc/lkdtm_xpfo.c:31:14: error: implicit declaration of function 'user_virt_to_phys' [-Werror=implicit-function-declaration]
     phys_addr = user_virt_to_phys(user_addr);
                 ^~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/user_virt_to_phys +31 drivers/misc/lkdtm_xpfo.c

    10	
    11	void read_user_with_flags(unsigned long flags)
    12	{
    13		unsigned long user_addr, user_data = 0xdeadbeef;
    14		phys_addr_t phys_addr;
    15		void *virt_addr;
    16	
    17		user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
    18				    PROT_READ | PROT_WRITE | PROT_EXEC,
    19				    flags, 0);
    20		if (user_addr >= TASK_SIZE) {
    21			pr_warn("Failed to allocate user memory\n");
    22			return;
    23		}
    24	
    25		if (copy_to_user((void __user *)user_addr, &user_data,
    26				 sizeof(user_data))) {
    27			pr_warn("copy_to_user failed\n");
    28			goto free_user;
    29		}
    30	
  > 31		phys_addr = user_virt_to_phys(user_addr);
    32		if (!phys_addr) {
    33			pr_warn("Failed to get physical address of user memory\n");
    34			goto free_user;
    35		}
    36	
    37		virt_addr = phys_to_virt(phys_addr);
    38		if (phys_addr != virt_to_phys(virt_addr)) {
    39			pr_warn("Physical address of user memory seems incorrect\n");
    40			goto free_user;
    41		}
    42	
    43		pr_info("Attempting bad read from kernel address %p\n", virt_addr);
    44		if (*(unsigned long *)virt_addr == user_data)
    45			pr_info("Huh? Bad read succeeded?!\n");
    46		else
    47			pr_info("Huh? Bad read didn't fail but data is incorrect?!\n");
    48	
    49	 free_user:
    50		vm_munmap(user_addr, PAGE_SIZE);
    51	}
    52	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot Aug. 12, 2017, 9:05 p.m. UTC | #2 
Hi Juerg,

[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on v4.13-rc4]
[cannot apply to next-20170811]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Tycho-Andersen/Add-support-for-eXclusive-Page-Frame-Ownership/20170813-035705
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=xtensa 

All error/warnings (new ones prefixed by >>):

   drivers//misc/lkdtm_xpfo.c: In function 'read_user_with_flags':
   drivers//misc/lkdtm_xpfo.c:31:2: error: implicit declaration of function 'user_virt_to_phys' [-Werror=implicit-function-declaration]
     phys_addr = user_virt_to_phys(user_addr);
     ^
>> drivers//misc/lkdtm_xpfo.c:37:2: error: implicit declaration of function 'phys_to_virt' [-Werror=implicit-function-declaration]
     virt_addr = phys_to_virt(phys_addr);
     ^
>> drivers//misc/lkdtm_xpfo.c:37:12: warning: assignment makes pointer from integer without a cast
     virt_addr = phys_to_virt(phys_addr);
               ^
>> drivers//misc/lkdtm_xpfo.c:38:2: error: implicit declaration of function 'virt_to_phys' [-Werror=implicit-function-declaration]
     if (phys_addr != virt_to_phys(virt_addr)) {
     ^
   cc1: some warnings being treated as errors

vim +/phys_to_virt +37 drivers//misc/lkdtm_xpfo.c

    10	
    11	void read_user_with_flags(unsigned long flags)
    12	{
    13		unsigned long user_addr, user_data = 0xdeadbeef;
    14		phys_addr_t phys_addr;
    15		void *virt_addr;
    16	
    17		user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
    18				    PROT_READ | PROT_WRITE | PROT_EXEC,
    19				    flags, 0);
    20		if (user_addr >= TASK_SIZE) {
    21			pr_warn("Failed to allocate user memory\n");
    22			return;
    23		}
    24	
    25		if (copy_to_user((void __user *)user_addr, &user_data,
    26				 sizeof(user_data))) {
    27			pr_warn("copy_to_user failed\n");
    28			goto free_user;
    29		}
    30	
  > 31		phys_addr = user_virt_to_phys(user_addr);
    32		if (!phys_addr) {
    33			pr_warn("Failed to get physical address of user memory\n");
    34			goto free_user;
    35		}
    36	
  > 37		virt_addr = phys_to_virt(phys_addr);
  > 38		if (phys_addr != virt_to_phys(virt_addr)) {
    39			pr_warn("Physical address of user memory seems incorrect\n");
    40			goto free_user;
    41		}
    42	
    43		pr_info("Attempting bad read from kernel address %p\n", virt_addr);
    44		if (*(unsigned long *)virt_addr == user_data)
    45			pr_info("Huh? Bad read succeeded?!\n");
    46		else
    47			pr_info("Huh? Bad read didn't fail but data is incorrect?!\n");
    48	
    49	 free_user:
    50		vm_munmap(user_addr, PAGE_SIZE);
    51	}
    52	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Tycho Andersen Aug. 14, 2017, 4:21 p.m. UTC | #3 
On Sun, Aug 13, 2017 at 04:24:23AM +0800, kbuild test robot wrote:
> Hi Juerg,
> 
> [auto build test ERROR on arm64/for-next/core]
> [also build test ERROR on v4.13-rc4]
> [cannot apply to next-20170811]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Tycho-Andersen/Add-support-for-eXclusive-Page-Frame-Ownership/20170813-035705
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
> config: x86_64-randconfig-x016-201733 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
>    drivers/misc/lkdtm_xpfo.c: In function 'read_user_with_flags':
> >> drivers/misc/lkdtm_xpfo.c:31:14: error: implicit declaration of function 'user_virt_to_phys' [-Werror=implicit-function-declaration]
>      phys_addr = user_virt_to_phys(user_addr);
>                  ^~~~~~~~~~~~~~~~~
>    cc1: some warnings being treated as errors

These are both the same error, looks like I forgot a dummy prototype
in the non CONFIG_XPFO case, I'll fix it in the next version.

Tycho
Kees Cook Aug. 14, 2017, 7:10 p.m. UTC | #4 
On Wed, Aug 9, 2017 at 1:07 PM, Tycho Andersen <tycho@docker.com> wrote:
> From: Juerg Haefliger <juerg.haefliger@hpe.com>
>
> This test simply reads from userspace memory via the kernel's linear
> map.
>
> hugepages is only supported on x86 right now, hence the ifdef.

I'd prefer that the #ifdef is handled in the .c file. The result is
that all architectures will have the XPFO_READ_USER_HUGE test, but it
can just fail when not available. This means no changes are needed for
lkdtm in the future and the test provides an actual test of hugepages
coverage.

-Kees
Tycho Andersen Aug. 14, 2017, 8:29 p.m. UTC | #5 
On Mon, Aug 14, 2017 at 12:10:47PM -0700, Kees Cook wrote:
> On Wed, Aug 9, 2017 at 1:07 PM, Tycho Andersen <tycho@docker.com> wrote:
> > From: Juerg Haefliger <juerg.haefliger@hpe.com>
> >
> > This test simply reads from userspace memory via the kernel's linear
> > map.
> >
> > hugepages is only supported on x86 right now, hence the ifdef.
> 
> I'd prefer that the #ifdef is handled in the .c file. The result is
> that all architectures will have the XPFO_READ_USER_HUGE test, but it
> can just fail when not available. This means no changes are needed for
> lkdtm in the future and the test provides an actual test of hugepages
> coverage.

If failing tests is okay, I think we can just drop that hunk entirely.
Everything compiles fine, it just doesn't work :). I'll do that for
the next version.

Tycho
diff mbox

Patch

diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index b0b766416306..8447b42a447d 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -62,6 +62,7 @@  lkdtm-$(CONFIG_LKDTM)		+= lkdtm_heap.o
 lkdtm-$(CONFIG_LKDTM)		+= lkdtm_perms.o
 lkdtm-$(CONFIG_LKDTM)		+= lkdtm_rodata_objcopy.o
 lkdtm-$(CONFIG_LKDTM)		+= lkdtm_usercopy.o
+lkdtm-$(CONFIG_LKDTM)		+= lkdtm_xpfo.o
 
 KCOV_INSTRUMENT_lkdtm_rodata.o	:= n
 
diff --git a/drivers/misc/lkdtm.h b/drivers/misc/lkdtm.h
index 3b4976396ec4..fc53546113c1 100644
--- a/drivers/misc/lkdtm.h
+++ b/drivers/misc/lkdtm.h
@@ -64,4 +64,8 @@  void lkdtm_USERCOPY_STACK_FRAME_FROM(void);
 void lkdtm_USERCOPY_STACK_BEYOND(void);
 void lkdtm_USERCOPY_KERNEL(void);
 
+/* lkdtm_xpfo.c */
+void lkdtm_XPFO_READ_USER(void);
+void lkdtm_XPFO_READ_USER_HUGE(void);
+
 #endif
diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c
index 42d2b8e31e6b..9431f80886bc 100644
--- a/drivers/misc/lkdtm_core.c
+++ b/drivers/misc/lkdtm_core.c
@@ -235,6 +235,10 @@  struct crashtype crashtypes[] = {
 	CRASHTYPE(USERCOPY_STACK_FRAME_FROM),
 	CRASHTYPE(USERCOPY_STACK_BEYOND),
 	CRASHTYPE(USERCOPY_KERNEL),
+	CRASHTYPE(XPFO_READ_USER),
+#ifdef CONFIG_X86
+	CRASHTYPE(XPFO_READ_USER_HUGE),
+#endif
 };
 
 
diff --git a/drivers/misc/lkdtm_xpfo.c b/drivers/misc/lkdtm_xpfo.c
new file mode 100644
index 000000000000..c72509128eb3
--- /dev/null
+++ b/drivers/misc/lkdtm_xpfo.c
@@ -0,0 +1,62 @@ 
+/*
+ * This is for all the tests related to XPFO (eXclusive Page Frame Ownership).
+ */
+
+#include "lkdtm.h"
+
+#include <linux/mman.h>
+#include <linux/uaccess.h>
+#include <linux/xpfo.h>
+
+void read_user_with_flags(unsigned long flags)
+{
+	unsigned long user_addr, user_data = 0xdeadbeef;
+	phys_addr_t phys_addr;
+	void *virt_addr;
+
+	user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
+			    PROT_READ | PROT_WRITE | PROT_EXEC,
+			    flags, 0);
+	if (user_addr >= TASK_SIZE) {
+		pr_warn("Failed to allocate user memory\n");
+		return;
+	}
+
+	if (copy_to_user((void __user *)user_addr, &user_data,
+			 sizeof(user_data))) {
+		pr_warn("copy_to_user failed\n");
+		goto free_user;
+	}
+
+	phys_addr = user_virt_to_phys(user_addr);
+	if (!phys_addr) {
+		pr_warn("Failed to get physical address of user memory\n");
+		goto free_user;
+	}
+
+	virt_addr = phys_to_virt(phys_addr);
+	if (phys_addr != virt_to_phys(virt_addr)) {
+		pr_warn("Physical address of user memory seems incorrect\n");
+		goto free_user;
+	}
+
+	pr_info("Attempting bad read from kernel address %p\n", virt_addr);
+	if (*(unsigned long *)virt_addr == user_data)
+		pr_info("Huh? Bad read succeeded?!\n");
+	else
+		pr_info("Huh? Bad read didn't fail but data is incorrect?!\n");
+
+ free_user:
+	vm_munmap(user_addr, PAGE_SIZE);
+}
+
+/* Read from userspace via the kernel's linear map. */
+void lkdtm_XPFO_READ_USER(void)
+{
+	read_user_with_flags(MAP_PRIVATE | MAP_ANONYMOUS);
+}
+
+void lkdtm_XPFO_READ_USER_HUGE(void)
+{
+	read_user_with_flags(MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB);
+}