keystone-enclave / linux-keystone-driver Goto Github PK

View Code? Open in Web Editor NEW

18.0 18.0 17.0 97 KB

Loadable Module for Keystone Enclave

License: Other

Makefile 1.74% C 98.26%

linux-keystone-driver's People

Contributors

Stargazers

Watchers

Forkers

thaumicmekanism dayeol dkohlbre pramodsu eddyim nirjharroy acidburn0zzz supriya363 nuclei-software a4lg gina-stoffel zhang-fa eobard26 oasisunofficial nanamiiiii sichunqin

linux-keystone-driver's Issues

Check length field in keystone_ioctl()

Hi,

First of all many thanks for your work on keystone, seems like a really nice project.

There is a bug in keystone_ioctl() here: https://github.com/keystone-enclave/linux-keystone-driver/blob/master/keystone-ioctl.c#L187

More precisely, the buffer on the stack (data) is 256 byte, but _IOC_SIZE(cmd) can be up to 2^14 - 1. For example, calling the ioctl handler from user mode as follows:

char data[1024 * 4] = {0};
unsigned int special_command = _MY_IOR(KEYSTONE_IOC_MAGIC, 0, sizeof(data));
ret = ioctl(fd, special_command, data);

leads to the following kernel warning

[   69.540187] ------------[ cut here ]------------
[   69.540966] WARNING: CPU: 0 PID: 61 at
./include/linux/thread_info.h:127 keystone_ioctl+0x14a/0x156
[keystone_driver]
[   69.542556] Buffer overflow detected (256 < 4096)!
[   69.543393] Modules linked in: keystone_driver(O)
[   69.544894] CPU: 0 PID: 61 Comm: enclave-host-du Tainted: G        W
 O     4.15.0-00060-g65e9297 #1
[   69.546849] Call Trace:
[   69.547396] [<        (ptrval)>] walk_stackframe+0x0/0xa2
[   69.548281] [<        (ptrval)>] show_stack+0x26/0x34
[   69.549088] [<        (ptrval)>] dump_stack+0x20/0x2c
[   69.549895] [<        (ptrval)>] __warn+0xcc/0xe2
[   69.550703] [<        (ptrval)>] warn_slowpath_fmt+0x2c/0x38
[   69.551658] [<        (ptrval)>] keystone_ioctl+0x146/0x156
[keystone_driver]
[   69.552821] [<        (ptrval)>] do_vfs_ioctl+0x76/0x4da
[   69.556999] [<        (ptrval)>] SyS_ioctl+0x36/0x60
[   69.557502] [<        (ptrval)>] check_syscall_nr+0x1e/0x22
[   69.558541] ---[ end trace 2ea6f7bf4c8e6535 ]---

Luckily this kernel is quite recent and actually size checks all copy operations to statically-sized buffers in kernel modules, so this is not exploitable.

I would still suggest fixing this with a size check in keystone_ioctl() to avoid relying on the kernel catching this, e.g. if someone ports keystone to an older kernel or so.

*filep can be NULL when used on ioctl

int keystone_create_enclave(struct file *filep, unsigned long arg)
{
  /* create parameters */
  struct keystone_ioctl_create_enclave *enclp = (struct keystone_ioctl_create_enclave *) arg;

  struct enclave *enclave;
  enclave = create_enclave(enclp->min_pages);

  if (enclave == NULL) {
    return -ENOMEM;
  }

  /* allocate UID */
  enclp->eid = enclave_idr_alloc(enclave);

  filep->private_data = (void *) enclp->eid;

  return 0;
}


// a null check would be needed for *fp just to protect against any possible error.

// if (!filep){
  keystone_err("no filep");
}

Large static allocations cause pagefaults

A simple static allocation in an enclave application
unsigned char large_array[1024];
works fine, and can be accessed.

A large allocation:
unsigned char large_array[1024 * 16];
will result in a runtime pagefault on ANY access to it.

Possibly related to #2 , I suspect the region is not actually being created at all.

Occurs on both qemu and board.

[Keystone Page Table] elf sh_flags to pgprot_t when initializing pgtable

When the Keystone driver initializes enclave page table,
it allocates page table entries for pages in each elf section with a same set of permissions (e.g., RWX or RW)
We need to modify it to use different permission bits based on the sh_flags in the section header.

PFNs Busy

https://imgur.com/C6XGTgy Got this error once while working on the multithreading bugs. Not sure how to reproduce though I have been launching the test script twice at the same time. I believe I have done this multiple times but have never seen this error before.

NULL pointer is freed + Lack of null checks.

