netlink: hold nl_sock_hash_lock during diag dump

Although RCU protection would be possible during diag dump, doing so allows for concurrent table mutations which can render the in-table offset between individual Netlink messages invalid and thus cause legitimate sockets to be skipped in the dump. Since the diag dump is relatively low volume and consistency is more important than performance, the table mutex is held during dump. Reported-by: Andrey Wagin <[email protected]> Signed-off-by: Thomas Graf <[email protected]> Fixes: e341694 ("netlink: Convert netlink_lookup() to use RCU protected hash table") Signed-off-by: David S. Miller <[email protected]>
chewitt · Aug 7, 2014 · 6c8f7e7 · 6c8f7e7
1 parent 9ea88a1
commit 6c8f7e7
Show file tree

Hide file tree

Showing 3 changed files with 5 additions and 0 deletions.
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
@@ -104,6 +104,7 @@ static atomic_t nl_table_users = ATOMIC_INIT(0);
 
 /* Protects netlink socket hash table mutations */
 DEFINE_MUTEX(nl_sk_hash_lock);
+EXPORT_SYMBOL_GPL(nl_sk_hash_lock);
 
 static int lockdep_nl_sk_hash_is_held(void)
 {

diff --git a/net/netlink/af_netlink.h b/net/netlink/af_netlink.h
@@ -73,5 +73,6 @@ struct netlink_table {
 
 extern struct netlink_table *nl_table;
 extern rwlock_t nl_table_lock;
+extern struct mutex nl_sk_hash_lock;
 
 #endif
diff --git a/net/netlink/diag.c b/net/netlink/diag.c
@@ -170,6 +170,7 @@ static int netlink_diag_dump(struct sk_buff *skb, struct netlink_callback *cb)
 
 	req = nlmsg_data(cb->nlh);
 
+	mutex_lock(&nl_sk_hash_lock);
 	read_lock(&nl_table_lock);
 
 	if (req->sdiag_protocol == NDIAG_PROTO_ALL) {
@@ -183,13 +184,15 @@ static int netlink_diag_dump(struct sk_buff *skb, struct netlink_callback *cb)
 	} else {
 		if (req->sdiag_protocol >= MAX_LINKS) {
 			read_unlock(&nl_table_lock);
+			mutex_unlock(&nl_sk_hash_lock);
 			return -ENOENT;
 		}
 
 		__netlink_diag_dump(skb, cb, req->sdiag_protocol, s_num);
 	}
 
 	read_unlock(&nl_table_lock);
+	mutex_unlock(&nl_sk_hash_lock);
 
 	return skb->len;
 }