linux-kernel - Re: [PATCH 3.14-rt] sched/numa: Fix task_numa

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] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 27 May 2014 20:25:41 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	Mike Galbraith <umgwanakikbuti@...il.com>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	linux-rt-users <linux-rt-users@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	John Kacur <jkacur@...hat.com>,
	LKML <linux-kernel@...r.kernel.org>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Sasha Levin <sasha.levin@...cle.com>
Subject: Re: [PATCH 3.14-rt] sched/numa: Fix task_numa_free() lockdep splat

On Tue, May 27, 2014 at 02:18:36PM -0400, Steven Rostedt wrote:
> [ moving this to LKML from linux-rt-users, as that's where it should be ]
> 
> On Sat, 17 May 2014 05:36:59 +0200
> Mike Galbraith <umgwanakikbuti@...il.com> wrote:
> 
> > 3.14-rt being build with a non-rt config is unlikely, but..
> > 
> > >From 60e69eed85bb7b5198ef70643b5895c26ad76ef7 Mon Sep 17 00:00:00 2001
> > From: Mike Galbraith <bitbucket@...ine.de>
> > Date: Mon, 7 Apr 2014 10:55:15 +0200
> > Subject: [PATCH] sched/numa: Fix task_numa_free() lockdep splat
> > 
> > Sasha reported that lockdep claims that the following commit:
> > made numa_group.lock interrupt unsafe:
> > 
> >   156654f491dd ("sched/numa: Move task_numa_free() to __put_task_struct()")
> > 
> > While I don't see how that could be, given the commit in question moved
> > task_numa_free() from one irq enabled region to another, the below does
> > make both gripes and lockups upon gripe with numa=fake=4 go away.
> 
> It wasn't the irqs that was causing the lockdep splat, but the
> softirqs. You moved it into __put_task_struct() which is called as a
> rcu callback that gets called from soft irqs. So yes, you need to
> prevent softirqs from happening whenever you take the lock.
> spin_lock_irq() is a bigger hammer than needed. The patch below should
> be good enough.
> 
> I kept the double_lock_irq() as there is no double_lock_bh(). Should we
> bother to make one?

Nope, its really IRQs.

do_exit()
  exit_itimers()
    itimer_delete()
      spin_lock_irqsave(&timer->it_lock, &flags);
      timer_delete_hook(timer);
        kc->timer_del(timer) := posix_cpu_timer_del()
          put_task_struct()
            __put_task_struct()
              task_numa_free()
                spin_lock(&grp->lock);

Which nests the grp->lock inside the timer->it_lock, and where the
timer->it_lock is IRQ-safe, the grp->lock is not.

This allows for IRQ deadlocks.



Content of type "application/pgp-signature" skipped