{
  struct epm* epm;
  struct utm* utm;
  if (enclave == NULL)
    return -ENOSYS;

  epm = enclave->epm;
  utm = enclave->utm;        <------------  //  may be NULL, lacking both checks

  if (epm)
  {
    epm_destroy(epm);
    kfree(epm);
  }
  if (utm)
  {
    utm_destroy(utm); 
    kfree(utm);                          <----------- // utm is freed
  }
  kfree(enclave);
  return 0;
}```

-------------

struct enclave* create_enclave(unsigned long min_pages)
{
  struct enclave* enclave;

  enclave = kmalloc(sizeof(struct enclave), GFP_KERNEL);
  if (!enclave){
    keystone_err("failed to allocate enclave struct\n");
    goto error_no_free;
  }

  enclave->utm = NULL;                   <------------ // asserted as NULL
  enclave->close_on_pexit = 1;

  enclave->epm = kmalloc(sizeof(struct epm), GFP_KERNEL);
  if (!enclave->epm)
  {
    keystone_err("failed to allocate epm\n");
    goto error_destroy_enclave;
  }

  if(epm_init(enclave->epm, min_pages)) {
    keystone_err("failed to initialize epm\n");
    goto error_destroy_enclave;
  }
  return enclave;

 error_destroy_enclave:
  destroy_enclave(enclave);                              <------------------ //  freeing  NULL pointer `utm`
 error_no_free:
  return NULL;
}

Integer overflow


int keystone_create_enclave(struct file *filep, unsigned long arg)
{
  /* create parameters */
  struct keystone_ioctl_create_enclave *enclp = (struct keystone_ioctl_create_enclave *) arg;

  struct enclave *enclave;
  enclave = create_enclave(enclp->min_pages);      <---------- // there's no boundary check for enclp->min_pages value on create_enclave().

  if (enclave == NULL) {
    return -ENOMEM;
  }

  /* allocate UID */
  enclp->eid = enclave_idr_alloc(enclave);

  filep->private_data = (void *) enclp->eid;

  return 0;
}




struct enclave* create_enclave(unsigned long min_pages)
{
  struct enclave* enclave;

  enclave = kmalloc(sizeof(struct enclave), GFP_KERNEL);
  if (!enclave){
    keystone_err("failed to allocate enclave struct\n");
    goto error_no_free;
  }

  enclave->utm = NULL;
  enclave->close_on_pexit = 1;

  enclave->epm = kmalloc(sizeof(struct epm), GFP_KERNEL);
  if (!enclave->epm)
  {
    keystone_err("failed to allocate epm\n");
    goto error_destroy_enclave;
  }

  if(epm_init(enclave->epm, min_pages)) {
    keystone_err("failed to initialize epm\n");
    goto error_destroy_enclave;
  }
  return enclave;

 error_destroy_enclave:
  destroy_enclave(enclave);
 error_no_free:
  return NULL;
}

why untrusted shared memory must be from the buddy allocator?

hello,we want to know why untrusted shared memory must be from the buddy allocator, which results in that the data transmitted at one time must be less than or equal to 4MB, and the large-size data that needs to be processed by the eapp must be split for multiple transmission without any sdk support.
Thank you!
hupeng

double-free on *epm - UAF ?

int epm_destroy(struct epm* epm) {

  /* Clean anything in the free list */
  epm_clean_free_list(epm);         <---------------- free every single  epm struct on the list

  if(!epm->ptr || !epm->size)
    return 0;                                <------------ if ->ptr and ->size is definitely unint we will break/return here.

  /* free the EPM hold by the enclave */
  if (epm->is_cma) {
    dma_free_coherent(keystone_dev.this_device,
        epm->size,
        (void*) epm->ptr,
        epm->pa);
  } else {
    free_pages(epm->ptr, epm->order);
  }

  return 0;
}



int destroy_enclave(struct enclave* enclave)
{
  struct epm* epm;
  struct utm* utm;
  if (enclave == NULL)
    return -ENOSYS;

  epm = enclave->epm;
  utm = enclave->utm;

  if (epm)
  {
    epm_destroy(epm);             <----------------- at this point we have freed all epm properties from the list...
    kfree(epm);        <------------ we're freeing them again....
  } 
  if (utm)
  {
    utm_destroy(utm);
    kfree(utm);
  }
  kfree(enclave);
  return 0;
}

keystone_finalize_enclave produces segfault

Hey, I observed some weird behavior while running the following code, which basically executes ioctl(open("/dev/keystone_enclave"), KEYSTONE_IOC_FINALIZE_ENCLAVE, {0x1000,0x400,0x80000000})

// autogenerated by syzkaller (https://github.com/google/syzkaller)

#define _GNU_SOURCE

#include <endian.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <unistd.h>

#ifndef __NR_ioctl
#define __NR_ioctl 29
#endif
#ifndef __NR_mmap
#define __NR_mmap 222
#endif
#ifndef __NR_openat
#define __NR_openat 56
#endif

uint64_t r[2] = {0xffffffffffffffff, 0xffffffffffffffff};

int crash(void)
{
  syscall(__NR_mmap, 0x1ffff000ul, 0x1000ul, 0ul, 0x32ul, -1, 0ul);
  syscall(__NR_mmap, 0x20000000ul, 0x1000000ul, 7ul, 0x32ul, -1, 0ul);
  syscall(__NR_mmap, 0x21000000ul, 0x1000ul, 0ul, 0x32ul, -1, 0ul);
  intptr_t res = 0;
  memcpy((void*)0x20000040, "/dev/keystone_enclave\000", 22);
  res = syscall(__NR_openat, 0xffffffffffffff9cul, 0x20000040ul, 0ul, 0ul);
  if (res != -1) 
    r[0] = res;
  *(uint64_t*)0x20000000 = 0;
  *(uint64_t*)0x20000008 = 3;
  *(uint64_t*)0x20000010 = 0;
  syscall(__NR_ioctl, r[0], 0x8088a400, 0x20000000ul);
  memcpy((void*)0x200000c0, "/dev/keystone_enclave\000", 22);
  res = syscall(__NR_openat, 0xffffffffffffff9cul, 0x200000c0ul, 0ul, 0ul);
  if (res != -1) 
    r[1] = res;
  *(uint64_t*)0x20000100 = 0x1000;
  *(uint64_t*)0x20000108 = 0x400;
  *(uint64_t*)0x20000110 = 0x80000000;


  syscall(__NR_ioctl, r[1], 0x8088a406, 0x20000100ul);
  return 0;
}
int main(void){
        crash();
}

It seems there are four different outcomes. I didn't manage to determine when which error occurs.

Kernel Oops / SegFault (Unable to handle kernel paging request)
"expected" behavior: sbi is throwing two error messages
Kernel Oops / SegFault (Unable to handle kernel paging request) which results in a recursive fault and a complete system crash
Kernel Oops / SegFault (Unable to handle kernel NULL pointer dereference). If the error occurs once, it occurs in every execution of the code above.

I think the bug might be related to #44

Several safety bugs

We've got a few known cases where the driver crashes the machine. We need to clean these up.

SBI call failure (something went wrong in enclave create):

# ./tests/tests.ke 
Verifying archive integrity... All good.
Uncompressing Keystone vault archive
testing stack
[   23.594192] keystone_enclave: keystone_create_enclave: SBI call failed
[   23.607449] Oops - store (or AMO) access fault [#1]
[   23.611540] Modules linked in: keystone_driver(O)
[   23.616234] CPU: 4 PID: 193 Comm: test-runner.ris Tainted: G        W  O     4.15.0-00060-g65e929792fb9-dirty #4
[   23.626389] sepc: ffffffe000fcc2f4 ra : ffffffe000d47410 sp : ffffffe1b6363da0
[   23.633595]  gp : ffffffe00110ccb8 tp : ffffffe1b6273300 t0 : ffffffe1bbc29000
[   23.640801]  t1 : ffffffe000fd09f0 t2 : ffffffe000fd0a70 s0 : ffffffe1b6363e50
[   23.648007]  s1 : ffffffe00110ce48 a0 : ffffffe1bbc29000 a1 : 0000000000000000
[   23.655213]  a2 : 0000000000001000 a3 : ffffffe1bbc2a000 a4 : 0000000000000000
[   23.662419]  a5 : 000000023be29000 a6 : 0000000000000001 a7 : 00000000014200ca
[   23.669626]  s2 : ffffffe00110ce50 s3 : ffffffe1b620e8a0 s4 : ffffffe1fef198f8
[   23.676832]  s5 : 6db6db6db6db6db7 s6 : ffffffe00110d560 s7 : ffffffe1b7c1f120
[   23.684037]  s8 : 000000000000000d s9 : 0000000000000001 s10: 000000000000000f
[   23.691244]  s11: 0000000000000055 t3 : 000000000004357c t4 : 0000000000000002
[   23.698449]  t5 : 000000200022d474 t6 : 0000000000000000
[   23.703747] sstatus: 0000000200000120 sbadaddr: ffffffe1bbc29000 scause: 0000000000000007
[   23.711955] ---[ end trace 66814e3a8c80ec12 ]---
/home/dkohlbre/keystone/keystone/riscv-pk/machine/mtrap.c:22: machine mode: unhandlable trap 5 @ 0x0000000080007a86
                                                                                                                   Power off

Bad paging/addrs into ioctls:

[   59.251885] Unable to handle kernel paging request at virtual address ffffffdf89e01000
[   59.259033] Oops [#1]
[   59.261276] Modules linked in: keystone_driver(O)
[   59.265971] CPU: 2 PID: 196 Comm: test-runner.ris Tainted: G        W  O     4.15.0-00060-g65e929792fb9-dirty #5
[   59.276126] sepc: ffffffd004038464 ra : ffffffd004038546 sp : ffffffe1b62adb30
[   59.283332]  gp : ffffffe00110acb8 tp : ffffffe1b7be2a80 t0 : ffffffd004038ce8
[   59.290537]  t1 : ffffffe000fce716 t2 : 0000000000200000 s0 : ffffffe1b62adb40
[   59.297744]  s1 : ffffffe1b62adbd0 a0 : ffffffe1b6240640 a1 : ffffffffc0000000
[   59.304950]  a2 : ffffffdf7fe00000 a3 : 000000000a001000 a4 : ffffffdf89e01000
[   59.312156]  a5 : ffffffe00110ae50 a6 : ffffffe1b7635b60 a7 : ffffffe1b7b88a80
[   59.319362]  s2 : ffffffe1b7b88a80 s3 : ffffffe00110ae50 s4 : 0000003fffc1f860
[   59.326569]  s5 : ffffffe1b759b600 s6 : 0000002000135720 s7 : 00000000000bb000
[   59.333774]  s8 : 0000000000000000 s9 : 00000000000bacb0 s10: 0000000000000000
[   59.340981]  s11: 0000003fff864f96 t3 : 0000000000000040 t4 : 0000000000000000
[   59.348186]  t5 : 0000000000000000 t6 : 0000000000040000
[   59.353484] sstatus: 0000000200000120 sbadaddr: ffffffdf89e01000 scause: 000000000000000d
[   59.361689] ---[ end trace 66814e3a8c80ec12 ]---
/home/dkohlbre/keystone/keystone/riscv-pk/machine/mtrap.c:22: machine mode: unhandlable trap 5 @ 0x0000000080007a86
                                                                                                                   Power off```

