Re: [PATCH] mm: Check we have the right vma in __access_remote_vm()

[Michael Ellerman <michael@ellerman.id.au>]

[-- Attachment #1: Type: text/plain, Size: 1621 bytes --]

On Mon, 2011-04-11 at 16:50 -0700, Andrew Morton wrote:
> On Fri,  8 Apr 2011 17:24:01 +1000 (EST)
> Michael Ellerman <michael@ellerman.id.au> wrote:
> 
> > In __access_remote_vm() we need to check that we have found the right
> > vma, not the following vma, before we try to access it. Otherwise we
> > might call the vma's access routine with an address which does not
> > fall inside the vma.
> > 
> 
> hm, mysteries.  Does this patch fix any known problem in any known
> kernel, or was the problem discovered by inspection, or what?

Sorry I meant to add that explanation but forgot.

It was discovered on a current kernel but with an unreleased driver,
from memory it was strace leading to a kernel bad access, but it
obviously depends on what the access implementation does. 

Looking at other access implementations I only see:

$ git grep -A 5 vm_operations|grep access
arch/powerpc/platforms/cell/spufs/file.c-	.access = spufs_mem_mmap_access,
arch/x86/pci/i386.c-	.access = generic_access_phys,
drivers/char/mem.c-	.access = generic_access_phys
fs/sysfs/bin.c-	.access		= bin_access,


The spufs one looks like it might behave badly given the wrong vma, it
assumes vma->vm_file->private_data is a spu_context, and looks like it
would probably blow up pretty quickly if it wasn't.

generic_access_phys() only uses the vma to check vm_flags and get the
mm, and then walks page tables using the address. So it should bail on
the vm_flags check, or at worst let you access some other VM_IO mapping.

And bin_access() just proxies to another access implementation.

cheers



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]