Skip to contentSkip to conversation

(2.6.24-rc8-mm1) -mm v2 Smack socket label setting fix

PATCH In review Casey Schaufler 2 participants 2 messages Jan 19, 2008 – Jan 22, 2008

Subsystems (2)
Security (LSM) Smack security module
lore
Share & tools
lore

Casey Schaufler

author
#0 lore raw

PATCH 1/1 (2.6.24-rc8-mm1) -mm v2 Smack socket label setting fix

From: Casey Schaufler <[email protected]>

Correct the checks in smack_inode_setxattr to include the socket labeling attributes. Simplify and correct smack_sock_graft, while the values it was setting were safe they were not correct and the job was not being done efficiently. smack_inode_setsecurity wasn't invoking the required netlabel function in the case where smk_ipout was set. It does now, but that change required the hook to be moved in the file. This movement accounts for the bulk of the patch.

Signed-off-by Casey Schaufler <[email protected]>

---

This version corrects an indentation error in the previous patch.

 security/smack/smack.h     |    2
 security/smack/smack_lsm.c |  136 ++++++++++++++++-------------------
 2 files changed, 67 insertions(+), 71 deletions(-)
security/smack/smack.h +2 -0
diff -uprN -X linux-2.6.24-rc8-mm1-base/Documentation/dontdiff linux-2.6.24-rc8-mm1-base/security/smack/smack.h linux-2.6.24-rc8-mm1-smack/security/smack/smack.h
@@ -130,6 +130,8 @@ struct smack_known {
 #define XATTR_SMACK_IPIN	"SMACK64IPIN"
 #define XATTR_SMACK_IPOUT	"SMACK64IPOUT"
 #define XATTR_NAME_SMACK	XATTR_SECURITY_PREFIX XATTR_SMACK_SUFFIX
+#define XATTR_NAME_SMACKIPIN	XATTR_SECURITY_PREFIX XATTR_SMACK_IPIN
+#define XATTR_NAME_SMACKIPOUT	XATTR_SECURITY_PREFIX XATTR_SMACK_IPOUT
 
 /*
  * smackfs macic number
security/smack/smack_lsm.c +66 -72

170 lines not on this page · raw message

Signature · 4 lines
Casey Schaufler
MontaVista Software
[email protected]
408.572.7881
#1 lore raw
Quoted 14 lines · Casey Schaufler

From: Casey Schaufler <[email protected]>

Correct the checks in smack_inode_setxattr to include the socket labeling attributes. Simplify and correct smack_sock_graft, while the values it was setting were safe they were not correct and the job was not being done efficiently. smack_inode_setsecurity wasn't invoking the required netlabel function in the case where smk_ipout was set. It does now, but that change required the hook to be moved in the file. This movement accounts for the bulk of the patch.

Signed-off-by Casey Schaufler <[email protected]>

...

security/smack/smack_lsm.c static int smack_socket_getpeersec_dgram · line 2202

Quoted 11 lines
security/smack/smack_lsm.c +1 -2
+/**
  * smack_socket_post_create - finish socket setup
  * @sock: the socket
  * @family: protocol family

@@ -2192,33 +2199,20 @@ static int smack_socket_getpeersec_dgram
 static void smack_sock_graft(struct sock *sk, struct socket *parent)
 {
 	struct socket_smack *ssp;
-	struct netlbl_lsm_secattr secattr;
-	char smack[SMK_LABELLEN];
 	int rc;

I don't think you need 'rc'.

security/smack/smack_lsm.c static int smack_socket_getpeersec_dgram · line 2205

Quoted 3 lines
security/smack/smack_lsm.c +1 -1
-	if (sk == NULL || parent == NULL || parent->sk == NULL)
+	if (sk == NULL)
 		return;

I'm pretty sure you don't need to check 'sk' to ensure it is non-NULL; SELinux assumes 'sk' is non-NULL and it hasn't caused any problems.

Quoted 15 lines
security/smack/smack_lsm.c +0 -12
 	if (sk->sk_family != PF_INET && sk->sk_family != PF_INET6)
 		return;
 
-	ssp = parent->sk->sk_security;
-
-	memset(smack, '\0', SMK_LABELLEN);
-	netlbl_secattr_init(&secattr);
-	rc = netlbl_sock_getattr(sk, &secattr);
-	if (rc == 0)
-		smack_from_secattr(&secattr, smack);
-	else
-		strncpy(smack, smack_known_huh.smk_known, SMK_MAXLEN);
-	netlbl_secattr_destroy(&secattr);
-
-	netlbl_secattr_init(&secattr);

security/smack/smack_lsm.c static int smack_socket_getpeersec_dgram · line 2215

Quoted 10 lines
security/smack/smack_lsm.c +5 -4
+	ssp = sk->sk_security;
+	ssp->smk_in = current->security;
+	ssp->smk_out = current->security;
+	ssp->smk_packet[0] = '\0';
 
-	smack_to_secattr(smack, &secattr);
-	if (secattr.flags != NETLBL_SECATTR_NONE)
-		rc = netlbl_sock_setattr(parent->sk, &secattr);
-	netlbl_secattr_destroy(&secattr);
+	rc = smack_netlabel(sk);

I haven't checked the latest SMACK bits, but I'm pretty sure you don't need to assign the return value of 'smack_netlabel()' to anything here since the function doesn't return a value.

Quoted 3 lines
(quoted diff) +0 -0
 }
 
 /**
Signature · 2 lines
paul moore
linux security @ hp