linux-kernel - [PATCH] locking, rwsem: Fix down_write

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: <20160512115745.GP3192@twins.programming.kicks-ass.net>
Date:	Thu, 12 May 2016 13:57:45 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Michal Hocko <mhocko@...nel.org>
Cc:	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
	LKML <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	"David S. Miller" <davem@...emloft.net>,
	Tony Luck <tony.luck@...el.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Chris Zankel <chris@...kel.net>,
	Max Filippov <jcmvbkbc@...il.com>,
	Davidlohr Bueso <dave@...olabs.net>,
	Waiman Long <Waiman.Long@....com>
Subject: [PATCH] locking, rwsem: Fix down_write_killable()

On Wed, May 11, 2016 at 08:03:46PM +0200, Michal Hocko wrote:
> On Wed 11-05-16 15:59:38, Michal Hocko wrote:
> > On Wed 11-05-16 11:41:28, Peter Zijlstra wrote:
> > > On Wed, May 11, 2016 at 11:31:27AM +0200, Michal Hocko wrote:
> > > 
> > > > Care to cook up a full patch?
> > > 
> > > compile tested only, if someone could please test it?
> > 
> > I have tried to run the test case from Tetsuo[1] with a small printk to
> > show the interrupted writer case:
> > [ 2753.596678] XXX: Writer interrupted. Woken waiters:0
> > [ 2998.266978] XXX: Writer interrupted. Woken waiters:0
> > 
> > which means rwsem_atomic_update(-RWSEM_WAITING_BIAS, sem) path which is
> > the problematic case. oom_reaper was always able to succeed so I guess
> > the patch works as expected. I will leave the test run for longer to be
> > sure.
> 
> And just for the reference I am able to reproduce the lockup without the
> patch applied and the same test case and a debugging patch
> 
> [ 1522.036379] XXX interrupted. list_is_singular:1
> [ 1523.040462] oom_reaper: unable to reap pid:3736 (tgid=3736)

Here the complete patch. Ingo could you make it appear in the right tip
branch?


---
Subject: locking, rwsem: Fix down_write_killable()
From: Peter Zijlstra <peterz@...radead.org>
Date: Wed, 11 May 2016 11:41:28 +0200

The new signal_pending exit path in __rwsem_down_write_failed_common()
was fingered as breaking his kernel by Tetsuo Handa.

Upon inspection it was found that there are two things wrong with it;

 - it forgets to remove WAITING_BIAS if it leaves the list empty, or
 - it forgets to wake further waiters that were blocked on the now
   removed waiter.

Especially the first issue causes new lock attempts to block and stall
indefinitely, as the code assumes that pending waiters mean there is
an owner that will wake when it releases the lock.

Cc: "David S. Miller" <davem@...emloft.net>
Cc: Waiman Long <Waiman.Long@....com>
Cc: Chris Zankel <chris@...kel.net>
Cc: Ingo Molnar <mingo@...hat.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>
Cc: Thomas Gleixner <tglx@...utronix.de>
Cc: Davidlohr Bueso <dave@...olabs.net>
Cc: Tony Luck <tony.luck@...el.com>
Cc: "H. Peter Anvin" <hpa@...or.com>
Cc: Max Filippov <jcmvbkbc@...il.com>
Reported-by: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
Tested-by: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
Tested-by: Michal Hocko <mhocko@...nel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
---
 kernel/locking/rwsem-xadd.c |   21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

--- a/kernel/locking/rwsem-xadd.c
+++ b/kernel/locking/rwsem-xadd.c
@@ -487,23 +487,32 @@ __rwsem_down_write_failed_common(struct
 
 		/* Block until there are no active lockers. */
 		do {
-			if (signal_pending_state(state, current)) {
-				raw_spin_lock_irq(&sem->wait_lock);
-				ret = ERR_PTR(-EINTR);
-				goto out;
-			}
+			if (signal_pending_state(state, current))
+				goto out_nolock;
+
 			schedule();
 			set_current_state(state);
 		} while ((count = sem->count) & RWSEM_ACTIVE_MASK);
 
 		raw_spin_lock_irq(&sem->wait_lock);
 	}
-out:
 	__set_current_state(TASK_RUNNING);
 	list_del(&waiter.list);
 	raw_spin_unlock_irq(&sem->wait_lock);
 
 	return ret;
+
+out_nolock:
+	__set_current_state(TASK_RUNNING);
+	raw_spin_lock_irq(&sem->wait_lock);
+	list_del(&waiter.list);
+	if (list_empty(&sem->wait_list))
+		rwsem_atomic_update(-RWSEM_WAITING_BIAS, sem);
+	else
+		__rwsem_do_wake(sem, RWSEM_WAKE_ANY);
+	raw_spin_unlock_irq(&sem->wait_lock);
+
+	return ERR_PTR(-EINTR);
 }
 
 __visible struct rw_semaphore * __sched