linux-kernel - Re: sched, timers: use after free in __lock_task

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, 15 Jul 2014 15:28:04 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Sasha Levin <sasha.levin@...cle.com>,
	Ingo Molnar <mingo@...nel.org>,
	John Stultz <john.stultz@...aro.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Frederic Weisbecker <fweisbec@...il.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Dave Jones <davej@...hat.com>,
	Andrey Ryabinin <a.ryabinin@...sung.com>
Subject: Re: sched, timers: use after free in __lock_task_sighand when
 exiting a process

On Mon, Jul 14, 2014 at 04:49:53PM +0200, Oleg Nesterov wrote:
> > Oleg, what guarantees the RCU free of task-struct and sighand?
> 
> > The only RCU I can find is delayed_put_task_struct() but that's not
> > often used.
> 
> Yes, usually the code uses put_task_struct(). delayed_put_task_struct()
> acts almost as "if (dec_and_test(usage)) kfree_rcu(), but allows to use
> get_task_struct() if you observe this task under rcu_read_lock().
> 
> Say,
> 	rcu_read_lock();
> 	task = find_task_by_vpid(...);
> 	if (task)
> 		get_task_struct(task);
> 	rcu_read_unlock();
> 
> If release_task() used dec_and_test + kfree_rcu, the code above could
> not work.

Agreed.

> > TASK_DEAD etc. use regular put_task_struct() and that
> > doesn't seem to involve RCU.
> 
> Yes, the task itself (or, depending ob pov, scheduler) has a reference.
> copy_process() does
> 
> 	/*
> 	 * One for us, one for whoever does the "release_task()" (usually
> 	 * parent)
> 	 */
> 	atomic_set(&tsk->usage, 2);
> 
> "us" actually means that put_task_struct(TASK_DEAD).

Right, that's it. I got confused in the exit code. Thanks!

Content of type "application/pgp-signature" skipped