netdev - Re: [PATCH net-next-2.6 v4 10/14] l2tp: Convert rwlock to RCU

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, 21 Apr 2010 14:53:58 +0100
From:	James Chapman <jchapman@...alix.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
CC:	netdev@...r.kernel.org
Subject: Re: [PATCH net-next-2.6 v4 10/14] l2tp: Convert rwlock to RCU

Eric Dumazet wrote:
> Hi James
> 
> I started a while ago patching l2tp but I wont be able to finish and
> test the thing...
> 
> There is a fundamental problem with this kind of construct :
> (this was wrong even better your RCU conversion)
> 
> rcu_read_lock_bh()
> hlist_for_each_entry_rcu(session, walk, session_list, global_hlist) {
> 	if (session->session_id == session_id) {
> 		rcu_read_unlock_bh();
> 		return session;
> 	}
> }
> rcu_read_unlock_bh();
> 
> 
> While the lookup _is_ protected, the result is not.
> 
> As soon as you call rcu_read_unlock_bh(); and before the "return
> session;", current thread could be preempted and an other thread frees
> session under first thread. Unexpected things can then happen.
> 
> Therefore, you need either to :
> 
> 1) Take a refcount on session (or tunnel) before the return
> 2) Or move the rcu_read_lock_bh()/rcu_read_unlock_bh() at callers.
> 3) Or all callers use a stronger lock. But then, why use RCU ;)
> 
> Here is a preliminary patch, obviously not finished, nor compiled, nor
> tested, to give possible ways to handle this problem.
> 
> (I added the ref parameter to make sure to change function signatures,
> maybe its not necessary and we should always take references)

Thanks Eric. I'll take a look at this.

-- 
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html