linux-kernel - Re: [PATCH net-next 2/3] netlink: Convert netlink

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, 05 Aug 2014 07:50:07 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	David Miller <davem@...emloft.net>, Thomas Graf <tgraf@...g.ch>
Cc:	sasha.levin@...cle.com, tgraf@...g.ch, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, kaber@...sh.net,
	paulmck@...ux.vnet.ibm.com, josh@...htriplett.org,
	challa@...ronetworks.com, walpole@...pdx.edu, dev@...nvswitch.org,
	netfilter-devel@...r.kernel.org, nikolay@...hat.com
Subject: Re: [PATCH net-next 2/3] netlink: Convert netlink_lookup() to use
 RCU protected hash table

On Mon, 2014-08-04 at 19:58 -0700, David Miller wrote:
> From: Sasha Levin <sasha.levin@...cle.com>
> Date: Mon, 04 Aug 2014 22:10:19 -0400
> 
> > On 08/02/2014 05:47 AM, Thomas Graf wrote:
> >>  static void *netlink_seq_start(struct seq_file *seq, loff_t *pos)
> >> -	__acquires(nl_table_lock)
> >>  {
> >> -	read_lock(&nl_table_lock);
> >> +	rcu_read_lock();
> >>  	return *pos ? netlink_seq_socket_idx(seq, *pos - 1) : SEQ_START_TOKEN;
> >>  }
> > 
> > I'm not sure how you expect this code to work. You're replacing a local lock
> > with a RCU critical section. Imagine you're doing spin_lock() and just going
> > back to userspace.
> > 
> > It's quite easy to trigger this issue:
> 
> I think he expected the end of the seq sequence to drop the RCU lock,
> via netlink_seq_stop().
> --

Yes, two places use rht_dereference() instead of rht_dereference_rcu()

[PATCH net-next] netlink: fix lockdep splats

With netlink_lookup() conversion to RCU, we need to use appropriate
rcu dereference in netlink_seq_socket_idx() & netlink_seq_next()

Reported-by: Sasha Levin <sasha.levin@...cle.com>
Signed-off-by: Eric Dumazet <edumazet@...gle.com>
Cc: 
Fixes: e341694e3eb57fc ("netlink: Convert netlink_lookup() to use RCU protected hash table")
---
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 0b89ca51a3af..479a344563d8 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2899,7 +2899,7 @@ static struct sock *netlink_seq_socket_idx(struct seq_file *seq, loff_t pos)
 
 	for (i = 0; i < MAX_LINKS; i++) {
 		struct rhashtable *ht = &nl_table[i].hash;
-		const struct bucket_table *tbl = rht_dereference(ht->tbl, ht);
+		const struct bucket_table *tbl = rht_dereference_rcu(ht->tbl, ht);
 
 		for (j = 0; j < tbl->size; j++) {
 			rht_for_each_entry_rcu(nlk, tbl->buckets[j], node) {
@@ -2950,7 +2950,7 @@ static void *netlink_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 
 	do {
 		struct rhashtable *ht = &nl_table[i].hash;
-		const struct bucket_table *tbl = rht_dereference(ht->tbl, ht);
+		const struct bucket_table *tbl = rht_dereference_rcu(ht->tbl, ht);
 
 		for (; j < tbl->size; j++) {
 			rht_for_each_entry_rcu(nlk, tbl->buckets[j], node) {


--
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/