Skip to content

Commit

Permalink
SELinux: Correct the NetLabel locking for the sk_security_struct
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
Paul Moore authored and James Morris committed Apr 18, 2008
1 parent 4b119e2 commit f74af6e
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 87 deletions.
4 changes: 2 additions & 2 deletions security/selinux/hooks.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ static int sk_alloc_security(struct sock *sk, int family, gfp_t priority)
ssec->sid = SECINITSID_UNLABELED;
sk->sk_security = ssec;

selinux_netlbl_sk_security_init(ssec, family);
selinux_netlbl_sk_security_reset(ssec, family);

return 0;
}
Expand Down Expand Up @@ -4139,7 +4139,7 @@ static void selinux_sk_clone_security(const struct sock *sk, struct sock *newsk)
newssec->peer_sid = ssec->peer_sid;
newssec->sclass = ssec->sclass;

selinux_netlbl_sk_security_clone(ssec, newssec);
selinux_netlbl_sk_security_reset(newssec, newsk->sk_family);
}

static void selinux_sk_getsecid(struct sock *sk, u32 *secid)
Expand Down
16 changes: 0 additions & 16 deletions security/selinux/include/netlabel.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@ void selinux_netlbl_cache_invalidate(void);

void selinux_netlbl_sk_security_reset(struct sk_security_struct *ssec,
int family);
void selinux_netlbl_sk_security_init(struct sk_security_struct *ssec,
int family);
void selinux_netlbl_sk_security_clone(struct sk_security_struct *ssec,
struct sk_security_struct *newssec);

int selinux_netlbl_skbuff_getsid(struct sk_buff *skb,
u16 family,
Expand Down Expand Up @@ -73,18 +69,6 @@ static inline void selinux_netlbl_sk_security_reset(
{
return;
}
static inline void selinux_netlbl_sk_security_init(
struct sk_security_struct *ssec,
int family)
{
return;
}
static inline void selinux_netlbl_sk_security_clone(
struct sk_security_struct *ssec,
struct sk_security_struct *newssec)
{
return;
}

static inline int selinux_netlbl_skbuff_getsid(struct sk_buff *skb,
u16 family,
Expand Down
1 change: 0 additions & 1 deletion security/selinux/include/objsec.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ struct sk_security_struct {
NLBL_REQUIRE,
NLBL_LABELED,
} nlbl_state;
spinlock_t nlbl_lock; /* protects nlbl_state */
#endif
};

Expand Down
81 changes: 13 additions & 68 deletions security/selinux/netlabel.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,7 @@ static int selinux_netlbl_sidlookup_cached(struct sk_buff *skb,
*
* 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)
Expand All @@ -86,11 +84,8 @@ static int selinux_netlbl_sock_setsid(struct sock *sk, u32 sid)
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);
Expand Down Expand Up @@ -128,45 +123,6 @@ void selinux_netlbl_sk_security_reset(struct sk_security_struct *ssec,
ssec->nlbl_state = NLBL_UNSET;
}

/**
* 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
Expand Down Expand Up @@ -221,12 +177,8 @@ void selinux_netlbl_sock_graft(struct sock *sk, struct socket *sock)
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 &&
Expand All @@ -239,8 +191,6 @@ void selinux_netlbl_sock_graft(struct sock *sk, struct socket *sock)
* 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();
}

/**
Expand All @@ -254,16 +204,13 @@ void selinux_netlbl_sock_graft(struct sock *sk, struct socket *sock)
*/
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);
}

/**
Expand All @@ -288,21 +235,21 @@ int selinux_netlbl_inode_permission(struct inode *inode, int mask)
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;
}
Expand Down Expand Up @@ -385,7 +332,6 @@ int selinux_netlbl_socket_setsockopt(struct socket *sock,
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);
Expand All @@ -396,7 +342,6 @@ int selinux_netlbl_socket_setsockopt(struct socket *sock,
rc = -EACCES;
netlbl_secattr_destroy(&secattr);
}
rcu_read_unlock();

return rc;
}

0 comments on commit f74af6e

Please sign in to comment.