On Tue, Aug 16, 2016 at 07:44:59PM -0700, Arve Hjønnevåg wrote:
> On Mon, Aug 15, 2016 at 7:58 AM, Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > On Thu, Jun 16, 2016 at 12:45:33AM +0200, Jann Horn wrote:
> >> If /dev/binder is opened and the opener process then e.g. calls execve,
> >> proc->vma_vm_mm will still point to the location of the now-freed
> >> mm_struct. If the process then calls ioctl(binder_fd, ...), the dangling
> >> proc->vma_vm_mm pointer will be compared to current->mm.
> >>
> >> Let the binder take a reference to the mm_struct to avoid this.
> >>
> >> v2: use the right refcounter
> >>
> >> Fixes: a906d6931f3ccaf7de805643190765ddd7378e27
> >
> > Nit, the proper way to do this is:
> >
> > Fixes: a906d6931f3c ("android: binder: Sanity check at binder ioctl")
> >
> > You can get that by doing:
> > git show -s --abbrev-commit --abbrev=12 --pretty=format:"%h (\"%s\")%n" a906d6931f3ccaf7de805643190765ddd7378e27
> >
> > Also, I'm guessing this should go to the stable kernels that include the
> > above patch?
> >
>
> Has this patch been tested?
>
> Does it actually fix anything?
>
> Changes to vma_vm_mm are normally protected by the mmap_sem it
> contains, and is set to NULL in the vma->vm_ops->close call on the
> binder vma. The vma_vm_mm pointer is only used to check if the mm
> struct that the binder driver was mmapped into is the same mm struct
> that the driver just locked (in binder_update_page_range). I don't
> think change description here is accurate. If a process calls execve,
> the old vma should be closed (and vma_vm_mm cleared) before the
> mm_struct that it belongs too is freed.
Yeah, this whole thing feels wrong...
Combined with the original author's email address now bouncing, so I've
reverted this patch from the tree.
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
