From b32429fe2ff2f1fbfcf2a939f9ff9e2e798d7e72 Mon Sep 17 00:00:00 2001 From: Vishal Goel <vishal.g...@samsung.com> 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 <vishal.g...@samsung.com> Signed-off-by: Himanshu Shukla <himanshu...@samsung.com> --- 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<vishal.g...@samsung.com> 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.2N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±‘êçzX§¶›¡Ü¨}©ž²Æ zÚ&j:+v‰¨¾«‘êçzZ+€Ê+zf£¢·hšˆ§~††Ûiÿûàz¹®w¥¢¸?™¨èÚ&¢)ߢf”ù^jÇ«y§m…á@A«a¶Úÿ 0¶ìh®å’i