linux-kernel - Re: [PATCH] futex: Remove the owner check when waking task in handle_futex

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 for Android: free password hash cracker in your pocket

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

Message-ID: <1380219317.32071.6.camel@dvhart-mobl4.amr.corp.intel.com>
Date:	Thu, 26 Sep 2013 11:15:17 -0700
From:	Darren Hart <dvhart@...ux.intel.com>
To:	zhang.yi20@....com.cn
Cc:	linux-kernel@...r.kernel.org,
	Peter Zijlstra <peterz@...radead.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...nel.org>
Subject: Re: [PATCH] futex: Remove the owner check when waking task in
 handle_futex_death

On Thu, 2013-09-26 at 09:09 +0800, zhang.yi20@....com.cn wrote:
> Hi all,
> 
> Task processes all its owned robust futex when it is exiting,
> to ensure the futexes can be taken by other tasks.
> 
> Though this can not work good in sometimes.
> Think about this scene：
> 1. A robust mutex is shared for two processes, each process has
>    multi threads to lock the mutex.
> 2. One of the threads locks the mutex, and the others are waiting
>    and sorted in order of priority.
> 3. The process to which the mutex owner thread belongs is dying
>    without unlocking the mutex，and handle_futex_death is invoked
>    to wake the first waiter.
> 4. If the first waiter belongs to the same process，it has no chance
>    to return to the userspace to lock the mutex, and it won't wake
>    the next waiter because it is not the owner of the mutex.
> 5. The rest waiters of the other process may block forever.
> 
> This patch remove the owner check when waking task in handle_futex_death.
> If above occured, The dying task can wake the next waiter by processing its list_op_pending.
> The waked task could return to userspace and try to lock the mutex again.
> 

The problem is if you allow the non-owner to do the wake, you risk
multiple threads calling futex_wake(). Or is that your intention? Wake
one waiter for every thread that calls handle_futex_death()?

> 
> Signed-off-by: Zhang Yi <zhang.yi20@....com.cn>
> Reviewed-by: Xie Baoyou <xie.baoyou@....com.cn>
> Reviewed-by: Lu Zhongjun <lu.zhongjun@....com.cn>
> 
> 
> 
> --- linux/kernel/futex.c	2013-09-25 09:24:34.639634244 +0000
> +++ linux/kernel/futex.c	2013-09-25 10:12:17.619673546 +0000
> @@ -2541,14 +2541,15 @@ retry:
>  		}
>  		if (nval != uval)
>  			goto retry;
> -
> -		/*
> -		 * Wake robust non-PI futexes here. The wakeup of
> -		 * PI futexes happens in exit_pi_state():
> -		 */
> -		if (!pi && (uval & FUTEX_WAITERS))
> -			futex_wake(uaddr, 1, 1, FUTEX_BITSET_MATCH_ANY);
>  	}
> +
> +	/*
> +	 * Wake robust non-PI futexes here. The wakeup of
> +	 * PI futexes happens in exit_pi_state():
> +	 */
> +	if (!pi)


Why did you drop the FUTEX_WAITERS condition?

You sent a different patch earlier this year that didn't appear to get
any review:

https://lkml.org/lkml/2013/4/8/65

This one woke all the waiters and let them sort it out.

It seems we've hashed through this already, but I'm not finding the
email logs and I don't recall off the top of my head.

> +		futex_wake(uaddr, 1, 1, FUTEX_BITSET_MATCH_ANY);
> +
>  	return 0;
>  }

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/