linux-kernel - Re: while_each_thread() under rcu_read

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]

Message-Id: <20100624211446.5713743188@magilla.sf.frob.com>
Date:	Thu, 24 Jun 2010 14:14:46 -0700 (PDT)
From:	Roland McGrath <roland@...hat.com>
To:	paulmck@...ux.vnet.ibm.com
Cc:	Oleg Nesterov <oleg@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Don Zickus <dzickus@...hat.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Ingo Molnar <mingo@...e.hu>,
	Jerome Marchand <jmarchan@...hat.com>,
	Mandeep Singh Baines <msb@...gle.com>,
	linux-kernel@...r.kernel.org, stable@...nel.org,
	"Eric W. Biederman" <ebiederm@...ssion.com>
Subject: Re: while_each_thread() under rcu_read_lock() is broken?

> First, what "bad things" can happen to a reader scanning a thread
> group?
> 
> 1.	The thread-group leader might do exec(), destroying the old
> 	list and forming a new one.  In this case, we want any readers
> 	to stop scanning.

This doesn't do anything different (for these concerns) from just all the
other threads happening to exit right before the exec.  There is no
"destroying the old" and "forming the new", it's just that all the other
threads are convinced to die now.  There is no problem here.

> 2.	Some other thread might do exec(), destroying the old list and
> 	forming a new one.  In this case, we also want any readers to
> 	stop scanning.

Again, the list is not really destroyed, just everybody dies.  What is
different here is that ->group_leader changes.  This is the only time
that ever happens.  Moreover, it's the only time that a task that was
previously pointed to by any ->group_leader can be reaped before the
rest of the group has already been reaped first (and thus the
thread_group made a singleton).

> 3.	The thread-group leader might do pthread_exit(), removing itself
> 	from the thread group -- and might do so while the hapless reader
> 	is referencing that thread.

This is called the delay_group_leader() case.  It doesn't happen in a
way that has the problems you are concerned with.  The group_leader
remains in EXIT_ZOMBIE state and can't be reaped until all the other
threads have been reaped.  There is no time at which any thread in the
group is in any hashes or accessible by any means after the (final)
group_leader is reaped.

> 4.	Some other thread might do pthread_exit(), removing itself
> 	from the thread group, and again might do so while the hapless
> 	reader is referencing that thread.  In this case, we want
> 	the hapless reader to continue scanning the remainder of the
> 	thread group.

This is the most normal case (and #1 is effectively just this repeated
by every thread in parallel).

> 5.	The thread-group leader might do exit(), destroying the old
> 	list without forming a new one.  In this case, we want any
> 	readers to stop scanning.

All this means is everybody is convinced to die, and the group_leader
dies too.  It is not discernibly different from #6.

> 6.	Some other thread might do exit(), destroying the old list
> 	without forming a new one.  In this case, we also want any
> 	readers to stop scanning.

This just means everybody is convinced to die and is not materially
different from each individual thread all happening to die at the same
time.  

You've described all these cases as "we want any readers to stop
scanning".  That is far too strong, and sounds like some kind of
guaranteed synchronization, which does not and need not exist.  Any
reader that needs a dead thread to be off the list holds siglock
and/or tasklist_lock.  For the casual readers that only use
rcu_read_lock, we only "want any readers' loops eventually to
terminate and never to dereference stale pointers".  That's why
normal RCU listiness is generally fine.

The only problem we have is in #2.  This is only a problem because
readers' loops may be using the old ->group_leader pointer as the
anchor for their circular-list round-robin loop.  Once the former
leader is removed from the list, that loop termination condition can
never be met.

Thanks,
Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/