Messages in this thread | | | Subject | Re: [PATCH] binder: Drop needless locking in binder_mmap() | From | Dmitry Safonov <> | Date | Mon, 19 Jun 2017 17:53:34 +0300 |
| |
On 05/24/2017 05:51 PM, Dmitry Safonov wrote: > binder_mmap() protected proc->buffer from concurrent changes. > It was introduced with commit bd1eff9741af ("Staging: android: binder: > Fix crashes when sharing a binder file between processes"). > > The commit fixed such races alike: > > task1 > ===== > fd = open("/dev/binder",..) > fork() > ---------------------------------------->task2 > | ===== > mmap(fd) mmap(fd) > > Afterwards the situation was changed with the commit a79f41ed9786 > ("binder: don't allow mmap() by process other than proc->tsk"). > That commit prohibited mmap'ing of fd, that was opened by some > other task. > That restriction has made locking in binder_mmap() unnecessary. > > Cc: Al Viro <viro@zeniv.linux.org.uk> > Cc: "Arve Hjønnevåg" <arve@android.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Riley Andrews <riandrews@android.com> > Cc: devel@driverdev.osuosl.org > Cc: Android Kernel Team <kernel-team@android.com> > Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com> > --- > drivers/android/binder.c | 5 ----- > 1 file changed, 5 deletions(-)
Ping?
> > diff --git a/drivers/android/binder.c b/drivers/android/binder.c > index aae4d8d4be36..b5b32be85ab8 100644 > --- a/drivers/android/binder.c > +++ b/drivers/android/binder.c > @@ -49,7 +49,6 @@ > > static DEFINE_MUTEX(binder_main_lock); > static DEFINE_MUTEX(binder_deferred_lock); > -static DEFINE_MUTEX(binder_mmap_lock); > > static HLIST_HEAD(binder_devices); > static HLIST_HEAD(binder_procs); > @@ -3381,7 +3380,6 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma) > } > vma->vm_flags = (vma->vm_flags | VM_DONTCOPY) & ~VM_MAYWRITE; > > - mutex_lock(&binder_mmap_lock); > if (proc->buffer) { > ret = -EBUSY; > failure_string = "already mapped"; > @@ -3396,7 +3394,6 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma) > } > proc->buffer = area->addr; > proc->user_buffer_offset = vma->vm_start - (uintptr_t)proc->buffer; > - mutex_unlock(&binder_mmap_lock); > > #ifdef CONFIG_CPU_CACHE_VIPT > if (cache_is_vipt_aliasing()) { > @@ -3441,12 +3438,10 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma) > kfree(proc->pages); > proc->pages = NULL; > err_alloc_pages_failed: > - mutex_lock(&binder_mmap_lock); > vfree(proc->buffer); > proc->buffer = NULL; > err_get_vm_area_failed: > err_already_mapped: > - mutex_unlock(&binder_mmap_lock); > err_bad_arg: > pr_err("binder_mmap: %d %lx-%lx %s failed %d\n", > proc->pid, vma->vm_start, vma->vm_end, failure_string, ret); >
| |