Re: [PATCH v2] binder: fix proc->files use-after-free

2017-11-20 Thread Todd Kjos
Al, thanks for the detailed feedback. I didn't know about these rules (are they written down somewhere?). I'll rework this and post a compliant v3. On Fri, Nov 17, 2017 at 11:31 AM, Al Viro wrote: > On Thu, Nov 16, 2017 at 09:56:50AM -0800, Todd Kjos wrote: > >> +static

Re: [PATCH v2] binder: fix proc->files use-after-free

2017-11-20 Thread Todd Kjos
Al, thanks for the detailed feedback. I didn't know about these rules (are they written down somewhere?). I'll rework this and post a compliant v3. On Fri, Nov 17, 2017 at 11:31 AM, Al Viro wrote: > On Thu, Nov 16, 2017 at 09:56:50AM -0800, Todd Kjos wrote: > >> +static struct files_struct

Re: [PATCH v2] binder: fix proc->files use-after-free

2017-11-17 Thread Al Viro
On Thu, Nov 16, 2017 at 09:56:50AM -0800, Todd Kjos wrote: > +static struct files_struct *binder_get_files_struct(struct binder_proc *proc) > +{ > + return get_files_struct(proc->tsk); > +} Hell, _no_. You should never, ever use the result of get_files_struct() for write access. It's

Re: [PATCH v2] binder: fix proc->files use-after-free

2017-11-17 Thread Al Viro
On Thu, Nov 16, 2017 at 09:56:50AM -0800, Todd Kjos wrote: > +static struct files_struct *binder_get_files_struct(struct binder_proc *proc) > +{ > + return get_files_struct(proc->tsk); > +} Hell, _no_. You should never, ever use the result of get_files_struct() for write access. It's

Re: [PATCH v2] binder: fix proc->files use-after-free

2017-11-17 Thread Greg KH
On Thu, Nov 16, 2017 at 12:37:02PM -0800, Todd Kjos wrote: > Sorry about that, do you want a v3 with correct annotations? Nah, this time is fine :)

Re: [PATCH v2] binder: fix proc->files use-after-free

2017-11-17 Thread Greg KH
On Thu, Nov 16, 2017 at 12:37:02PM -0800, Todd Kjos wrote: > Sorry about that, do you want a v3 with correct annotations? Nah, this time is fine :)

Re: [PATCH v2] binder: fix proc->files use-after-free

2017-11-16 Thread Todd Kjos
Sorry about that, do you want a v3 with correct annotations? On Thu, Nov 16, 2017 at 12:27 PM, Greg KH wrote: > On Thu, Nov 16, 2017 at 09:56:50AM -0800, Todd Kjos wrote: >> proc->files cleanup is initiated by binder_vma_close. Therefore >> a reference on the

Re: [PATCH v2] binder: fix proc->files use-after-free

2017-11-16 Thread Todd Kjos
Sorry about that, do you want a v3 with correct annotations? On Thu, Nov 16, 2017 at 12:27 PM, Greg KH wrote: > On Thu, Nov 16, 2017 at 09:56:50AM -0800, Todd Kjos wrote: >> proc->files cleanup is initiated by binder_vma_close. Therefore >> a reference on the binder_proc is not enough to prevent

Re: [PATCH v2] binder: fix proc->files use-after-free

2017-11-16 Thread Greg KH
On Thu, Nov 16, 2017 at 09:56:50AM -0800, Todd Kjos wrote: > proc->files cleanup is initiated by binder_vma_close. Therefore > a reference on the binder_proc is not enough to prevent the > files_struct from being released while the binder_proc still has > a reference. This can lead to an attempt

Re: [PATCH v2] binder: fix proc->files use-after-free

2017-11-16 Thread Greg KH
On Thu, Nov 16, 2017 at 09:56:50AM -0800, Todd Kjos wrote: > proc->files cleanup is initiated by binder_vma_close. Therefore > a reference on the binder_proc is not enough to prevent the > files_struct from being released while the binder_proc still has > a reference. This can lead to an attempt

[PATCH v2] binder: fix proc->files use-after-free

2017-11-16 Thread Todd Kjos
proc->files cleanup is initiated by binder_vma_close. Therefore a reference on the binder_proc is not enough to prevent the files_struct from being released while the binder_proc still has a reference. This can lead to an attempt to dereference the stale pointer obtained from proc->files prior to

[PATCH v2] binder: fix proc->files use-after-free

2017-11-16 Thread Todd Kjos
proc->files cleanup is initiated by binder_vma_close. Therefore a reference on the binder_proc is not enough to prevent the files_struct from being released while the binder_proc still has a reference. This can lead to an attempt to dereference the stale pointer obtained from proc->files prior to