On 2019-01-03 20:40, Michal Hocko wrote:
On Thu 03-01-19 20:27:26, Roman Penyaev wrote:
On 2019-01-03 16:13, Michal Hocko wrote:
> On Thu 03-01-19 15:59:52, Roman Penyaev wrote:
> > area->size can include adjacent guard page but get_vm_area_size()
> > returns actual size of the area.
> >
> > This fixes possible kernel crash when userspace tries to map area
> > on 1 page bigger: size check passes but the following
> > vmalloc_to_page()
> > returns NULL on last guard (non-existing) page.
>
> Can this actually happen? I am not really familiar with all the callers
> of this API but VM_NO_GUARD is not really used wildly in the kernel.
Exactly, by default (VM_NO_GUARD is not set) each area has guard page,
thus the area->size will be bigger. The bug is not reproduced if
VM_NO_GUARD is set.
> All I can see is kasan na arm64 which doesn't really seem to use it
> for vmalloc.
>
> So is the problem real or this is a mere cleanup?
This is the real problem, try this hunk for any file descriptor which
provides
mapping, or say modify epoll as example:
OK, my response was more confusing than I intended. I meant to say. Is
there any in kernel code that would allow the bug have had in mind?
In other words can userspace trick any existing code?
In theory any existing caller of remap_vmalloc_range() which does
not have an explicit size check should trigger an oops, e.g. this is
a good candidate:
*** drivers/media/usb/stkwebcam/stk-webcam.c:
v4l_stk_mmap[789] ret = remap_vmalloc_range(vma,
sbuf->buffer, 0);
According to the code no explicit size check, should be easy to
reproduce:
mmap the frame buffer and you are done.
Other callers are not so easy to follow. But wait, here is another
example:
(drivers/video/fbdev/core/fbmem.c)
static int
fb_mmap(struct file *file, struct vm_area_struct * vma)
...
res = fb->fb_mmap(info, vma);
(drivers/video/fbdev/vfb.c)
static int vfb_mmap(struct fb_info *info,
struct vm_area_struct *vma)
{
return remap_vmalloc_range(vma, (void *)info->fix.smem_start,
vma->vm_pgoff);
}
No checks, naked calls, should be also the candidate.
--
Roman
