Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1312021 > unrolled thread

[PATCH v2] android: binder: Sanity check at binder ioctl

Back to article view | Back to linux.kernel

Contents

  [PATCH v2] android: binder: Sanity check at binder ioctl Chen Feng <puck.chen@hisilicon.com> - 2016-01-19 10:10 +0100
    Re: [PATCH v2] android: binder: Sanity check at binder ioctl David Rientjes <rientjes@google.com> - 2016-01-19 23:50 +0100

#1312021 — [PATCH v2] android: binder: Sanity check at binder ioctl

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.

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 <puck.chen@hisilicon.com>
Signed-off-by: Wei  Dong <weidong2@hisilicon.com>
Signed-off-by: Junmin Zhao <zhaojunmin@huawei.com>
Reviewed-by: Zhuangluan Su <suzhuangluan@hisilicon.com>
---
 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);
 
-- 
1.9.1

[toc] | [next] | [standalone]

#1312566

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 <puck.chen@hisilicon.com>
> Signed-off-by: Wei  Dong <weidong2@hisilicon.com>
> Signed-off-by: Junmin Zhao <zhaojunmin@huawei.com>
> Reviewed-by: Zhuangluan Su <suzhuangluan@hisilicon.com>
> ---
>  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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web