From 8ca81822fffb2536aa1076fb17d1ea1413df3e7f Mon Sep 17 00:00:00 2001 From: Ethan Zhao Date: Fri, 29 Sep 2017 10:50:35 +0800 Subject: [PATCH] drivers/char/mem.c: deny access in open operation when securelevel is set Orabug: 26943864 There is still a secure hole left in mem.c driver -- when securelevel is set userland application could access PCI configuration space via this driver. -- Attempting to write via mmap() API using acc_test. [root@ban92uut054 ~]# ./acc_test mmap 0x846000c4=0x1 Using mmap() API for access mmap write wrote 0x1 [root@ban92uut054 ~]# setpci -s 46:00.0 0xc4.l 00000001 How, write 0x0 to offset 0xc4 [root@ban92uut054 ~]# ./acc_test mmap 0x846000c4=0x0 Using mmap() API for access mmap write wrote 0x0 [root@ban92uut054 ~]# setpci -s 46:00.0 0xc4.l 00000000 -- source code of acc_test program: main(int argc, char *argv[]) { int fd; int retval; int val; off_t addr; char *tmp; int operation = 0; //read int access_type = -1; off_t page_base = 0; off_t page_offset = 0; off_t pagesize = sysconf(_SC_PAGE_SIZE); char *mem; int prot; if (argc < 3) { printf("Insufficient args: acc_test rw|mmap [-w ]\n"); return -1; } if (strcmp("rw", argv[1]) == 0) { access_type = 1; printf("Using pread()/pwrite() API for access\n"); } else if (strcmp("mmap", argv[1]) == 0) { access_type = 2; printf("Using mmap() API for access\n"); } else { printf("Illegal access type: must be rw or mmap\n"); return -1; } addr = strtoul(argv[2], &tmp, 16); if ((tmp && (*tmp != '=')) && ((*tmp != '\0') || (errno == EINVAL) || (addr == ULONG_MAX && errno == ERANGE))) { fprintf(stderr, "Invalid address specified; must be hex based\n"); if (errno) perror("error : "); exit(1); } else if (tmp && (*tmp == '=')) { // write case tmp++; val = strtoul(tmp, NULL, 16); operation = 1; } //fd = open("/sys/bus/pci/devices/0000:46:00.0/config",O_RDWR | O_SYNC); //retval = pread(fd, &val, 4, 0xc4); if (operation == 1) fd = open("/dev/mem",O_RDWR); else fd = open("/dev/mem",O_RDONLY); if (fd < 0) { perror("open failed"); exit(1); } switch (access_type) { case 1 : // pread/pwrite API if (!operation) { if (pread(fd, &val, 4, addr) < 0) { perror("pread failed"); return -1; } else printf("pread returned 0x%x from 0x%x\n",val,addr); } else { if (pwrite(fd, &val, 4, addr) < 0) { perror ("pwrite failed"); return -1; } printf("pwrite() wrote 0x%x to 0x%x\n",val,addr); } break; case 2 : // mmap API page_base = (addr / pagesize) * pagesize; page_offset = addr - page_base; prot = PROT_READ; if (operation) prot |= PROT_WRITE; mem = mmap(NULL, page_offset + 4, prot, MAP_SHARED, fd, page_base); if (mem == MAP_FAILED) { perror("can't mmap"); return -1; } if (!operation) printf("mmap read returned 0x%x\n",*(uint32_t *)&mem[page_offset]); else { *(uint32_t *)&mem[page_offset] = (uint32_t)val; printf("mmap write wrote 0x%x\n",val); } break; default : printf("Illegal access mode\n"); return -1; } close(fd); } -- This patch is purposed to fix this hole when securelevel is set where one could write to /dev/mem via the mmap() API. The fix to disallow opening /dev/mem or /dev/kmem for access. The fix checks access at open rather than have get_securelevel() called at the various write/read locations. This issue is specific to UEK4 ! Signed-off-by: James Puthukattukaran Signed-off-by: Ethan Zhao Reviewed-by: Eric Snowberg Reviewed-by: Khalid Aziz --- drivers/char/mem.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/drivers/char/mem.c b/drivers/char/mem.c index fff12a61d8cb..5ffb67599431 100644 --- a/drivers/char/mem.c +++ b/drivers/char/mem.c @@ -183,9 +183,6 @@ static ssize_t write_mem(struct file *file, const char __user *buf, if (p != *ppos) return -EFBIG; - if (get_securelevel() > 0) - return -EPERM; - if (!valid_phys_addr_range(p, count)) return -EFAULT; @@ -538,9 +535,6 @@ static ssize_t write_kmem(struct file *file, const char __user *buf, char *kbuf; /* k-addr because vwrite() takes vmlist_lock rwlock */ int err = 0; - if (get_securelevel() > 0) - return -EPERM; - if (p < (unsigned long) high_memory) { unsigned long to_write = min_t(unsigned long, count, (unsigned long)high_memory - p); @@ -588,9 +582,6 @@ static ssize_t read_port(struct file *file, char __user *buf, unsigned long i = *ppos; char __user *tmp = buf; - if (get_securelevel() > 0) - return -EPERM; - if (!access_ok(VERIFY_WRITE, buf, count)) return -EFAULT; while (count-- > 0 && i < 65536) { @@ -609,9 +600,6 @@ static ssize_t write_port(struct file *file, const char __user *buf, unsigned long i = *ppos; const char __user *tmp = buf; - if (get_securelevel() > 0) - return -EPERM; - if (!access_ok(VERIFY_READ, buf, count)) return -EFAULT; while (count-- > 0 && i < 65536) { @@ -747,6 +735,9 @@ static loff_t memory_lseek(struct file *file, loff_t offset, int orig) static int open_port(struct inode *inode, struct file *filp) { + if (get_securelevel() > 0) + return -EPERM; + return capable(CAP_SYS_RAWIO) ? 0 : -EPERM; } -- 2.50.1