diff mbox

[VIVID,WILY,XENIAL] UBUNTU: SAUCE: (no-up): apparmor: fix for failed mediation of socket that is being shutdown

Message ID 1453767011-18241-1-git-send-email-tyhicks@canonical.com
State New
Headers show

Commit Message

Tyler Hicks Jan. 26, 2016, 12:10 a.m. UTC
From: John Johansen <john.johansen@canonical.com>

BugLink: http://bugs.launchpad.net/bugs/1446906

This is a horrendous HACK, that is a temporary fix until typesplitting
can land.

Store off the path reference on connection to make up for the path
being wiped out on socket shutdown.

Signed-off-by: John Johansen <john.johansen@canonical.com>
Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
---
 security/apparmor/af_unix.c     |  4 ++++
 security/apparmor/include/net.h |  2 ++
 security/apparmor/lsm.c         | 20 ++++++++++++++++++++
 3 files changed, 26 insertions(+)

Comments

Tim Gardner Jan. 26, 2016, 2:12 a.m. UTC | #1
Positive test results
Brad Figg Jan. 26, 2016, 4:34 p.m. UTC | #2
On Mon, Jan 25, 2016 at 06:10:11PM -0600, Tyler Hicks wrote:
> From: John Johansen <john.johansen@canonical.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1446906
> 
> This is a horrendous HACK, that is a temporary fix until typesplitting
> can land.
> 
> Store off the path reference on connection to make up for the path
> being wiped out on socket shutdown.
> 
> Signed-off-by: John Johansen <john.johansen@canonical.com>
> Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
> ---
>  security/apparmor/af_unix.c     |  4 ++++
>  security/apparmor/include/net.h |  2 ++
>  security/apparmor/lsm.c         | 20 ++++++++++++++++++++
>  3 files changed, 26 insertions(+)
> 
> diff --git a/security/apparmor/af_unix.c b/security/apparmor/af_unix.c
> index 62e7fd1..1da5ee6 100644
> --- a/security/apparmor/af_unix.c
> +++ b/security/apparmor/af_unix.c
> @@ -40,6 +40,10 @@ static inline int unix_fs_perm(int op, u32 mask, struct aa_label *label,
>  		/* socket path has been cleared because it is being shutdown
>  		 * can only fall back to original sun_path request
>  		 */
> +		struct aa_sk_cxt *cxt = SK_CXT(&u->sk);
> +		if (cxt->path.dentry)
> +			return aa_path_perm(op, label, &cxt->path, flags, mask,
> +					    &cond);
>  		return fn_for_each_confined(label, profile,
>  			((flags | profile->path_flags) & PATH_MEDIATE_DELETED) ?
>  				__aa_path_perm(op, profile,
> diff --git a/security/apparmor/include/net.h b/security/apparmor/include/net.h
> index 4a5fae5..1aedbf6 100644
> --- a/security/apparmor/include/net.h
> +++ b/security/apparmor/include/net.h
> @@ -16,6 +16,7 @@
>  #define __AA_NET_H
>  
>  #include <net/sock.h>
> +#include <linux/path.h>
>  
>  #include "apparmorfs.h"
>  #include "label.h"
> @@ -52,6 +53,7 @@
>  struct aa_sk_cxt {
>  	struct aa_label *label;
>  	struct aa_label *peer;
> +	struct path path;
>  };
>  
>  #define SK_CXT(X) (X)->sk_security
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 87f470c..f8a1a6d 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -754,6 +754,7 @@ static void apparmor_sk_free_security(struct sock *sk)
>  	SK_CXT(sk) = NULL;
>  	aa_put_label(cxt->label);
>  	aa_put_label(cxt->peer);
> +	path_put(&cxt->path);
>  	kfree(cxt);
>  }
>  
> @@ -768,6 +769,17 @@ static void apparmor_sk_clone_security(const struct sock *sk,
>  
>  	new->label = aa_get_label(cxt->label);
>  	new->peer = aa_get_label(cxt->peer);
> +	new->path = cxt->path;
> +	path_get(&new->path);
> +}
> +
> +static struct path *UNIX_FS_CONN_PATH(struct sock *sk, struct sock *newsk)
> +{
> +	if (sk->sk_family == PF_UNIX && UNIX_FS(sk))
> +		return &unix_sk(sk)->path;
> +	else if (newsk->sk_family == PF_UNIX && UNIX_FS(newsk))
> +		return &unix_sk(newsk)->path;
> +	return NULL;
>  }
>  
>  /**
> @@ -782,6 +794,7 @@ static int apparmor_unix_stream_connect(struct sock *sk, struct sock *peer_sk,
>  	struct aa_sk_cxt *peer_cxt = SK_CXT(peer_sk);
>  	struct aa_sk_cxt *new_cxt = SK_CXT(newsk);
>  	struct aa_label *label;
> +	struct path *path;
>  	int error;
>  
>  	label = aa_begin_current_label();
> @@ -817,6 +830,13 @@ static int apparmor_unix_stream_connect(struct sock *sk, struct sock *peer_sk,
>  	new_cxt->peer = aa_get_label(sk_cxt->label);
>  	sk_cxt->peer = aa_get_label(peer_cxt->label);
>  
> +	path = UNIX_FS_CONN_PATH(sk, peer_sk);
> +	if (path) {
> +		new_cxt->path = *path;
> +		sk_cxt->path = *path;
> +		path_get(path);
> +		path_get(path);
> +	}
>  	return 0;
>  }
>  
> -- 
> 2.5.0
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

Positive testing
Kamal Mostafa Jan. 26, 2016, 5:19 p.m. UTC | #3
This has been applied to the branches:

xenial/master-next
wily/master-next
trusty/lts-backport-vivid-next

 -Kamal
diff mbox

Patch

diff --git a/security/apparmor/af_unix.c b/security/apparmor/af_unix.c
index 62e7fd1..1da5ee6 100644
--- a/security/apparmor/af_unix.c
+++ b/security/apparmor/af_unix.c
@@ -40,6 +40,10 @@  static inline int unix_fs_perm(int op, u32 mask, struct aa_label *label,
 		/* socket path has been cleared because it is being shutdown
 		 * can only fall back to original sun_path request
 		 */
+		struct aa_sk_cxt *cxt = SK_CXT(&u->sk);
+		if (cxt->path.dentry)
+			return aa_path_perm(op, label, &cxt->path, flags, mask,
+					    &cond);
 		return fn_for_each_confined(label, profile,
 			((flags | profile->path_flags) & PATH_MEDIATE_DELETED) ?
 				__aa_path_perm(op, profile,
diff --git a/security/apparmor/include/net.h b/security/apparmor/include/net.h
index 4a5fae5..1aedbf6 100644
--- a/security/apparmor/include/net.h
+++ b/security/apparmor/include/net.h
@@ -16,6 +16,7 @@ 
 #define __AA_NET_H
 
 #include <net/sock.h>
+#include <linux/path.h>
 
 #include "apparmorfs.h"
 #include "label.h"
@@ -52,6 +53,7 @@ 
 struct aa_sk_cxt {
 	struct aa_label *label;
 	struct aa_label *peer;
+	struct path path;
 };
 
 #define SK_CXT(X) (X)->sk_security
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 87f470c..f8a1a6d 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -754,6 +754,7 @@  static void apparmor_sk_free_security(struct sock *sk)
 	SK_CXT(sk) = NULL;
 	aa_put_label(cxt->label);
 	aa_put_label(cxt->peer);
+	path_put(&cxt->path);
 	kfree(cxt);
 }
 
@@ -768,6 +769,17 @@  static void apparmor_sk_clone_security(const struct sock *sk,
 
 	new->label = aa_get_label(cxt->label);
 	new->peer = aa_get_label(cxt->peer);
+	new->path = cxt->path;
+	path_get(&new->path);
+}
+
+static struct path *UNIX_FS_CONN_PATH(struct sock *sk, struct sock *newsk)
+{
+	if (sk->sk_family == PF_UNIX && UNIX_FS(sk))
+		return &unix_sk(sk)->path;
+	else if (newsk->sk_family == PF_UNIX && UNIX_FS(newsk))
+		return &unix_sk(newsk)->path;
+	return NULL;
 }
 
 /**
@@ -782,6 +794,7 @@  static int apparmor_unix_stream_connect(struct sock *sk, struct sock *peer_sk,
 	struct aa_sk_cxt *peer_cxt = SK_CXT(peer_sk);
 	struct aa_sk_cxt *new_cxt = SK_CXT(newsk);
 	struct aa_label *label;
+	struct path *path;
 	int error;
 
 	label = aa_begin_current_label();
@@ -817,6 +830,13 @@  static int apparmor_unix_stream_connect(struct sock *sk, struct sock *peer_sk,
 	new_cxt->peer = aa_get_label(sk_cxt->label);
 	sk_cxt->peer = aa_get_label(peer_cxt->label);
 
+	path = UNIX_FS_CONN_PATH(sk, peer_sk);
+	if (path) {
+		new_cxt->path = *path;
+		sk_cxt->path = *path;
+		path_get(path);
+		path_get(path);
+	}
 	return 0;
 }