Re: [PATCH net-next 2/3] netlink: Convert netlink_lookup() to use RCU protected hash table

[Eric Dumazet]

On Mon, 2014-08-04 at 19:58 -0700, David Miller wrote:
> From: Sasha Levin <sasha.levin@xxxxxxxxxx>
> 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@xxxxxxxxxx>
Signed-off-by: Eric Dumazet <edumazet@xxxxxxxxxx>
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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/