Skip to contentSkip to conversation

NetLabel cleanups

PATCH Applied Paul Moore 3 participants 7 messages Jun 7, 2007 – Jun 8, 2007

Subsystems (4)
Networking Network drivers SELinux security module Labeled networking (NetLabel)
lore
Share & tools
lore

Paul Moore

author
#0 lore raw

cover NetLabel cleanups

Two straight-forward, smallish patches against David's net-2.6 tree which fix some kernel unaligned access errors/messages and remove some redundant code. I've tested this patchset on both x86 and ia64 platforms and have not seen any regressions.

I'm not sure about my timing, but if it is too late for 2.6.22 please consider these for 2.6.23.

Thanks.

Signature · 2 lines
paul moore
linux security @ hp
author
#1 lore raw

PATCH 1/2 NetLabel: consolidate the struct socket/sock handling to just struct sock

The current NetLabel code has some redundant APIs which allow both "struct socket" and "struct sock" types to be used; this may have made sense at some point but it is wasteful now. Remove the functions that operate on sockets and convert the callers. Not only does this make the code smaller and more consistent but it pushes the locking burden up to the caller which can be more intelligent about the locks. Also, perform the same conversion (socket to sock) on the SELinux/NetLabel glue code where it make sense.

Signed-off-by Paul Moore <[email protected]>
---
 include/net/cipso_ipv4.h     |   20 ++++++--------------
 include/net/netlabel.h       |   14 +++-----------
 net/ipv4/cipso_ipv4.c        |   41 ++++++++---------------------------------
 net/netlabel/netlabel_kapi.c |   43 +++++++++----------------------------------
 security/selinux/netlabel.c  |   36 +++++++++++++++++++++---------------
 5 files changed, 47 insertions(+), 107 deletions(-)
