Date:	Thu, 05 Feb 2015 06:05:37 +0000 (GMT)
From:	Vishal Goel <vishal.goel@...sung.com>
To:	"linux-security-module@...r.kernel.org" 
	<linux-security-module@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"casey@...aufler-ca.com" <casey@...aufler-ca.com>
Cc:	"himanshu.sh@...sung.com" <himanshu.sh@...sung.com>
Subject: [PATCH 3/3] smack : Fixes the undesired smack label update in IPv6
 code when a second bind call is made to same IP and Port by second instance of
 server (patches based on smack-for-3.20-0 branch)

From 2ac41a9bbbf267c33d1741d94f28aff2369b5cc1 Mon Sep 17 00:00:00 2001
From: Vishal Goel <vishal.goel@...sung.com>
Date: Wed, 4 Feb 2015 22:59:50 +0530
Subject: This patch fixes the undesired SMACK label (SMACK64IPIN)
 update when a second bind call is made to same IP address & port, but with
 different SMACK label (SMACK64IPIN) by second instance of server. In this
 case server returns with "Bind:Address already in use" error but before
 returning, SMACK label is updated in SMACK port-label mapping list inside
 "smack_socket_bind()" hook.

To fix the issue a new check has been added in smk_ipv6_port_label() function
before updating the existing port entry. It checks whether the socket for corresponding
port entry is closed or not. If it is closed then it means port is not bound and it is
safe to update the existing port entry else return if port is still getting used.
For checking whether socket is closed or not, one more field "smk_can_reuse" has been
added in the "smk_port_label" structure. This field will be set to '1' in "smack_sk_free_security()"
function which is called to free the socket security blob when the socket is being closed.
In this function, port entry is searched in the SMACK port-label mapping list for the closing socket.
If entry is found then "smk_can_reuse" field is set to '1'.Initially "smk_can_reuse" field is
set to '0' in smk_ipv6_port_label() function after creating a new entry in the list which
indicates that socket is in use.

Signed-off-by: Vishal Goel <vishal.goel@...sung.com>
Signed-off-by: Himanshu Shukla <himanshu.sh@...sung.com>
---
 security/smack/smack.h     |  1 +
 security/smack/smack_lsm.c | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/security/smack/smack.h b/security/smack/smack.h
index 781d6fd..3f95f36 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -136,6 +136,7 @@ struct smk_port_label {
 	struct smack_known	*smk_in;	/* inbound label */
 	struct smack_known	*smk_out;	/* outgoing label */
 	short                   sock_type;      /*Socket type*/
+	short                   smk_can_reuse;
 };
 
 /*
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 5529a52..590d56e 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2092,6 +2092,18 @@ static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t gfp_flags)
  */
 static void smack_sk_free_security(struct sock *sk)
 {
+	struct smk_port_label *spp;
+
+	if (sk->sk_family == PF_INET6) {
+		rcu_read_lock();
+		list_for_each_entry_rcu(spp, &smk_ipv6_port_list, list) {
+			if (spp->smk_sock != sk)
+				continue;
+			spp->smk_can_reuse = 1;
+			break;
+		}
+		rcu_read_unlock();
+	}
 	kfree(sk->sk_security);
 }
 
@@ -2274,10 +2286,15 @@ static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address)
 	list_for_each_entry_rcu(spp, &smk_ipv6_port_list, list) {
 		if (spp->smk_port != port || spp->sock_type != sock->type)
 			continue;
+		if (spp->smk_can_reuse != 1) {
+			rcu_read_unlock();
+			return;
+		}
 		spp->smk_port = port;
 		spp->smk_sock = sk;
 		spp->smk_in = ssp->smk_in;
 		spp->smk_out = ssp->smk_out;
+		spp->smk_can_reuse = 0;
 		rcu_read_unlock();
 		return;
 	}
@@ -2294,6 +2311,7 @@ static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address)
 	spp->smk_in = ssp->smk_in;
 	spp->smk_out = ssp->smk_out;
 	spp->sock_type = sock->type;
+	spp->smk_can_reuse = 0;
 
 	mutex_lock(&smack_ipv6_lock);
 	list_add_rcu(&spp->list, &smk_ipv6_port_list);
-- 
1.8.3.2

------- Original Message -------
Sender : Vishal Goel<vishal.goel@...sung.com> Senior Software Engineer (2)/SRI-Delhi-Linux/Samsung Electronics
Date : Feb 05, 2015 15:01 (GMT+09:00)
Title : [PATCH 2/3] smack : Fixes the undesired smack label update when 2 servers are run with different protocols(tcp,udp)

From b32429fe2ff2f1fbfcf2a939f9ff9e2e798d7e72 Mon Sep 17 00:00:00 2001
From: Vishal Goel 
Date: Wed, 4 Feb 2015 19:45:08 +0530
Subject: This patch fixes the issue of "permission denied error" which
comes when 2 IPv6 servers are running and client tries to connect one of
them. Scenario is that both servers are using same IP and port but different
protocols(Udp and tcp). They are using different SMACK64IPIN labels.Tcp
server is using "test" and udp server is using "test-in". When we try to run
tcp client with SMACK64IPOUT label as "test", then connection denied error
comes. It should not happen since both tcp server and client labels are same.
This happens because there is no check for protocol in smk_ipv6_port_label()
function while searching for the earlier port entry. It checks whether there
is an existing port entry on the basis of port only. So it updates the
earlier port entry in the list. Due to which smack label gets changed for
earlier entry in the "smk_ipv6_port_list" list and permission denied error
comes.

Now a check is added for socket type also.Now if 2 processes use same
port  but different protocols (tcp or udp), then 2 different port entries
will be  added in the list. Similarly while checking smack access in
smk_ipv6_port_check() function,  port entry is searched on the basis of both
port and protocol.

