diff mbox

[3/3] Smack: Fix the issue of wrong SMACK label update in socket bind fail case

Message ID 1479877374-23604-1-git-send-email-vishal.goel@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vishal Goel Nov. 23, 2016, 5:02 a.m. UTC
Fix the issue of wrong 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 this 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 matching 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@samsung.com>
Signed-off-by: Himanshu Shukla <himanshu.sh@samsung.com>
---
 security/smack/smack.h     |  1 +
 security/smack/smack_lsm.c | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

Comments

Casey Schaufler Nov. 28, 2016, 10:33 p.m. UTC | #1
On 11/22/2016 9:02 PM, Vishal Goel wrote:
> Fix the issue of wrong 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 this 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 matching 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@samsung.com>
> Signed-off-by: Himanshu Shukla <himanshu.sh@samsung.com>

Acked-by: Casey Schaufler <casey@schaufler-ca.com>

I have queued this for 4.11 as it's too late for 4.10.

> ---
>  security/smack/smack.h     |  1 +
>  security/smack/smack_lsm.c | 20 ++++++++++++++++++++
>  2 files changed, 21 insertions(+)
>
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index 7d92c74..3ea0cc6 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -174,6 +174,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;
>  };
>  #endif /* SMACK_IPV6_PORT_LABELING */
>  
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 0ed61e9..8b8b496 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -2355,6 +2355,20 @@ static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t gfp_flags)
>   */
>  static void smack_sk_free_security(struct sock *sk)
>  {
> +#ifdef SMACK_IPV6_PORT_LABELING
> +	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();
> +	}
> +#endif
>  	kfree(sk->sk_security);
>  }
>  
> @@ -2638,10 +2652,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;
>  	}
> @@ -2658,6 +2677,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);

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/security/smack/smack.h b/security/smack/smack.h
index 7d92c74..3ea0cc6 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -174,6 +174,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;
 };
 #endif /* SMACK_IPV6_PORT_LABELING */
 
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 0ed61e9..8b8b496 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2355,6 +2355,20 @@  static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t gfp_flags)
  */
 static void smack_sk_free_security(struct sock *sk)
 {
+#ifdef SMACK_IPV6_PORT_LABELING
+	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();
+	}
+#endif
 	kfree(sk->sk_security);
 }
 
@@ -2638,10 +2652,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;
 	}
@@ -2658,6 +2677,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);