linux-kernel - Re: [PATCH] binder: skip dead binder_proc during binder

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aBDieyvgjcNc61gy@google.com>
Date: Tue, 29 Apr 2025 14:30:19 +0000
From: Carlos Llamas <cmllamas@...gle.com>
To: zhengyan <zhengyan@...micro.com>
Cc: gregkh@...uxfoundation.org, arve@...roid.com, tkjos@...roid.com,
	maco@...roid.com, joel@...lfernandes.org, brauner@...nel.org,
	surenb@...gle.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] binder: skip dead binder_proc during binder_open

On Tue, Apr 29, 2025 at 07:50:30AM +0000, zhengyan wrote:
> During binder_open, the binder_proc list is travesed to check
> for the existing binder_proc instances. binder_proc objects
> are async released in a deferred work after binder_release,
> and may remain temporarily on the binder_procs list even after
> being marked as dead.
> 
> Without checking the flag, binder_open may face a crash as
> "Unable to handle kernel paging request at virtual address
> dead000000000140"
> 
> Signed-off-by: zhengyan <zhengyan@...micro.com>
> ---
>  drivers/android/binder.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 76052006bd87..43ab4350e589 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -6041,6 +6041,8 @@ static int binder_open(struct inode *nodp, struct file *filp)
>  
>  	mutex_lock(&binder_procs_lock);
>  	hlist_for_each_entry(itr, &binder_procs, proc_node) {
> +		if (itr->is_dead)
> +			continue;
>  		if (itr->pid == proc->pid) {
>  			existing_pid = true;
>  			break;
> -- 
> 2.25.1

Can you please explain this scenario? My understanding is that dead
procs are removed from the binder_procs list (with appropriate locks)
before they are marked as ->is_dead. So how is this even possible?

--
Carlos Llamas