From 0c34bfe555baa9b2be97d402d23b4f420054d65e Mon Sep 17 00:00:00 2001 From: Brian Zhu Date: Wed, 25 Apr 2018 01:20:05 +0800 Subject: [PATCH 1/2] codec: fix the mmap security issue fail to do bounds-check the "offset" value or the map's size, may result in security issue. Signed-off-by: Brian Zhu Signed-off-by: Yixun Lan --- drivers/amlogic/amports/encoder.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/drivers/amlogic/amports/encoder.c b/drivers/amlogic/amports/encoder.c index 12e982d091a0..20c2cee785b9 100644 --- a/drivers/amlogic/amports/encoder.c +++ b/drivers/amlogic/amports/encoder.c @@ -3139,15 +3139,27 @@ static s32 avc_mmap(struct file *filp, struct vm_area_struct *vma) ulong off = vma->vm_pgoff << PAGE_SHIFT; ulong vma_size = vma->vm_end - vma->vm_start; - if (vma_size == 0) { - enc_pr(LOG_ERROR, "vma_size is 0, wq:%p.\n", (void *)wq); + if ((vma_size == 0) || !wq) { + enc_pr(LOG_ERROR, "vma_size is 0x%lx or wq:%p.\n", + vma_size, (void *)wq); return -EAGAIN; } + if (!off) - off += wq->mem.buf_start; + off = wq->mem.buf_start; + + if ((off + vma_size) > (wq->mem.buf_start + wq->mem.buf_size) + || (off < wq->mem.buf_start)) { + enc_pr(LOG_ERROR, + "vma_size:0x%lx, off:0x%lx, buffsize:0x%x, wq:%p.\n", + vma_size, off, wq->mem.buf_size, (void *)wq); + return -EAGAIN; + } + enc_pr(LOG_ALL, - "vma_size is %ld , off is %ld, wq:%p.\n", - vma_size , off, (void *)wq); + "vm_pgoff:0x%lx, size:0x%lx, buffstart:0x%x, wq:%p.\n", + (vma->vm_pgoff << PAGE_SHIFT), vma_size, + wq->mem.buf_start, (void *)wq); vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP | VM_IO; /* vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); */ if (remap_pfn_range(vma, vma->vm_start, off >> PAGE_SHIFT, From 1c0e875b6ea41206a8b65f735c060d78fa99878e Mon Sep 17 00:00:00 2001 From: Brian Zhu Date: Wed, 25 Apr 2018 15:10:38 +0000 Subject: [PATCH 2/2] codecs: drop mmap in jpegdec the mmap is not really been used, drop it for now. It protentially have write primitive which would allow userspace process to write to any physical address, which is a security issue. Signed-off-by: Brian Zhu Signed-off-by: Yixun Lan --- drivers/amlogic/amports/jpegdec.c | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/drivers/amlogic/amports/jpegdec.c b/drivers/amlogic/amports/jpegdec.c index 06f0ee6c5a6b..5244aea00274 100644 --- a/drivers/amlogic/amports/jpegdec.c +++ b/drivers/amlogic/amports/jpegdec.c @@ -592,27 +592,6 @@ static long amjpegdec_ioctl(struct file *file, unsigned int cmd, ulong arg) return r; } -static int mmap(struct file *filp, struct vm_area_struct *vma) -{ - unsigned long off = vma->vm_pgoff << PAGE_SHIFT; - unsigned vm_size = vma->vm_end - vma->vm_start; - - if (vm_size == 0) - return -EAGAIN; - /* pr_info("mmap:%x\n",vm_size); */ - off += jegdec_mem_info.canv_addr; - - vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP | VM_IO; - - if (remap_pfn_range(vma, vma->vm_start, off >> PAGE_SHIFT, - vma->vm_end - vma->vm_start, vma->vm_page_prot)) { - pr_info("set_cached: failed remap_pfn_range\n"); - return -EAGAIN; - } - return 0; - -} - #ifdef CONFIG_COMPAT static long amjpegdec_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long args) @@ -629,7 +608,6 @@ static long amjpegdec_compat_ioctl(struct file *filp, static const struct file_operations amjpegdec_fops = { .owner = THIS_MODULE, .open = amjpegdec_open, - .mmap = mmap, .release = amjpegdec_release, .unlocked_ioctl = amjpegdec_ioctl, #ifdef CONFIG_COMPAT