linux-kernel - Re: [PATCH] rcu: fix warning in rcu_seq

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: <20170305202018.GE30506@linux.vnet.ibm.com>
Date:   Sun, 5 Mar 2017 12:20:18 -0800
From:   "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:     Dmitry Vyukov <dvyukov@...gle.com>
Cc:     syzkaller@...glegroups.com, linux-kernel@...r.kernel.org,
        Steven Rostedt <rostedt@...dmis.org>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        josh@...htriplett.org, jiangshanlai@...il.com
Subject: Re: [PATCH] rcu: fix warning in rcu_seq_end()

On Sun, Mar 05, 2017 at 11:48:17AM +0100, Dmitry Vyukov wrote:
> rcu_seq_end() increments seq signifying completion of a grace period,
> after that checks that the seq is even and wakes
> _synchronize_rcu_expedited(). _synchronize_rcu_expedited() uses
> wait_event() to wait for even seq. The problem is that wait_event()
> can return as soon as seq becomes even without waiting for the wakeup.
> In such case the warning in rcu_seq_end() can falsely fire if the next
> expedited grace period starts before the check.
> 
> Check that seq has good value before incrementing it.
> 
> Signed-off-by: Dmitry Vyukov <dvyukov@...gle.com>
> Cc: syzkaller@...glegroups.com
> Cc: linux-kernel@...r.kernel.org
> Cc: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> Cc: Steven Rostedt <rostedt@...dmis.org>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
> Cc: josh@...htriplett.org
> Cc: jiangshanlai@...il.com

Queued for testing and review, thank you!

							Thanx, Paul

> ---
> 
> syzkaller-triggered warning:
> 
> WARNING: CPU: 0 PID: 4832 at kernel/rcu/tree.c:3533
> rcu_seq_end+0x110/0x140 kernel/rcu/tree.c:3533
> CPU: 0 PID: 4832 Comm: kworker/0:3 Not tainted 4.10.0+ #276
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Workqueue: events wait_rcu_exp_gp
> Call Trace:
>  __dump_stack lib/dump_stack.c:15 [inline]
>  dump_stack+0x2ee/0x3ef lib/dump_stack.c:51
>  panic+0x1fb/0x412 kernel/panic.c:179
>  __warn+0x1c4/0x1e0 kernel/panic.c:540
>  warn_slowpath_null+0x2c/0x40 kernel/panic.c:583
>  rcu_seq_end+0x110/0x140 kernel/rcu/tree.c:3533
>  rcu_exp_gp_seq_end kernel/rcu/tree_exp.h:36 [inline]
>  rcu_exp_wait_wake+0x8a9/0x1330 kernel/rcu/tree_exp.h:517
>  rcu_exp_sel_wait_wake kernel/rcu/tree_exp.h:559 [inline]
>  wait_rcu_exp_gp+0x83/0xc0 kernel/rcu/tree_exp.h:570
>  process_one_work+0xc06/0x1c20 kernel/workqueue.c:2096
>  worker_thread+0x223/0x19c0 kernel/workqueue.c:2230
>  kthread+0x326/0x3f0 kernel/kthread.c:227
>  ret_from_fork+0x31/0x40 arch/x86/entry/entry_64.S:430
> ---
>  kernel/rcu/tree.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 50fee7689e71..47339b242423 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -3531,8 +3531,8 @@ static void rcu_seq_start(unsigned long *sp)
>  static void rcu_seq_end(unsigned long *sp)
>  {
>  	smp_mb(); /* Ensure update-side operation before counter increment. */
> +	WARN_ON_ONCE(!(*sp & 0x1));
>  	WRITE_ONCE(*sp, *sp + 1);
> -	WARN_ON_ONCE(*sp & 0x1);
>  }
> 
>  /* Take a snapshot of the update side's sequence number. */
> -- 
> 2.12.0.rc1.440.g5b76565f74-goog
>