include/net/cipso_ipv4.h +6 -14
Index: net-2.6_cleanup/include/net/cipso_ipv4.h
===================================================================
@@ -203,12 +203,10 @@ static inline int cipso_v4_cache_add(con
 
 #ifdef CONFIG_NETLABEL
 void cipso_v4_error(struct sk_buff *skb, int error, u32 gateway);
-int cipso_v4_socket_setattr(const struct socket *sock,
-			    const struct cipso_v4_doi *doi_def,
-			    const struct netlbl_lsm_secattr *secattr);
+int cipso_v4_sock_setattr(struct sock *sk,
+			  const struct cipso_v4_doi *doi_def,
+			  const struct netlbl_lsm_secattr *secattr);
 int cipso_v4_sock_getattr(struct sock *sk, struct netlbl_lsm_secattr *secattr);
-int cipso_v4_socket_getattr(const struct socket *sock,
-			    struct netlbl_lsm_secattr *secattr);
 int cipso_v4_skbuff_getattr(const struct sk_buff *skb,
 			    struct netlbl_lsm_secattr *secattr);
 int cipso_v4_validate(unsigned char **option);

@@ -220,9 +218,9 @@ static inline void cipso_v4_error(struct
 	return;
 }
 
-static inline int cipso_v4_socket_setattr(const struct socket *sock,
-				  const struct cipso_v4_doi *doi_def,
-				  const struct netlbl_lsm_secattr *secattr)
+static inline int cipso_v4_sock_setattr(struct sock *sk,
+				      const struct cipso_v4_doi *doi_def,
+				      const struct netlbl_lsm_secattr *secattr)
 {
 	return -ENOSYS;
 }

@@ -233,12 +231,6 @@ static inline int cipso_v4_sock_getattr(
 	return -ENOSYS;
 }
 
-static inline int cipso_v4_socket_getattr(const struct socket *sock,
-					  struct netlbl_lsm_secattr *secattr)
-{
-	return -ENOSYS;
-}
-
 static inline int cipso_v4_skbuff_getattr(const struct sk_buff *skb,
 					  struct netlbl_lsm_secattr *secattr)
 {
include/net/netlabel.h +3 -11
Index: net-2.6_cleanup/include/net/netlabel.h
===================================================================
@@ -332,17 +332,15 @@ static inline int netlbl_secattr_catmap_
  */
 
 #ifdef CONFIG_NETLABEL
-int netlbl_socket_setattr(const struct socket *sock,
-			  const struct netlbl_lsm_secattr *secattr);
+int netlbl_sock_setattr(struct sock *sk,
+			const struct netlbl_lsm_secattr *secattr);
 int netlbl_sock_getattr(struct sock *sk,
 			struct netlbl_lsm_secattr *secattr);
-int netlbl_socket_getattr(const struct socket *sock,
-			  struct netlbl_lsm_secattr *secattr);
 int netlbl_skbuff_getattr(const struct sk_buff *skb,
 			  struct netlbl_lsm_secattr *secattr);
 void netlbl_skbuff_err(struct sk_buff *skb, int error);
 #else
-static inline int netlbl_socket_setattr(const struct socket *sock,
+static inline int netlbl_sock_setattr(struct sock *sk,
 				     const struct netlbl_lsm_secattr *secattr)
 {
 	return -ENOSYS;

@@ -354,12 +352,6 @@ static inline int netlbl_sock_getattr(st
 	return -ENOSYS;
 }
 
-static inline int netlbl_socket_getattr(const struct socket *sock,
-					struct netlbl_lsm_secattr *secattr)
-{
-	return -ENOSYS;
-}
-
 static inline int netlbl_skbuff_getattr(const struct sk_buff *skb,
 					struct netlbl_lsm_secattr *secattr)
 {
net/ipv4/cipso_ipv4.c +8 -33

73 lines not on this page · raw message

net/netlabel/netlabel_kapi.c +9 -34

67 lines not on this page · raw message

security/selinux/netlabel.c +21 -15
Index: net-2.6_cleanup/security/selinux/netlabel.c
===================================================================
@@ -36,8 +36,8 @@
 #include "security.h"
 
 /**
- * selinux_netlbl_socket_setsid - Label a socket using the NetLabel mechanism
- * @sock: the socket to label
+ * selinux_netlbl_sock_setsid - Label a socket using the NetLabel mechanism
+ * @sk: the socket to label
  * @sid: the SID to use
  *
  * Description:

@@ -47,17 +47,17 @@
  * this function and rcu_read_unlock() after this function returns.
  *
  */
-static int selinux_netlbl_socket_setsid(struct socket *sock, u32 sid)
+static int selinux_netlbl_sock_setsid(struct sock *sk, u32 sid)
 {
 	int rc;
-	struct sk_security_struct *sksec = sock->sk->sk_security;
+	struct sk_security_struct *sksec = sk->sk_security;
 	struct netlbl_lsm_secattr secattr;
 
 	rc = security_netlbl_sid_to_secattr(sid, &secattr);
 	if (rc != 0)
 		return rc;
 
-	rc = netlbl_socket_setattr(sock, &secattr);
+	rc = netlbl_sock_setattr(sk, &secattr);
 	if (rc == 0) {
 		spin_lock_bh(&sksec->nlbl_lock);
 		sksec->nlbl_state = NLBL_LABELED;

@@ -206,7 +206,7 @@ void selinux_netlbl_sock_graft(struct so
 	/* Try to set the NetLabel on the socket to save time later, if we fail
 	 * here we will pick up the pieces in later calls to
 	 * selinux_netlbl_inode_permission(). */
-	selinux_netlbl_socket_setsid(sock, sksec->sid);
+	selinux_netlbl_sock_setsid(sk, sksec->sid);
 
 	rcu_read_unlock();
 }

@@ -223,14 +223,15 @@ void selinux_netlbl_sock_graft(struct so
 int selinux_netlbl_socket_post_create(struct socket *sock)
 {
 	int rc = 0;
+	struct sock *sk = sock->sk;
 	struct inode_security_struct *isec = SOCK_INODE(sock)->i_security;
-	struct sk_security_struct *sksec = sock->sk->sk_security;
+	struct sk_security_struct *sksec = sk->sk_security;
 
 	sksec->sclass = isec->sclass;
 
 	rcu_read_lock();
 	if (sksec->nlbl_state == NLBL_REQUIRE)
-		rc = selinux_netlbl_socket_setsid(sock, sksec->sid);
+		rc = selinux_netlbl_sock_setsid(sk, sksec->sid);
 	rcu_read_unlock();
 
 	return rc;

@@ -251,14 +252,16 @@ int selinux_netlbl_socket_post_create(st
 int selinux_netlbl_inode_permission(struct inode *inode, int mask)
 {
 	int rc;
-	struct sk_security_struct *sksec;
+	struct sock *sk;
 	struct socket *sock;
+	struct sk_security_struct *sksec;
 
 	if (!S_ISSOCK(inode->i_mode) ||
 	    ((mask & (MAY_WRITE | MAY_APPEND)) == 0))
 		return 0;
 	sock = SOCKET_I(inode);
-	sksec = sock->sk->sk_security;
+	sk = sock->sk;
+	sksec = sk->sk_security;
 
 	rcu_read_lock();
 	if (sksec->nlbl_state != NLBL_REQUIRE) {

@@ -266,9 +269,9 @@ int selinux_netlbl_inode_permission(stru
 		return 0;
 	}
 	local_bh_disable();
-	bh_lock_sock_nested(sock->sk);
-	rc = selinux_netlbl_socket_setsid(sock, sksec->sid);
-	bh_unlock_sock(sock->sk);
+	bh_lock_sock_nested(sk);
+	rc = selinux_netlbl_sock_setsid(sk, sksec->sid);
+	bh_unlock_sock(sk);
 	local_bh_enable();
 	rcu_read_unlock();
 

@@ -345,14 +348,17 @@ int selinux_netlbl_socket_setsockopt(str
 				     int optname)
 {
 	int rc = 0;
-	struct sk_security_struct *sksec = sock->sk->sk_security;
+	struct sock *sk = sock->sk;
+	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);
-		rc = netlbl_socket_getattr(sock, &secattr);
+		lock_sock(sk);
+		rc = netlbl_sock_getattr(sk, &secattr);
+		release_sock(sk);
 		if (rc == 0 && secattr.flags != NETLBL_SECATTR_NONE)
 			rc = -EACCES;
 		netlbl_secattr_destroy(&secattr);
Signature · 2 lines
paul moore
linux security @ hp
Acked-by on v1 1/2
#2 lore raw

Re: [PATCH 1/2] NetLabel: consolidate the struct socket/sock handling to just struct sock

Quoted 9 lines · Paul Moore

The current NetLabel code has some redundant APIs which allow both "struct socket" and "struct sock" types to be used; this may have made sense at some point but it is wasteful now. Remove the functions that operate on sockets and convert the callers. Not only does this make the code smaller and more consistent but it pushes the locking burden up to the caller which can be more intelligent about the locks. Also, perform the same conversion (socket to sock) on the SELinux/NetLabel glue code where it make sense.

Signed-off-by Paul Moore <[email protected]>
Acked-by James Morris <[email protected]>
maintainer Applied
#3 lore raw

Re: [PATCH 1/2] NetLabel: consolidate the struct socket/sock handling to just struct sock

From: James Morris <[email protected]>
Date: Thu, 7 Jun 2007 21:18:26 -0400 (EDT)

Quoted 10 lines · Paul Moore
Quoted 9 lines

The current NetLabel code has some redundant APIs which allow both "struct socket" and "struct sock" types to be used; this may have made sense at some point but it is wasteful now. Remove the functions that operate on sockets and convert the callers. Not only does this make the code smaller and more consistent but it pushes the locking burden up to the caller which can be more intelligent about the locks. Also, perform the same conversion (socket to sock) on the SELinux/NetLabel glue code where it make sense.

Quoted 3 lines
Quoted 1 line
Signed-off-by Paul Moore <[email protected]>
Acked-by James Morris <[email protected]>

Applied.

author
#4 lore raw

PATCH 2/2 fix several unaligned kernel accesses in the CIPSO engine

In reply to Paul Moore

IPv4 options are not very well aligned within the packet and the format of a CIPSO option is even worse. The result is that the CIPSO engine in the kernel does a few unaligned accesses when parsing and validating incoming packets with CIPSO options attached which generate error messages on certain alignment sensitive platforms. This patch fixes this by marking these unaligned accesses with the get_unaliagned() macro.

Signed-off-by Paul Moore <[email protected]>
---
 net/ipv4/cipso_ipv4.c |   23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)
net/ipv4/cipso_ipv4.c +13 -10
Index: net-2.6_cleanup/net/ipv4/cipso_ipv4.c
===================================================================
@@ -45,6 +45,7 @@
 #include <net/cipso_ipv4.h>
 #include <asm/atomic.h>
 #include <asm/bug.h>
+#include <asm/unaligned.h>
 
 struct cipso_v4_domhsh_entry {
 	char *domain;

@@ -1000,7 +1001,7 @@ static int cipso_v4_map_cat_enum_valid(c
 		return -EFAULT;
 
 	for (iter = 0; iter < enumcat_len; iter += 2) {
-		cat = ntohs(*((__be16 *)&enumcat[iter]));
+		cat = ntohs(get_unaligned((__be16 *)&enumcat[iter]));
 		if (cat <= cat_prev)
 			return -EFAULT;
 		cat_prev = cat;

@@ -1068,8 +1069,8 @@ static int cipso_v4_map_cat_enum_ntoh(co
 
 	for (iter = 0; iter < net_cat_len; iter += 2) {
 		ret_val = netlbl_secattr_catmap_setbit(secattr->mls_cat,
-					    ntohs(*((__be16 *)&net_cat[iter])),
-					    GFP_ATOMIC);
+				ntohs(get_unaligned((__be16 *)&net_cat[iter])),
+				GFP_ATOMIC);
 		if (ret_val != 0)
 			return ret_val;
 	}

@@ -1102,9 +1103,10 @@ static int cipso_v4_map_cat_rng_valid(co
 		return -EFAULT;
 
 	for (iter = 0; iter < rngcat_len; iter += 4) {
-		cat_high = ntohs(*((__be16 *)&rngcat[iter]));
+		cat_high = ntohs(get_unaligned((__be16 *)&rngcat[iter]));
 		if ((iter + 4) <= rngcat_len)
-			cat_low = ntohs(*((__be16 *)&rngcat[iter + 2]));
+			cat_low = ntohs(
+				get_unaligned((__be16 *)&rngcat[iter + 2]));
 		else
 			cat_low = 0;
 

@@ -1201,9 +1203,10 @@ static int cipso_v4_map_cat_rng_ntoh(con
 	u16 cat_high;
 
 	for (net_iter = 0; net_iter < net_cat_len; net_iter += 4) {
-		cat_high = ntohs(*((__be16 *)&net_cat[net_iter]));
+		cat_high = ntohs(get_unaligned((__be16 *)&net_cat[net_iter]));
 		if ((net_iter + 4) <= net_cat_len)
-			cat_low = ntohs(*((__be16 *)&net_cat[net_iter + 2]));
+			cat_low = ntohs(
+			      get_unaligned((__be16 *)&net_cat[net_iter + 2]));
 		else
 			cat_low = 0;
 

@@ -1565,7 +1568,7 @@ int cipso_v4_validate(unsigned char **op
 	}
 
 	rcu_read_lock();
-	doi_def = cipso_v4_doi_search(ntohl(*((__be32 *)&opt[2])));
+	doi_def = cipso_v4_doi_search(ntohl(get_unaligned((__be32 *)&opt[2])));
 	if (doi_def == NULL) {
 		err_offset = 2;
 		goto validate_return_locked;

@@ -1856,7 +1859,7 @@ int cipso_v4_sock_getattr(struct sock *s
 	if (ret_val == 0)
 		return ret_val;
 
-	doi = ntohl(*(__be32 *)&cipso_ptr[2]);
+	doi = ntohl(get_unaligned((__be32 *)&cipso_ptr[2]));
 	rcu_read_lock();
 	doi_def = cipso_v4_doi_search(doi);
 	if (doi_def == NULL) {

@@ -1911,7 +1914,7 @@ int cipso_v4_skbuff_getattr(const struct
 	if (cipso_v4_cache_check(cipso_ptr, cipso_ptr[1], secattr) == 0)
 		return 0;
 
-	doi = ntohl(*(__be32 *)&cipso_ptr[2]);
+	doi = ntohl(get_unaligned((__be32 *)&cipso_ptr[2]));
 	rcu_read_lock();
 	doi_def = cipso_v4_doi_search(doi);
 	if (doi_def == NULL)
Signature · 2 lines
paul moore
linux security @ hp
Acked-by on v1 2/2
#5 lore raw

Re: [PATCH 2/2] fix several unaligned kernel accesses in the CIPSO engine

Quoted 8 lines · Paul Moore

IPv4 options are not very well aligned within the packet and the format of a CIPSO option is even worse. The result is that the CIPSO engine in the kernel does a few unaligned accesses when parsing and validating incoming packets with CIPSO options attached which generate error messages on certain alignment sensitive platforms. This patch fixes this by marking these unaligned accesses with the get_unaliagned() macro.

Signed-off-by Paul Moore <[email protected]>
Acked-by James Morris <[email protected]>
maintainer
#6 lore raw

Re: [PATCH 2/2] fix several unaligned kernel accesses in the CIPSO engine

From: James Morris <[email protected]>
Date: Thu, 7 Jun 2007 21:20:20 -0400 (EDT)

Quoted 9 lines · Paul Moore
Quoted 8 lines

IPv4 options are not very well aligned within the packet and the format of a CIPSO option is even worse. The result is that the CIPSO engine in the kernel does a few unaligned accesses when parsing and validating incoming packets with CIPSO options attached which generate error messages on certain alignment sensitive platforms. This patch fixes this by marking these unaligned accesses with the get_unaliagned() macro.

Quoted 4 lines
Quoted 1 line
Signed-off-by Paul Moore <[email protected]>
Acked-by James Morris <[email protected]>

Also applied, thanks.