netdev - Re: [PATCH net] inet: add proper locking in __inet{6}

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: <20160327.223222.486104825905902179.davem@davemloft.net>
Date:	Sun, 27 Mar 2016 22:32:22 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	eric.dumazet@...il.com
Cc:	netdev@...r.kernel.org, lorenzo@...gle.com
Subject: Re: [PATCH net] inet: add proper locking in __inet{6}_lookup()

From: Eric Dumazet <eric.dumazet@...il.com>
Date: Fri, 25 Mar 2016 15:15:15 -0700

> From: Eric Dumazet <edumazet@...gle.com>
> 
> Blocking BH in __inet{6}_lookup() is not correct, as the lookups
> are done using RCU protection.
> 
> It matters only starting from Lorenzo Colitti patches to destroy
> a TCP socket, since rcu_read_lock() is already held by other users
> of these functions.
> 
> This can be backported to all known stable versions, since TCP got RCU
> lookups back in 2.6.29 : Even if iproute2 contained no code to trigger
> the bug, some user programs could have used the API.
> 
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> Cc: Lorenzo Colitti <lorenzo@...gle.com>

This is quite the maze of RCU locking here.  With this change,
inet_lookup is now:

	rcu_read_lock();
	sk = x(); {
		rcu_read_lock();
		...
		rcu_read_unlock();
	}
	if (!sk) {
		sk = y(); {
			rcu_read_lock();
			...
			rcu_read_unlock();
		}
	}
	rcu_read_unlock();

It would seem to me that we should bubble up the rcu locking calls.

If, as you say, the other users do RCU locking already, then it should
be safe to do what your patch is doing and also remove the RCU locking
done by __inet_lookup_established() and __inet_lookup_listener().