Off-by-One

long keystone_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
{
  long ret;
  char data[256];
  size_t ioc_size;

  if (!arg)
    return -EINVAL;

  ioc_size = _IOC_SIZE(cmd);
  ioc_size = ioc_size > sizeof(data) ? sizeof(data) : ioc_size;        <-------- // dont let ioc_size be == sizeof(data)

  if (copy_from_user(data,(void __user *) arg, ioc_size))    <------------ // (a)
    return -EFAULT;
  [...]



// (a) - as we're dealing with possible strings a null termination would be needed if __user *arg == sizeof(data)
// specially coming from the userspace; off-by-one may occur

PAGE_UP marco missing in kernel version 5.14 and later

Describe the bug
linux-keystone-driver version: v1.0.0
riscv64-unknown-linux-gnu-gcc version: 10.2.0

As of kernel version 5.14, the marco function PAGE_UP is no longer included.
Commit: riscv: Cleanup unused functions

Error Log

/work/linux-keystone-driver/keystone-page.c:93:16: error: implicit declaration of function 'PAGE_UP' [-Werror=implicit-function-declaration]
   93 |   req_pages += PAGE_UP(untrusted_size)/PAGE_SIZE;
      |                ^~~~~~~

Workaround
I added the PAGE_UP function to riscv64.h

diff --git a/riscv64.h b/riscv64.h
index cbbdf42..61872f3 100644
--- a/riscv64.h
+++ b/riscv64.h
@@ -42,6 +42,8 @@
 
 #define PTE_TABLE(PTE) (((PTE) & (PTE_V | PTE_R | PTE_W | PTE_X)) == PTE_V)
 
+#define PAGE_UP(addr)  (((addr)+((PAGE_SIZE)-1))&(~((PAGE_SIZE)-1)))
+
 #define MSTATUS_SD MSTATUS64_SD
 #define SSTATUS_SD SSTATUS64_SD
 #define RISCV_PGLEVEL_BITS 9

Possible double-free?

vaddr_t get_free_page(struct list_head* pg_list)
{

	<---------------- (a) //  null checks should be needed for pg_list


  struct free_page* page;
  vaddr_t addr;

  if(list_empty(pg_list))
    return 0;

  page = list_first_entry(pg_list, struct free_page, freelist);  <--------- (b) //    this probably can return an unexpected/undefined value
  addr = page->vaddr;
  list_del(&page->freelist);
  kfree(page);                  <--------- //  because of (a), (b), page may never be allocated and 
                                                              therefore happen a some sort of 	double free?

  return addr;
}