On Thu, Jan 2, 2014 at 12:58 PM, <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> From: Rik van Riel <riel@xxxxxxxxxx>
> Subject: mm: fix use-after-free in sys_remap_file_pages
>
> remap_file_pages calls mmap_region, which may merge the VMA with other
> existing VMAs, and free "vma". This can lead to a use-after-free bug.
> Avoid the bug by remembering vm_flags before calling mmap_region, and not
> trying to dereference vma later.
>
>
> diff -puN mm/fremap.c~mm-fix-use-after-free-in-sys_remap_file_pages mm/fremap.c
> --- a/mm/fremap.c~mm-fix-use-after-free-in-sys_remap_file_pages
> +++ a/mm/fremap.c
> @@ -208,9 +208,10 @@ get_write_lock:
> if (mapping_cap_account_dirty(mapping)) {
> unsigned long addr;
> struct file *file = get_file(vma->vm_file);
> + /* mmap_region may free vma; grab the info now */
> + vm_flags = ACCESS_ONCE(vma->vm_flags);
>
> - addr = mmap_region(file, start, size,
> - vma->vm_flags, pgoff);
> + addr = mmap_region(file, start, size, vm_flags, pgoff);
What's the logic begind the ACCESS_ONCE() part? That makes no sense to
me. Sure, mmap_region() may free the vma, and I agree with the patch
per se, but the ACCESS_ONCE() looks like some random voodoo
programming to me..
Linus
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html