linux-kernel - Re: [PATCH linux-4.1.y] sched/wait: Fix signal handling in bit wait helpers

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: <20170418134631.GA13416@redhat.com>
Date:   Tue, 18 Apr 2017 15:46:31 +0200
From:   Oleg Nesterov <oleg@...hat.com>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        alexander.levin@...izon.com, Peter Zijlstra <peterz@...radead.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Mike Galbraith <efault@....de>,
        Thomas Gleixner <tglx@...utronix.de>, mark.rutland@....com,
        neilb@...e.de, Ingo Molnar <mingo@...nel.org>
Subject: Re: [PATCH linux-4.1.y] sched/wait: Fix signal handling in bit wait
 helpers

Just curious, any particular reason you want to backport this patch?

because nobody understands why it actually helped.

and please note that this patch is buggy, fixed by dfd01f0260
"sched/wait: Fix the signal handling fix"

Oleg.

On 04/12, Florian Fainelli wrote:
>
> From: Peter Zijlstra <peterz@...radead.org>
> 
> [ Upstream commit 68985633bccb6066bf1803e316fbc6c1f5b796d6 ]
> 
> Vladimir reported getting RCU stall warnings and bisected it back to
> commit:
> 
>   743162013d40 ("sched: Remove proliferation of wait_on_bit() action functions")
> 
> That commit inadvertently reversed the calls to schedule() and signal_pending(),
> thereby not handling the case where the signal receives while we sleep.
> 
> Reported-by: Vladimir Murzin <vladimir.murzin@....com>
> Tested-by: Vladimir Murzin <vladimir.murzin@....com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> Cc: Linus Torvalds <torvalds@...ux-foundation.org>
> Cc: Mike Galbraith <efault@....de>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: mark.rutland@....com
> Cc: neilb@...e.de
> Cc: oleg@...hat.com
> Fixes: 743162013d40 ("sched: Remove proliferation of wait_on_bit() action functions")
> Fixes: cbbce8220949 ("SCHED: add some "wait..on_bit...timeout()" interfaces.")
> Link: http://lkml.kernel.org/r/20151201130404.GL3816@twins.programming.kicks-ass.net
> Signed-off-by: Ingo Molnar <mingo@...nel.org>
> ---
>  kernel/sched/wait.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
> index 852143a79f36..e0bb7e6c4fb0 100644
> --- a/kernel/sched/wait.c
> +++ b/kernel/sched/wait.c
> @@ -583,18 +583,18 @@ EXPORT_SYMBOL(wake_up_atomic_t);
>  
>  __sched int bit_wait(struct wait_bit_key *word)
>  {
> -	if (signal_pending_state(current->state, current))
> -		return 1;
>  	schedule();
> +	if (signal_pending(current))
> +		return -EINTR;
>  	return 0;
>  }
>  EXPORT_SYMBOL(bit_wait);
>  
>  __sched int bit_wait_io(struct wait_bit_key *word)
>  {
> -	if (signal_pending_state(current->state, current))
> -		return 1;
>  	io_schedule();
> +	if (signal_pending(current))
> +		return -EINTR;
>  	return 0;
>  }
>  EXPORT_SYMBOL(bit_wait_io);
> @@ -602,11 +602,11 @@ EXPORT_SYMBOL(bit_wait_io);
>  __sched int bit_wait_timeout(struct wait_bit_key *word)
>  {
>  	unsigned long now = ACCESS_ONCE(jiffies);
> -	if (signal_pending_state(current->state, current))
> -		return 1;
>  	if (time_after_eq(now, word->timeout))
>  		return -EAGAIN;
>  	schedule_timeout(word->timeout - now);
> +	if (signal_pending(current))
> +		return -EINTR;
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(bit_wait_timeout);
> @@ -614,11 +614,11 @@ EXPORT_SYMBOL_GPL(bit_wait_timeout);
>  __sched int bit_wait_io_timeout(struct wait_bit_key *word)
>  {
>  	unsigned long now = ACCESS_ONCE(jiffies);
> -	if (signal_pending_state(current->state, current))
> -		return 1;
>  	if (time_after_eq(now, word->timeout))
>  		return -EAGAIN;
>  	io_schedule_timeout(word->timeout - now);
> +	if (signal_pending(current))
> +		return -EINTR;
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(bit_wait_io_timeout);
> -- 
> 2.12.1
>