netlabel: Fix several rcu_dereference() calls used without RCU read locks
Subsystems (3)
PATCH 1/1 netlabel: Fix several rcu_dereference() calls used without RCU read locks
The recent changes to add RCU lock verification to rcu_dereference() calls caught out a problem with netlbl_unlhsh_hash(), see below.
===================================================
[ INFO: suspicious rcu_dereference_check() usage. ]
---------------------------------------------------
net/netlabel/netlabel_unlabeled.c:246 invoked rcu_dereference_check() without protection!
This patch fixes this problem as well as others like it in the NetLabel code. Also included in this patch is the identification of future work to eliminate the RCU read lock in netlbl_domhsh_add(), but in the interest of getting this patch out quickly that work will happen in another patch to be finished later.
Thanks to Eric Dumazet and Paul McKenney for their help in understanding the recent RCU changes.
--- net/netlabel/netlabel_domainhash.c | 28 ++++++++++----- net/netlabel/netlabel_unlabeled.c | 66 ++++++++++-------------------------- 2 files changed, 37 insertions(+), 57 deletions(-)
net/netlabel/netlabel_domainhash.c +18 -10
@@ -50,9 +50,12 @@ struct netlbl_domhsh_tbl { };/* Domain hash table */ -/* XXX - updates should be so rare that having one spinlock for the entire -* hash table should be okay */ +/* updates should be so rare that having one spinlock for the entire hash table +* should be okay */ static DEFINE_SPINLOCK (netlbl_domhsh_lock); +#define netlbl_domhsh_rcu_deref (p) \ + rcu_dereference_check(p, rcu_read_lock_held() || \ + lockdep_is_held(&netlbl_domhsh_lock))static struct netlbl_domhsh_tbl *netlbl_domhsh =NULL ;static struct netlbl_dom_map *netlbl_domhsh_def =NULL ;
@@ -106,7 +109,8 @@ static void netlbl_domhsh_free_entry(struct rcu_head *entry)* Description: * This is the hashing function for the domain hash table, it returns the * correct bucket number for the domain. The caller is responsibile for -* calling the rcu_read_[un]lock() functions. +* ensuring that the hash table is protected with either a RCU read lock or the +* hash table lock. * */ static u32 netlbl_domhsh_hash (const char *key)
@@ -120,7 +124,7 @@ static u32 netlbl_domhsh_hash(const char *key)for (iter =0 , val =0 , len =strlen (key); iter < len; iter++) val = (val <<4 | (val >> (8 *sizeof (u32) -4 ))) ^ key[iter]; -return val & (rcu_dereference (netlbl_domhsh)->size -1 ); +return val & (netlbl_domhsh_rcu_deref (netlbl_domhsh)->size -1 ); }/**
@@ -130,7 +134,8 @@ static u32 netlbl_domhsh_hash(const char *key)* Description: * Searches the domain hash table and returns a pointer to the hash table * entry if found, otherwise NULL is returned. The caller is responsibile for -* the rcu hash table locks (i.e. the caller much call rcu_read_[un]lock()). +* ensuring that the hash table is protected with either a RCU read lock or the +* hash table lock. * */ static struct netlbl_dom_map *netlbl_domhsh_search (const char *domain)
@@ -141,7 +146,7 @@ static struct netlbl_dom_map *netlbl_domhsh_search(const char *domain)if (domain !=NULL ) { bkt =netlbl_domhsh_hash (domain); - bkt_list = &rcu_dereference (netlbl_domhsh)->tbl[bkt]; + bkt_list = &netlbl_domhsh_rcu_deref (netlbl_domhsh)->tbl[bkt];list_for_each_entry_rcu (iter, bkt_list, list)if (iter->valid &&strcmp (iter->domain, domain) ==0 )return iter;
@@ -159,8 +164,8 @@ static struct netlbl_dom_map *netlbl_domhsh_search(const char *domain)* Searches the domain hash table and returns a pointer to the hash table * entry if an exact match is found, if an exact match is not present in the * hash table then the default entry is returned if valid otherwise NULL is -* returned. The caller is responsibile for the rcu hash table locks -* (i.e. the caller much call rcu_read_[un]lock()). +* returned. The caller is responsibile ensuring that the hash table is +* protected with either a RCU read lock or the hash table lock. * */ static struct netlbl_dom_map *netlbl_domhsh_search_def (const char *domain)
@@ -169,7 +174,7 @@ static struct netlbl_dom_map *netlbl_domhsh_search_def(const char *domain) entry =netlbl_domhsh_search (domain);if (entry ==NULL ) { - entry =rcu_dereference (netlbl_domhsh_def); + entry =netlbl_domhsh_rcu_deref (netlbl_domhsh_def);if (entry !=NULL && !entry->valid) entry =NULL ; }
@@ -306,8 +311,11 @@ int netlbl_domhsh_add(struct netlbl_dom_map *entry,struct netlbl_af6list *tmp6;#endif /* IPv6 */ +/* XXX - we can remove this RCU read lock as the spinlock protects the +* entire function, but before we do we need to fixup the +* netlbl_af[4,6]list RCU functions to do "the right thing" with +* respect to rcu_dereference() when only a spinlock is held. */ rcu_read_lock (); -spin_lock (&netlbl_domhsh_lock);if (entry->domain !=NULL ) entry_old =netlbl_domhsh_search (entry->domain);
net/netlabel/netlabel_unlabeled.c +19 -47
145 lines not on this page · raw message
Quoted 18 lines · Paul Moore
The recent changes to add RCU lock verification to
rcu_dereference()calls caught out a problem withnetlbl_unlhsh_hash(), see below.===================================================
[ INFO: suspiciousrcu_dereference_check()usage. ]
---------------------------------------------------
net/netlabel/netlabel_unlabeled.c:246invokedrcu_dereference_check()without protection!This patch fixes this problem as well as others like it in the NetLabel code. Also included in this patch is the identification of future work to eliminate the RCU read lock in
netlbl_domhsh_add(), but in the interest of getting this patch out quickly that work will happen in another patch to be finished later.Thanks to Eric Dumazet and Paul McKenney for their help in understanding the recent RCU changes.
Quoted 9 lines
Signed-off-by Paul Moore <[email protected]>Reported-by David Howells <[email protected]>CC Eric Dumazet <[email protected]>CC Paul E. McKenney <[email protected]>--- net/netlabel/netlabel_domainhash.c | 28 ++++++++++----- net/netlabel/netlabel_unlabeled.c | 66 ++++++++++-------------------------- 2 files changed, 37 insertions(+), 57 deletions(-)
Thanks Paul
From: Eric Dumazet <[email protected]>
Date: Thu, 01 Apr 2010 22:56:47 +0200
Quoted 29 lines · Paul Moore
Quoted 18 lines
The recent changes to add RCU lock verification to
rcu_dereference()calls caught out a problem withnetlbl_unlhsh_hash(), see below.===================================================
[ INFO: suspiciousrcu_dereference_check()usage. ]
---------------------------------------------------
net/netlabel/netlabel_unlabeled.c:246invokedrcu_dereference_check()without protection!This patch fixes this problem as well as others like it in the NetLabel code. Also included in this patch is the identification of future work to eliminate the RCU read lock in
netlbl_domhsh_add(), but in the interest of getting this patch out quickly that work will happen in another patch to be finished later.Thanks to Eric Dumazet and Paul McKenney for their help in understanding the recent RCU changes.
Quoted 9 lines
Signed-off-by Paul Moore <[email protected]>Reported-by David Howells <[email protected]>CC Eric Dumazet <[email protected]>CC Paul E. McKenney <[email protected]>--- net/netlabel/netlabel_domainhash.c | 28 ++++++++++----- net/netlabel/netlabel_unlabeled.c | 66 ++++++++++-------------------------- 2 files changed, 37 insertions(+), 57 deletions(-)
Quoted 1 line
Acked-by Eric Dumazet <[email protected]>
Applied.