From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933359AbcASWlK (ORCPT ); Tue, 19 Jan 2016 17:41:10 -0500 Received: from mail-pa0-f49.google.com ([209.85.220.49]:36354 "EHLO mail-pa0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932702AbcASWlC (ORCPT ); Tue, 19 Jan 2016 17:41:02 -0500 Date: Tue, 19 Jan 2016 14:40:59 -0800 (PST) From: David Rientjes X-X-Sender: rientjes@chino.kir.corp.google.com To: Chen Feng cc: gregkh@linuxfoundation.org, arve@android.com, riandrews@android.com, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org, yudongbin@hisilicon.com, saberlily.xia@hisilicon.com, suzhuangluan@hisilicon.com, kong.kongxinwei@hisilicon.com, xuyiping@hisilicon.com, z.liuxinliang@hisilicon.com, weidong2@hisilicon.com, w.f@huawei.com, puck.chen@foxmail.com, shimingxing@hisilicon.com, oliver.fu@hisilicon.com, albert.lubing@hisilicon.com, chenxiang9@huawei.com, liuzixing@huawei.com, haojian.zhuang@linaro.org, zhaojunmin@huawei.com, wangjing6@huawei.com, john.stultz@linaro.org, qijiwen@hisilicon.com, peter.panshilin@hisilicon.com, dan.zhao@hisilicon.com, linuxarm@huawei.com, dev@lists.96boards.org Subject: Re: [PATCH v2] android: binder: Sanity check at binder ioctl In-Reply-To: <1453194241-55201-1-git-send-email-puck.chen@hisilicon.com> Message-ID: References: <1453194241-55201-1-git-send-email-puck.chen@hisilicon.com> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 19 Jan 2016, Chen Feng wrote: > When a process fork a child process, we should not allow the > child process use the binder which opened by parent process. > > But if the binder-object creater is a thread of one process who exit, > the other thread can also use this binder-object normally. > We can distinguish this by the member proc->tsk->mm. > If the thread exit the tsk->mm will be NULL. > Why does exit_mm(), the point where tsk->mm == NULL, signify the point at which the binder can now be used by other threads? > proc->tsk->mm != current->mm && proc->tsk->mm > > So only allow the shared mm_struct to use the same binder-object and > check the existence of mm_struct. > > V2: Fix compile error for error commit > > Signed-off-by: Chen Feng > Signed-off-by: Wei Dong > Signed-off-by: Junmin Zhao > Reviewed-by: Zhuangluan Su > --- > drivers/android/binder.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c > index a39e85f..279063c 100644 > --- a/drivers/android/binder.c > +++ b/drivers/android/binder.c > @@ -2736,6 +2736,8 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > > /*pr_info("binder_ioctl: %d:%d %x %lx\n", > proc->pid, current->pid, cmd, arg);*/ > + if (unlikely(proc->tsk->mm != current->mm && proc->tsk->mm)) > + return -EINVAL; > > trace_binder_ioctl(cmd, arg); > I would imagine that you would want to do READ_ONCE(proc->tsk->mm) so you are guaranteed to be testing the same value.