linux-kernel - Re: [patch] futex: Cure exit race

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

Date:   Wed, 19 Dec 2018 20:13:22 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Peter Zijlstra <peterz@...radead.org>
cc:     LKML <linux-kernel@...r.kernel.org>,
        Stefan Liebler <stli@...ux.ibm.com>,
        Heiko Carstens <heiko.carstens@...ibm.com>,
        Darren Hart <dvhart@...radead.org>,
        Ingo Molnar <mingo@...nel.org>
Subject: Re: [patch] futex: Cure exit race

On Wed, 19 Dec 2018, Thomas Gleixner wrote:
> On 2018-12-18 10:31, Thomas Gleixner wrote:
> > On Wed, 12 Dec 2018, Peter Zijlstra wrote:
> > > On Mon, Dec 10, 2018 at 06:43:51PM +0100, Thomas Gleixner wrote:
> > > @@ -806,6 +806,8 @@ void __noreturn do_exit(long code)
> > >  		 * task into the wait for ever nirwana as well.
> > >  		 */
> > >  		tsk->flags |= PF_EXITPIDONE;
> > > +		smp_mb();
> > > +		wake_up_bit(&tsk->flags, 3 /* PF_EXITPIDONE */);
> > 
> > Using ilog2(PF_EXITPIDONE) spares that horrible inline comment and more
> > importantly selects the right bit. 0x04 is bit 2 ....
> 
> Plus wake_up_bit() and wait_on_bit() want an unsigned long, but tsk->flags is
> unsigned int....
> 
> Moar staring....

Aside of that calling wake_on_bit() unconditionally can be slow if the
waitqueue in the hash bucket is not empty.

So while cooking up an alternative solution I found yet another exit race:

  CPU0	 	       		   CPU1

  sys_futex()                      sys_exit()
   futex_lock_pi()                  do_exit()
   No waiters:
   *uaddr == 0x00000PID;
   Set waiter bit
   *uaddr = 0x80000PID;
   attach_to_pi_owner()
    tsk = get_task(PID);            exit_signals(tsk)
    if (!(tsk->flags & PF_EXITING))
       ...                           tsk->flags |= PF_EXITING;
                                    mm_release(tsk)
				      exit_robust_list(tsk)
				        Set owner died and clear PID
					*uaddr = 0xC0000000;
                                      if (unlikely(!list_empty(&tsk->pi_state_list)))
       list_add(&pi_state->list,
             &tsk->pi_state_list);

I put that all on hold until Jan 7.

If somebody is really bored, here is the WIP patch series which addresses
the live lock mess: https://tglx.de/~tglx/patches.tar

Thanks,

	tglx