| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-id: <1070174684.669521423116103299.JavaMail.weblogic@epmlwas06a>
Date: Thu, 05 Feb 2015 06:01:43 +0000 (GMT)
From: Vishal Goel <vishal.goel@...sung.com>
To: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-security-module@...r.kernel.org"
<linux-security-module@...r.kernel.org>,
"casey@...aufler-ca.com" <casey@...aufler-ca.com>
Cc: "himanshu.sh@...sung.com" <himanshu.sh@...sung.com>
Subject: [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 <vishal.goel@...sung.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.goel@...sung.com>
Signed-off-by: Himanshu Shukla <himanshu.sh@...sung.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.goel@...sung.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.2
Powered by blists - more mailing lists