SELinux: Correct the NetLabel locking for the sk_security_struct
The RCU/spinlock locking approach for the nlbl_state in the sk_security_struct
was almost certainly overkill. This patch removes both the RCU and spinlock
locking, relying on the existing socket locks to handle the case of multiple
writers. This change also makes several code reductions possible.
Less locking, less code - it's a Good Thing.
Signed-off-by: Paul Moore <paul.moore@hp.com>
Signed-off-by: James Morris <jmorris@namei.org>
diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c
index 0fa2be4..ccf71f6 100644
--- a/security/selinux/netlabel.c
+++ b/security/selinux/netlabel.c
@@ -69,9 +69,7 @@
*
* Description:
* Attempt to label a socket using the NetLabel mechanism using the given
- * SID. Returns zero values on success, negative values on failure. The
- * caller is responsibile for calling rcu_read_lock() before calling this
- * this function and rcu_read_unlock() after this function returns.
+ * SID. Returns zero values on success, negative values on failure.
*
*/
static int selinux_netlbl_sock_setsid(struct sock *sk, u32 sid)
@@ -86,11 +84,8 @@
if (rc != 0)
goto sock_setsid_return;
rc = netlbl_sock_setattr(sk, &secattr);
- if (rc == 0) {
- spin_lock_bh(&sksec->nlbl_lock);
+ if (rc == 0)
sksec->nlbl_state = NLBL_LABELED;
- spin_unlock_bh(&sksec->nlbl_lock);
- }
sock_setsid_return:
netlbl_secattr_destroy(&secattr);
@@ -129,45 +124,6 @@
}
/**
- * selinux_netlbl_sk_security_init - Setup the NetLabel fields
- * @ssec: the sk_security_struct
- * @family: the socket family
- *
- * Description:
- * Called when a new sk_security_struct is allocated to initialize the NetLabel
- * fields.
- *
- */
-void selinux_netlbl_sk_security_init(struct sk_security_struct *ssec,
- int family)
-{
- /* No locking needed, we are the only one who has access to ssec */
- selinux_netlbl_sk_security_reset(ssec, family);
- spin_lock_init(&ssec->nlbl_lock);
-}
-
-/**
- * selinux_netlbl_sk_security_clone - Copy the NetLabel fields
- * @ssec: the original sk_security_struct
- * @newssec: the cloned sk_security_struct
- *
- * Description:
- * Clone the NetLabel specific sk_security_struct fields from @ssec to
- * @newssec.
- *
- */
-void selinux_netlbl_sk_security_clone(struct sk_security_struct *ssec,
- struct sk_security_struct *newssec)
-{
- /* We don't need to take newssec->nlbl_lock because we are the only
- * thread with access to newssec, but we do need to take the RCU read
- * lock as other threads could have access to ssec */
- rcu_read_lock();
- selinux_netlbl_sk_security_reset(newssec, ssec->sk->sk_family);
- rcu_read_unlock();
-}
-
-/**
* selinux_netlbl_skbuff_getsid - Get the sid of a packet using NetLabel
* @skb: the packet
* @family: protocol family
@@ -221,12 +177,8 @@
struct netlbl_lsm_secattr secattr;
u32 nlbl_peer_sid;
- rcu_read_lock();
-
- if (sksec->nlbl_state != NLBL_REQUIRE) {
- rcu_read_unlock();
+ if (sksec->nlbl_state != NLBL_REQUIRE)
return;
- }
netlbl_secattr_init(&secattr);
if (netlbl_sock_getattr(sk, &secattr) == 0 &&
@@ -239,8 +191,6 @@
* here we will pick up the pieces in later calls to
* selinux_netlbl_inode_permission(). */
selinux_netlbl_sock_setsid(sk, sksec->sid);
-
- rcu_read_unlock();
}
/**
@@ -254,16 +204,13 @@
*/
int selinux_netlbl_socket_post_create(struct socket *sock)
{
- int rc = 0;
struct sock *sk = sock->sk;
struct sk_security_struct *sksec = sk->sk_security;
- rcu_read_lock();
- if (sksec->nlbl_state == NLBL_REQUIRE)
- rc = selinux_netlbl_sock_setsid(sk, sksec->sid);
- rcu_read_unlock();
+ if (sksec->nlbl_state != NLBL_REQUIRE)
+ return 0;
- return rc;
+ return selinux_netlbl_sock_setsid(sk, sksec->sid);
}
/**
@@ -288,21 +235,21 @@
if (!S_ISSOCK(inode->i_mode) ||
((mask & (MAY_WRITE | MAY_APPEND)) == 0))
return 0;
+
sock = SOCKET_I(inode);
sk = sock->sk;
sksec = sk->sk_security;
-
- rcu_read_lock();
- if (sksec->nlbl_state != NLBL_REQUIRE) {
- rcu_read_unlock();
+ if (sksec->nlbl_state != NLBL_REQUIRE)
return 0;
- }
+
local_bh_disable();
bh_lock_sock_nested(sk);
- rc = selinux_netlbl_sock_setsid(sk, sksec->sid);
+ if (likely(sksec->nlbl_state == NLBL_REQUIRE))
+ rc = selinux_netlbl_sock_setsid(sk, sksec->sid);
+ else
+ rc = 0;
bh_unlock_sock(sk);
local_bh_enable();
- rcu_read_unlock();
return rc;
}
@@ -385,7 +332,6 @@
struct sk_security_struct *sksec = sk->sk_security;
struct netlbl_lsm_secattr secattr;
- rcu_read_lock();
if (level == IPPROTO_IP && optname == IP_OPTIONS &&
sksec->nlbl_state == NLBL_LABELED) {
netlbl_secattr_init(&secattr);
@@ -396,7 +342,6 @@
rc = -EACCES;
netlbl_secattr_destroy(&secattr);
}
- rcu_read_unlock();
return rc;
}