Signed-off-by: Vishal Goel 
Signed-off-by: Himanshu Shukla 
---
security/smack/smack.h     | 1 +
security/smack/smack_lsm.c | 5 +++--
2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/security/smack/smack.h b/security/smack/smack.h
index 7629eae..781d6fd 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -135,6 +135,7 @@ struct smk_port_label {
unsigned short smk_port; /* the port number */
struct smack_known *smk_in; /* inbound label */
struct smack_known *smk_out; /* outgoing label */
+ short                   sock_type;      /*Socket type*/
};

/*
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 579a177..5529a52 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2272,7 +2272,7 @@ static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address)
*/
rcu_read_lock();
list_for_each_entry_rcu(spp, &smk_ipv6_port_list, list) {
- if (spp->smk_port != port)
+ if (spp->smk_port != port || spp->sock_type != sock->type)
continue;
spp->smk_port = port;
spp->smk_sock = sk;
@@ -2293,6 +2293,7 @@ static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address)
spp->smk_sock = sk;
spp->smk_in = ssp->smk_in;
spp->smk_out = ssp->smk_out;
+ spp->sock_type = sock->type;

mutex_lock(&smack_ipv6_lock);
list_add_rcu(&spp->list, &smk_ipv6_port_list);
@@ -2354,7 +2355,7 @@ static int smk_ipv6_port_check(struct sock *sk, struct sockaddr_in6 *address,

rcu_read_lock();
list_for_each_entry_rcu(spp, &smk_ipv6_port_list, list) {
- if (spp->smk_port != port)
+ if (spp->smk_port != port || spp->sock_type != sk->sk_type)
continue;
object = spp->smk_in;
if (act == SMK_CONNECTING)
-- 
1.8.3.2

------- Original Message -------
Sender : Vishal Goel Senior Software Engineer (2)/SRI-Delhi-Linux/Samsung Electronics
Date : Feb 05, 2015 14:55 (GMT+09:00)
Title : [PATCH 1/3] smack : Adds the synchronization mechanism in smack IPv6 hooks

From 875727546f9ba0d3a98a906cff07fd710d72cadc Mon Sep 17 00:00:00 2001
From: Vishal Goel 
Date: Wed, 4 Feb 2015 03:02:55 +0530
Subject: This patch adds the rcu synchronization mechanism in SMACK
IPv6 hooks while accessing smk_ipv6_port_list. Access to the port list is
vulnerable to a race condition issue, it does not apply proper
synchronization methods while working on critical section. It is possible
that when one thread is reading the list, at the same time another thread is
modifying the same port list, which can cause the major problems. To ensure
proper synchronization between two threads, rcu mechanism has been applied
while accessing and modifying the port list. RCU will also not affect the
performance as in access control module there are more accesses than
modification where RCU is most effective synchronization mechanism.

Signed-off-by: Vishal Goel 
Signed-off-by: Himanshu Shukla 
---
security/smack/smack_lsm.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index a688f7b..579a177 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -53,6 +53,7 @@
#define SMK_SENDING 2

#ifndef CONFIG_SECURITY_SMACK_NETFILTER
+DEFINE_MUTEX(smack_ipv6_lock);
LIST_HEAD(smk_ipv6_port_list);
#endif
static struct kmem_cache *smack_inode_cache;
@@ -2240,17 +2241,20 @@ static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address)
* on the bound socket. Take the changes to the port
* as well.
*/
- list_for_each_entry(spp, &smk_ipv6_port_list, list) {
+ rcu_read_lock();
+ list_for_each_entry_rcu(spp, &smk_ipv6_port_list, list) {
if (sk != spp->smk_sock)
continue;
spp->smk_in = ssp->smk_in;
spp->smk_out = ssp->smk_out;
+ rcu_read_unlock();
return;
}
/*
* A NULL address is only used for updating existing
* bound entries. If there isn't one, it's OK.
*/
+ rcu_read_unlock();
return;
}

@@ -2266,16 +2270,18 @@ static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address)
* Look for an existing port list entry.
* This is an indication that a port is getting reused.
*/
- list_for_each_entry(spp, &smk_ipv6_port_list, list) {
+ rcu_read_lock();
+ list_for_each_entry_rcu(spp, &smk_ipv6_port_list, list) {
if (spp->smk_port != port)
continue;
spp->smk_port = port;
spp->smk_sock = sk;
spp->smk_in = ssp->smk_in;
spp->smk_out = ssp->smk_out;
+ rcu_read_unlock();
return;
}
-
+ rcu_read_unlock();
/*
* A new port entry is required.
*/
@@ -2288,7 +2294,9 @@ static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address)
spp->smk_in = ssp->smk_in;
spp->smk_out = ssp->smk_out;

- list_add(&spp->list, &smk_ipv6_port_list);
+ mutex_lock(&smack_ipv6_lock);
+ list_add_rcu(&spp->list, &smk_ipv6_port_list);
+ mutex_unlock(&smack_ipv6_lock);
return;
}

@@ -2344,7 +2352,8 @@ static int smk_ipv6_port_check(struct sock *sk, struct sockaddr_in6 *address,
goto auditout;
}

- list_for_each_entry(spp, &smk_ipv6_port_list, list) {
+ rcu_read_lock();
+ list_for_each_entry_rcu(spp, &smk_ipv6_port_list, list) {
if (spp->smk_port != port)
continue;
object = spp->smk_in;
@@ -2352,6 +2361,7 @@ static int smk_ipv6_port_check(struct sock *sk, struct sockaddr_in6 *address,
ssp->smk_packet = spp->smk_out;
break;
}
+ rcu_read_unlock();

auditout:

-- 
1.8.3.2

Powered by blists - more mailing lists