From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751646AbbBEGBq (ORCPT ); Thu, 5 Feb 2015 01:01:46 -0500 Received: from mailout3.samsung.com ([203.254.224.33]:23480 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751026AbbBEGBp (ORCPT ); Thu, 5 Feb 2015 01:01:45 -0500 X-AuditID: cbfee68f-f791c6d000004834-c6-54d30747e714 Date: Thu, 05 Feb 2015 06:01:43 +0000 (GMT) From: Vishal Goel Subject: [PATCH 2/3] smack : Fixes the undesired smack label update when 2 servers are run with different protocols(tcp,udp) To: "linux-kernel@vger.kernel.org" , "linux-security-module@vger.kernel.org" , "casey@schaufler-ca.com" Cc: "himanshu.sh@samsung.com" Reply-to: vishal.goel@samsung.com MIME-version: 1.0 X-MTR: 20150205055610612@vishal.goel Msgkey: 20150205055610612@vishal.goel X-EPLocale: en_US.windows-1252 X-Priority: 3 X-EPWebmail-Msg-Type: personal X-EPWebmail-Reply-Demand: 0 X-EPApproval-Locale: X-EPHeader: ML X-MLAttribute: X-RootMTR: 20150205055123117@vishal.goel X-ParentMTR: 20150205055123117@vishal.goel X-ArchiveUser: X-CPGSPASS: N X-ConfirmMail: N,general Content-type: text/plain; charset=windows-1252 MIME-version: 1.0 Message-id: <1070174684.669521423116103299.JavaMail.weblogic@epmlwas06a> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrKIsWRmVeSWpSXmKPExsWyRsSkSted/XKIwfKzchaXd81hc2D0+LxJ LoAxissmJTUnsyy1SN8ugSvjwa9DzAX7vCo+tnI3MB7x6GLk5BASUJNoermLFcSWEDCR2Na8 jBHCFpO4cG89WxcjF1DNUkaJaYs2ssEUrVlxngkiMYdRYtv1y+wgCRYBFYmur6eYQGw2AR2J y70tLCC2sECNxKclL9lBGkQETjJKdF79CdbALGAusXzBAXaIMxQllq98DbaaV0BQ4uTMJ0DN HEDbVCSWrJWHCKtK9LSdZ4E4Qk5iydTLTBA2r8SM9qdw8Wlf1zBD2NIS52dtgPtm8ffHUHF+ iWO3d0D1CkhMPXMQqkZTYs2pu1BxbYlVzxcxwtTvOrWcGWbX/S1zoWokJLa2PGGFeEVRYkr3 Q6i3DCSOLJrDiuoVENtD4sWpR6ygcJAQ6OWQ2HPmA8sERqVZSOpmIZk1C8ksZDULGFlWMYqm FiQXFCelFxnrFSfmFpfmpesl5+duYgSmhtP/nvXvYLx7wPoQowAHoxIPr8W+SyFCrIllxZW5 hxhNgfE0kVlKNDkfmIDySuINjc2MLExNTI2NzC3NlMR5F0r9DBYSSE8sSc1OTS1ILYovKs1J LT7EyMTBKdXAKN/gfzJoXdXpVPUqs5eLbI2TO77cSvj13MBg04tpXJOZfT5/enx4itWiXX5s ycvPLt/GXiuYtevlxEM2LhNyxTKZvs39syBw7pXV95j2yHvK+7/fnb31nerVgvLaJVf1mQIb rKV3be7zvqx2lrm2r+Jw2/TvlRKNh5Idjbxt/H+3zmT0UCjKUWIpzkg01GIuKk4EABMmaUEI AwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrGKsWRmVeSWpSXmKPExsVy+t/tXl139sshBudniVlc3jWHzYHR4/Mm uQDGqDSbjNTElNQihdS85PyUzLx0WyXv4HjneFMzA0NdQ0sLcyWFvMTcVFslF58AXbfMHKCh SgpliTmlQKGAxOJiJX07m6L80pJUhYz84hJbpWhDcyM9IwM9UyM9Q9NYK0MDAyNToJqEtIwH vw4xF+zzqvjYyt3AeMSji5GTQ0hATaLp5S5WEFtCwERizYrzTBC2mMSFe+vZuhi5gGrmMEps u36ZHSTBIqAi0fX1FFgRm4COxOXeFhYQW1igRuLTkpfsIA0iAicZJTqv/gRrYBYwl1i+4AA7 xDZFieUrXzOC2LwCghInZz4BauYA2qYisWStPERYVaKn7TwLxBFyEkumXoY6iFdiRvtTuPi0 r2uYIWxpifOzNjDCHL34+2OoOL/Esds7oHoFJKaeOQhVoymx5tRdqLi2xKrnixhh6nedWs4M s+v+lrlQNRISW1uesEK8oigxpfsh1FsGEkcWzWFF9QqI7SHx4tQj1gmMsrOQpGYhaZ+FpB1Z zQJGllWMoqkFyQXFSekVJnrFibnFpXnpesn5uZsYwWno2ZIdjA0XrA8xCnAwKvHwWuy7FCLE mlhWXJl7iFGCg1lJhNf3LVCINyWxsiq1KD++qDQntfgQoykw1iYyS4km5wNTZF5JvKGxibmp samFgaG5uZmSOO//c7khQgLpiSWp2ampBalFMH1MHJxSDYz1QdzvtWdcMpS41zFj0Rtv7jMT 3btv2L5pn70wtfDRhxkJ849tfNL4adv5GzP2PO+2np/rNkk453YD54+2GR9MMjYsEMhb+8vq 6ONvJaufr3HsOmR4VsfyHZum0cmJPKfudNfefz59SyLjwkNWS37ezHxzSPfkI+GdH+eGCeS/ sij5uKwjx6jvgRJLcUaioRZzUXEiAC60+3BZAwAA DLP-Filter: Pass X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by nfs id t1561qoA022811 >>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{.n++%ݶw{.n+{G{ayʇڙ,jfhz_(階ݢj"mG?&~iOzv^m ?I