[PATCH 0/6] LSM: Infrastructure blob allocation

public inbox for linux-security-module@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/6] LSM: Infrastructure blob allocation
       [not found] <20240708213957.20519-1-casey.ref@schaufler-ca.com>
@ 2024-07-08 21:39 ` Casey Schaufler
  2024-07-08 21:39   ` [PATCH 1/6] LSM: Infrastructure management of the sock security Casey Schaufler
                     ` (5 more replies)
  0 siblings, 6 replies; 28+ messages in thread
From: Casey Schaufler @ 2024-07-08 21:39 UTC (permalink / raw)
  To: casey, paul, linux-security-module
  Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
	stephen.smalley.work, mic

When more than one Linux Security Module (LSM) can use the security
blob for a partincular object the management of the memory associated
with that blob needs to be done by the infrastructure rather than the
individual modules.  Until now, this has been done on an as needed basis,
with the blob management remaining in the modules until such time as a
new configuration of modules requires sharing the blob.  This piecemeal
approach makes adding new modules that use blobs more difficult, as
moving the blob management to the infrastructure isn't as simple as
it might seem. This patch set moves management of the security blobs
that is done in the modules into the infrastructure. Making security
blob management more consistant improves mantainablity and makes the
possibilty of general improvement of LSM blob managment easier.

No effort has been put into pursuing the possible performance
optimizations these changes introduce. For example, sk_security blobs
might be moved to use kmem_zone_alloc(). The option of changing the
blob sizes to being compile time determined rather than calculated at
run time has been considered for future exploration.

Security blobs for the xfrm subsystem are problematic as the only
security module that implements them (SELinux) has a variable size blob
that has a published external API. Management of these blobs by the
infrastructure will require significant consideration and negotiation
with the maintainers of the existing code.  This has been deferred until
such time as another user of xfrm appears.

Casey Schaufler (6):
  LSM: Infrastructure management of the sock security
  LSM: Infrastructure management of the key security blob
  LSM: Add helper for blob allocations
  LSM: Infrastructure management of the dev_tun blob
  LSM: Infrastructure management of the infiniband blob
  LSM: Infrastructure management of the perf_event security blob

 include/linux/lsm_hook_defs.h     |   4 +-
 include/linux/lsm_hooks.h         |   5 +
 security/apparmor/include/net.h   |   3 +-
 security/apparmor/lsm.c           |  17 +--
 security/apparmor/net.c           |   2 +-
 security/security.c               | 166 +++++++++++++++++++++---------
 security/selinux/hooks.c          | 159 ++++++++++------------------
 security/selinux/include/objsec.h |  30 ++++++
 security/selinux/netlabel.c       |  23 +++--
 security/smack/smack.h            |  12 +++
 security/smack/smack_lsm.c        | 103 +++++++++---------
 security/smack/smack_netfilter.c  |   4 +-
 12 files changed, 288 insertions(+), 240 deletions(-)

-- 
2.41.0

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 1/6] LSM: Infrastructure management of the sock security
  2024-07-08 21:39 ` [PATCH 0/6] LSM: Infrastructure blob allocation Casey Schaufler
@ 2024-07-08 21:39   ` Casey Schaufler
  2024-07-09 19:15     ` Paul Moore
                       ` (2 more replies)
  2024-07-08 21:39   ` [PATCH 2/6] LSM: Infrastructure management of the key security blob Casey Schaufler
                     ` (4 subsequent siblings)
  5 siblings, 3 replies; 28+ messages in thread
From: Casey Schaufler @ 2024-07-08 21:39 UTC (permalink / raw)
  To: casey, paul, linux-security-module
  Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
	stephen.smalley.work, mic

Move management of the sock->sk_security blob out
of the individual security modules and into the security
infrastructure. Instead of allocating the blobs from within
the modules the modules tell the infrastructure how much
space is required, and the space is allocated there.

Acked-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: John Johansen <john.johansen@canonical.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 include/linux/lsm_hooks.h         |  1 +
 security/apparmor/include/net.h   |  3 +-
 security/apparmor/lsm.c           | 17 +------
 security/apparmor/net.c           |  2 +-
 security/security.c               | 36 +++++++++++++-
 security/selinux/hooks.c          | 80 ++++++++++++++-----------------
 security/selinux/include/objsec.h |  5 ++
 security/selinux/netlabel.c       | 23 ++++-----
 security/smack/smack.h            |  5 ++
 security/smack/smack_lsm.c        | 70 +++++++++++++--------------
 security/smack/smack_netfilter.c  |  4 +-
 11 files changed, 133 insertions(+), 113 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index a2ade0ffe9e7..efd4a0655159 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -73,6 +73,7 @@ struct lsm_blob_sizes {
 	int	lbs_cred;
 	int	lbs_file;
 	int	lbs_inode;
+	int	lbs_sock;
 	int	lbs_superblock;
 	int	lbs_ipc;
 	int	lbs_msg_msg;
diff --git a/security/apparmor/include/net.h b/security/apparmor/include/net.h
index 67bf888c3bd6..c42ed8a73f1c 100644
--- a/security/apparmor/include/net.h
+++ b/security/apparmor/include/net.h
@@ -51,10 +51,9 @@ struct aa_sk_ctx {
 	struct aa_label *peer;
 };
 
-#define SK_CTX(X) ((X)->sk_security)
 static inline struct aa_sk_ctx *aa_sock(const struct sock *sk)
 {
-	return sk->sk_security;
+	return sk->sk_security + apparmor_blob_sizes.lbs_sock;
 }
 
 #define DEFINE_AUDIT_NET(NAME, OP, SK, F, T, P)				  \
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 6239777090c4..b3eb0a2f999a 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1057,27 +1057,12 @@ static int apparmor_userns_create(const struct cred *cred)
 	return error;
 }
 
-static int apparmor_sk_alloc_security(struct sock *sk, int family, gfp_t flags)
-{
-	struct aa_sk_ctx *ctx;
-
-	ctx = kzalloc(sizeof(*ctx), flags);
-	if (!ctx)
-		return -ENOMEM;
-
-	sk->sk_security = ctx;
-
-	return 0;
-}
-
 static void apparmor_sk_free_security(struct sock *sk)
 {
 	struct aa_sk_ctx *ctx = aa_sock(sk);
 
-	sk->sk_security = NULL;
 	aa_put_label(ctx->label);
 	aa_put_label(ctx->peer);
-	kfree(ctx);
 }
 
 /**
@@ -1425,6 +1410,7 @@ struct lsm_blob_sizes apparmor_blob_sizes __ro_after_init = {
 	.lbs_cred = sizeof(struct aa_label *),
 	.lbs_file = sizeof(struct aa_file_ctx),
 	.lbs_task = sizeof(struct aa_task_ctx),
+	.lbs_sock = sizeof(struct aa_sk_ctx),
 };
 
 static const struct lsm_id apparmor_lsmid = {
@@ -1470,7 +1456,6 @@ static struct security_hook_list apparmor_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(getprocattr, apparmor_getprocattr),
 	LSM_HOOK_INIT(setprocattr, apparmor_setprocattr),
 
-	LSM_HOOK_INIT(sk_alloc_security, apparmor_sk_alloc_security),
 	LSM_HOOK_INIT(sk_free_security, apparmor_sk_free_security),
 	LSM_HOOK_INIT(sk_clone_security, apparmor_sk_clone_security),
 
diff --git a/security/apparmor/net.c b/security/apparmor/net.c
index 87e934b2b548..77413a519117 100644
--- a/security/apparmor/net.c
+++ b/security/apparmor/net.c
@@ -151,7 +151,7 @@ static int aa_label_sk_perm(const struct cred *subj_cred,
 			    const char *op, u32 request,
 			    struct sock *sk)
 {
-	struct aa_sk_ctx *ctx = SK_CTX(sk);
+	struct aa_sk_ctx *ctx = aa_sock(sk);
 	int error = 0;
 
 	AA_BUG(!label);
diff --git a/security/security.c b/security/security.c
index e5ca08789f74..5e93a72bdca6 100644
--- a/security/security.c
+++ b/security/security.c
@@ -29,6 +29,7 @@
 #include <linux/msg.h>
 #include <linux/overflow.h>
 #include <net/flow.h>
+#include <net/sock.h>
 
 /* How many LSMs were built into the kernel? */
 #define LSM_COUNT (__end_lsm_info - __start_lsm_info)
@@ -227,6 +228,7 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
 	lsm_set_blob_size(&needed->lbs_inode, &blob_sizes.lbs_inode);
 	lsm_set_blob_size(&needed->lbs_ipc, &blob_sizes.lbs_ipc);
 	lsm_set_blob_size(&needed->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
+	lsm_set_blob_size(&needed->lbs_sock, &blob_sizes.lbs_sock);
 	lsm_set_blob_size(&needed->lbs_superblock, &blob_sizes.lbs_superblock);
 	lsm_set_blob_size(&needed->lbs_task, &blob_sizes.lbs_task);
 	lsm_set_blob_size(&needed->lbs_xattr_count,
@@ -401,6 +403,7 @@ static void __init ordered_lsm_init(void)
 	init_debug("inode blob size      = %d\n", blob_sizes.lbs_inode);
 	init_debug("ipc blob size        = %d\n", blob_sizes.lbs_ipc);
 	init_debug("msg_msg blob size    = %d\n", blob_sizes.lbs_msg_msg);
+	init_debug("sock blob size       = %d\n", blob_sizes.lbs_sock);
 	init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
 	init_debug("task blob size       = %d\n", blob_sizes.lbs_task);
 	init_debug("xattr slots          = %d\n", blob_sizes.lbs_xattr_count);
@@ -4647,6 +4650,28 @@ int security_socket_getpeersec_dgram(struct socket *sock,
 }
 EXPORT_SYMBOL(security_socket_getpeersec_dgram);
 
+/**
+ * lsm_sock_alloc - allocate a composite sock blob
+ * @sock: the sock that needs a blob
+ * @priority: allocation mode
+ *
+ * Allocate the sock blob for all the modules
+ *
+ * Returns 0, or -ENOMEM if memory can't be allocated.
+ */
+static int lsm_sock_alloc(struct sock *sock, gfp_t priority)
+{
+	if (blob_sizes.lbs_sock == 0) {
+		sock->sk_security = NULL;
+		return 0;
+	}
+
+	sock->sk_security = kzalloc(blob_sizes.lbs_sock, priority);
+	if (sock->sk_security == NULL)
+		return -ENOMEM;
+	return 0;
+}
+
 /**
  * security_sk_alloc() - Allocate and initialize a sock's LSM blob
  * @sk: sock
@@ -4660,7 +4685,14 @@ EXPORT_SYMBOL(security_socket_getpeersec_dgram);
  */
 int security_sk_alloc(struct sock *sk, int family, gfp_t priority)
 {
-	return call_int_hook(sk_alloc_security, sk, family, priority);
+	int rc = lsm_sock_alloc(sk, priority);
+
+	if (unlikely(rc))
+		return rc;
+	rc = call_int_hook(sk_alloc_security, sk, family, priority);
+	if (unlikely(rc))
+		security_sk_free(sk);
+	return rc;
 }
 
 /**
@@ -4672,6 +4704,8 @@ int security_sk_alloc(struct sock *sk, int family, gfp_t priority)
 void security_sk_free(struct sock *sk)
 {
 	call_void_hook(sk_free_security, sk);
+	kfree(sk->sk_security);
+	sk->sk_security = NULL;
 }
 
 /**
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 7eed331e90f0..19346e1817ff 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4579,7 +4579,7 @@ static int socket_sockcreate_sid(const struct task_security_struct *tsec,
 
 static int sock_has_perm(struct sock *sk, u32 perms)
 {
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
 	struct common_audit_data ad;
 	struct lsm_network_audit net;
 
@@ -4647,7 +4647,7 @@ static int selinux_socket_post_create(struct socket *sock, int family,
 	isec->initialized = LABEL_INITIALIZED;
 
 	if (sock->sk) {
-		sksec = sock->sk->sk_security;
+		sksec = selinux_sock(sock->sk);
 		sksec->sclass = sclass;
 		sksec->sid = sid;
 		/* Allows detection of the first association on this socket */
@@ -4663,8 +4663,8 @@ static int selinux_socket_post_create(struct socket *sock, int family,
 static int selinux_socket_socketpair(struct socket *socka,
 				     struct socket *sockb)
 {
-	struct sk_security_struct *sksec_a = socka->sk->sk_security;
-	struct sk_security_struct *sksec_b = sockb->sk->sk_security;
+	struct sk_security_struct *sksec_a = selinux_sock(socka->sk);
+	struct sk_security_struct *sksec_b = selinux_sock(sockb->sk);
 
 	sksec_a->peer_sid = sksec_b->sid;
 	sksec_b->peer_sid = sksec_a->sid;
@@ -4679,7 +4679,7 @@ static int selinux_socket_socketpair(struct socket *socka,
 static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, int addrlen)
 {
 	struct sock *sk = sock->sk;
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
 	u16 family;
 	int err;
 
@@ -4819,7 +4819,7 @@ static int selinux_socket_connect_helper(struct socket *sock,
 					 struct sockaddr *address, int addrlen)
 {
 	struct sock *sk = sock->sk;
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
 	int err;
 
 	err = sock_has_perm(sk, SOCKET__CONNECT);
@@ -4997,9 +4997,9 @@ static int selinux_socket_unix_stream_connect(struct sock *sock,
 					      struct sock *other,
 					      struct sock *newsk)
 {
-	struct sk_security_struct *sksec_sock = sock->sk_security;
-	struct sk_security_struct *sksec_other = other->sk_security;
-	struct sk_security_struct *sksec_new = newsk->sk_security;
+	struct sk_security_struct *sksec_sock = selinux_sock(sock);
+	struct sk_security_struct *sksec_other = selinux_sock(other);
+	struct sk_security_struct *sksec_new = selinux_sock(newsk);
 	struct common_audit_data ad;
 	struct lsm_network_audit net;
 	int err;
@@ -5028,8 +5028,8 @@ static int selinux_socket_unix_stream_connect(struct sock *sock,
 static int selinux_socket_unix_may_send(struct socket *sock,
 					struct socket *other)
 {
-	struct sk_security_struct *ssec = sock->sk->sk_security;
-	struct sk_security_struct *osec = other->sk->sk_security;
+	struct sk_security_struct *ssec = selinux_sock(sock->sk);
+	struct sk_security_struct *osec = selinux_sock(other->sk);
 	struct common_audit_data ad;
 	struct lsm_network_audit net;
 
@@ -5066,7 +5066,7 @@ static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb,
 				       u16 family)
 {
 	int err = 0;
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
 	u32 sk_sid = sksec->sid;
 	struct common_audit_data ad;
 	struct lsm_network_audit net;
@@ -5095,7 +5095,7 @@ static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb,
 static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 {
 	int err, peerlbl_active, secmark_active;
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
 	u16 family = sk->sk_family;
 	u32 sk_sid = sksec->sid;
 	struct common_audit_data ad;
@@ -5163,7 +5163,7 @@ static int selinux_socket_getpeersec_stream(struct socket *sock,
 	int err = 0;
 	char *scontext = NULL;
 	u32 scontext_len;
-	struct sk_security_struct *sksec = sock->sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sock->sk);
 	u32 peer_sid = SECSID_NULL;
 
 	if (sksec->sclass == SECCLASS_UNIX_STREAM_SOCKET ||
@@ -5223,34 +5223,27 @@ static int selinux_socket_getpeersec_dgram(struct socket *sock,
 
 static int selinux_sk_alloc_security(struct sock *sk, int family, gfp_t priority)
 {
-	struct sk_security_struct *sksec;
-
-	sksec = kzalloc(sizeof(*sksec), priority);
-	if (!sksec)
-		return -ENOMEM;
+	struct sk_security_struct *sksec = selinux_sock(sk);
 
 	sksec->peer_sid = SECINITSID_UNLABELED;
 	sksec->sid = SECINITSID_UNLABELED;
 	sksec->sclass = SECCLASS_SOCKET;
 	selinux_netlbl_sk_security_reset(sksec);
-	sk->sk_security = sksec;
 
 	return 0;
 }
 
 static void selinux_sk_free_security(struct sock *sk)
 {
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
 
-	sk->sk_security = NULL;
 	selinux_netlbl_sk_security_free(sksec);
-	kfree(sksec);
 }
 
 static void selinux_sk_clone_security(const struct sock *sk, struct sock *newsk)
 {
-	struct sk_security_struct *sksec = sk->sk_security;
-	struct sk_security_struct *newsksec = newsk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
+	struct sk_security_struct *newsksec = selinux_sock(newsk);
 
 	newsksec->sid = sksec->sid;
 	newsksec->peer_sid = sksec->peer_sid;
@@ -5264,7 +5257,7 @@ static void selinux_sk_getsecid(const struct sock *sk, u32 *secid)
 	if (!sk)
 		*secid = SECINITSID_ANY_SOCKET;
 	else {
-		const struct sk_security_struct *sksec = sk->sk_security;
+		const struct sk_security_struct *sksec = selinux_sock(sk);
 
 		*secid = sksec->sid;
 	}
@@ -5274,7 +5267,7 @@ static void selinux_sock_graft(struct sock *sk, struct socket *parent)
 {
 	struct inode_security_struct *isec =
 		inode_security_novalidate(SOCK_INODE(parent));
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
 
 	if (sk->sk_family == PF_INET || sk->sk_family == PF_INET6 ||
 	    sk->sk_family == PF_UNIX)
@@ -5291,7 +5284,7 @@ static int selinux_sctp_process_new_assoc(struct sctp_association *asoc,
 {
 	struct sock *sk = asoc->base.sk;
 	u16 family = sk->sk_family;
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
 	struct common_audit_data ad;
 	struct lsm_network_audit net;
 	int err;
@@ -5346,7 +5339,7 @@ static int selinux_sctp_process_new_assoc(struct sctp_association *asoc,
 static int selinux_sctp_assoc_request(struct sctp_association *asoc,
 				      struct sk_buff *skb)
 {
-	struct sk_security_struct *sksec = asoc->base.sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(asoc->base.sk);
 	u32 conn_sid;
 	int err;
 
@@ -5379,7 +5372,7 @@ static int selinux_sctp_assoc_request(struct sctp_association *asoc,
 static int selinux_sctp_assoc_established(struct sctp_association *asoc,
 					  struct sk_buff *skb)
 {
-	struct sk_security_struct *sksec = asoc->base.sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(asoc->base.sk);
 
 	if (!selinux_policycap_extsockclass())
 		return 0;
@@ -5478,8 +5471,8 @@ static int selinux_sctp_bind_connect(struct sock *sk, int optname,
 static void selinux_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk,
 				  struct sock *newsk)
 {
-	struct sk_security_struct *sksec = sk->sk_security;
-	struct sk_security_struct *newsksec = newsk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
+	struct sk_security_struct *newsksec = selinux_sock(newsk);
 
 	/* If policy does not support SECCLASS_SCTP_SOCKET then call
 	 * the non-sctp clone version.
@@ -5495,8 +5488,8 @@ static void selinux_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk
 
 static int selinux_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
 {
-	struct sk_security_struct *ssksec = ssk->sk_security;
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct sk_security_struct *ssksec = selinux_sock(ssk);
+	struct sk_security_struct *sksec = selinux_sock(sk);
 
 	ssksec->sclass = sksec->sclass;
 	ssksec->sid = sksec->sid;
@@ -5511,7 +5504,7 @@ static int selinux_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
 static int selinux_inet_conn_request(const struct sock *sk, struct sk_buff *skb,
 				     struct request_sock *req)
 {
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
 	int err;
 	u16 family = req->rsk_ops->family;
 	u32 connsid;
@@ -5532,7 +5525,7 @@ static int selinux_inet_conn_request(const struct sock *sk, struct sk_buff *skb,
 static void selinux_inet_csk_clone(struct sock *newsk,
 				   const struct request_sock *req)
 {
-	struct sk_security_struct *newsksec = newsk->sk_security;
+	struct sk_security_struct *newsksec = selinux_sock(newsk);
 
 	newsksec->sid = req->secid;
 	newsksec->peer_sid = req->peer_secid;
@@ -5549,7 +5542,7 @@ static void selinux_inet_csk_clone(struct sock *newsk,
 static void selinux_inet_conn_established(struct sock *sk, struct sk_buff *skb)
 {
 	u16 family = sk->sk_family;
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
 
 	/* handle mapped IPv4 packets arriving via IPv6 sockets */
 	if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP))
@@ -5624,7 +5617,7 @@ static int selinux_tun_dev_attach_queue(void *security)
 static int selinux_tun_dev_attach(struct sock *sk, void *security)
 {
 	struct tun_security_struct *tunsec = security;
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
 
 	/* we don't currently perform any NetLabel based labeling here and it
 	 * isn't clear that we would want to do so anyway; while we could apply
@@ -5747,7 +5740,7 @@ static unsigned int selinux_ip_output(void *priv, struct sk_buff *skb,
 			return NF_ACCEPT;
 
 		/* standard practice, label using the parent socket */
-		sksec = sk->sk_security;
+		sksec = selinux_sock(sk);
 		sid = sksec->sid;
 	} else
 		sid = SECINITSID_KERNEL;
@@ -5770,7 +5763,7 @@ static unsigned int selinux_ip_postroute_compat(struct sk_buff *skb,
 	sk = skb_to_full_sk(skb);
 	if (sk == NULL)
 		return NF_ACCEPT;
-	sksec = sk->sk_security;
+	sksec = selinux_sock(sk);
 
 	ad_net_init_from_iif(&ad, &net, state->out->ifindex, state->pf);
 	if (selinux_parse_skb(skb, &ad, NULL, 0, &proto))
@@ -5859,7 +5852,7 @@ static unsigned int selinux_ip_postroute(void *priv,
 		u32 skb_sid;
 		struct sk_security_struct *sksec;
 
-		sksec = sk->sk_security;
+		sksec = selinux_sock(sk);
 		if (selinux_skb_peerlbl_sid(skb, family, &skb_sid))
 			return NF_DROP;
 		/* At this point, if the returned skb peerlbl is SECSID_NULL
@@ -5888,7 +5881,7 @@ static unsigned int selinux_ip_postroute(void *priv,
 	} else {
 		/* Locally generated packet, fetch the security label from the
 		 * associated socket. */
-		struct sk_security_struct *sksec = sk->sk_security;
+		struct sk_security_struct *sksec = selinux_sock(sk);
 		peer_sid = sksec->sid;
 		secmark_perm = PACKET__SEND;
 	}
@@ -5931,7 +5924,7 @@ static int selinux_netlink_send(struct sock *sk, struct sk_buff *skb)
 	unsigned int data_len = skb->len;
 	unsigned char *data = skb->data;
 	struct nlmsghdr *nlh;
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
 	u16 sclass = sksec->sclass;
 	u32 perm;
 
@@ -6989,6 +6982,7 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = {
 	.lbs_inode = sizeof(struct inode_security_struct),
 	.lbs_ipc = sizeof(struct ipc_security_struct),
 	.lbs_msg_msg = sizeof(struct msg_security_struct),
+	.lbs_sock = sizeof(struct sk_security_struct),
 	.lbs_superblock = sizeof(struct superblock_security_struct),
 	.lbs_xattr_count = SELINUX_INODE_INIT_XATTRS,
 };
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index dea1d6f3ed2d..b074099acbaf 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -195,4 +195,9 @@ selinux_superblock(const struct super_block *superblock)
 	return superblock->s_security + selinux_blob_sizes.lbs_superblock;
 }
 
+static inline struct sk_security_struct *selinux_sock(const struct sock *sock)
+{
+	return sock->sk_security + selinux_blob_sizes.lbs_sock;
+}
+
 #endif /* _SELINUX_OBJSEC_H_ */
diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c
index 55885634e880..fbe5f8c29f81 100644
--- a/security/selinux/netlabel.c
+++ b/security/selinux/netlabel.c
@@ -17,6 +17,7 @@
 #include <linux/gfp.h>
 #include <linux/ip.h>
 #include <linux/ipv6.h>
+#include <linux/lsm_hooks.h>
 #include <net/sock.h>
 #include <net/netlabel.h>
 #include <net/ip.h>
@@ -68,7 +69,7 @@ static int selinux_netlbl_sidlookup_cached(struct sk_buff *skb,
 static struct netlbl_lsm_secattr *selinux_netlbl_sock_genattr(struct sock *sk)
 {
 	int rc;
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
 	struct netlbl_lsm_secattr *secattr;
 
 	if (sksec->nlbl_secattr != NULL)
@@ -100,7 +101,7 @@ static struct netlbl_lsm_secattr *selinux_netlbl_sock_getattr(
 							const struct sock *sk,
 							u32 sid)
 {
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
 	struct netlbl_lsm_secattr *secattr = sksec->nlbl_secattr;
 
 	if (secattr == NULL)
@@ -240,7 +241,7 @@ int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
 	 * being labeled by it's parent socket, if it is just exit */
 	sk = skb_to_full_sk(skb);
 	if (sk != NULL) {
-		struct sk_security_struct *sksec = sk->sk_security;
+		struct sk_security_struct *sksec = selinux_sock(sk);
 
 		if (sksec->nlbl_state != NLBL_REQSKB)
 			return 0;
@@ -277,7 +278,7 @@ int selinux_netlbl_sctp_assoc_request(struct sctp_association *asoc,
 {
 	int rc;
 	struct netlbl_lsm_secattr secattr;
-	struct sk_security_struct *sksec = asoc->base.sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(asoc->base.sk);
 	struct sockaddr_in addr4;
 	struct sockaddr_in6 addr6;
 
@@ -356,7 +357,7 @@ int selinux_netlbl_inet_conn_request(struct request_sock *req, u16 family)
  */
 void selinux_netlbl_inet_csk_clone(struct sock *sk, u16 family)
 {
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
 
 	if (family == PF_INET)
 		sksec->nlbl_state = NLBL_LABELED;
@@ -374,8 +375,8 @@ void selinux_netlbl_inet_csk_clone(struct sock *sk, u16 family)
  */
 void selinux_netlbl_sctp_sk_clone(struct sock *sk, struct sock *newsk)
 {
-	struct sk_security_struct *sksec = sk->sk_security;
-	struct sk_security_struct *newsksec = newsk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
+	struct sk_security_struct *newsksec = selinux_sock(newsk);
 
 	newsksec->nlbl_state = sksec->nlbl_state;
 }
@@ -393,7 +394,7 @@ void selinux_netlbl_sctp_sk_clone(struct sock *sk, struct sock *newsk)
 int selinux_netlbl_socket_post_create(struct sock *sk, u16 family)
 {
 	int rc;
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
 	struct netlbl_lsm_secattr *secattr;
 
 	if (family != PF_INET && family != PF_INET6)
@@ -510,7 +511,7 @@ int selinux_netlbl_socket_setsockopt(struct socket *sock,
 {
 	int rc = 0;
 	struct sock *sk = sock->sk;
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
 	struct netlbl_lsm_secattr secattr;
 
 	if (selinux_netlbl_option(level, optname) &&
@@ -548,7 +549,7 @@ static int selinux_netlbl_socket_connect_helper(struct sock *sk,
 						struct sockaddr *addr)
 {
 	int rc;
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
 	struct netlbl_lsm_secattr *secattr;
 
 	/* connected sockets are allowed to disconnect when the address family
@@ -587,7 +588,7 @@ static int selinux_netlbl_socket_connect_helper(struct sock *sk,
 int selinux_netlbl_socket_connect_locked(struct sock *sk,
 					 struct sockaddr *addr)
 {
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct sk_security_struct *sksec = selinux_sock(sk);
 
 	if (sksec->nlbl_state != NLBL_REQSKB &&
 	    sksec->nlbl_state != NLBL_CONNLABELED)
diff --git a/security/smack/smack.h b/security/smack/smack.h
index 041688e5a77a..297f21446f45 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -355,6 +355,11 @@ static inline struct superblock_smack *smack_superblock(
 	return superblock->s_security + smack_blob_sizes.lbs_superblock;
 }
 
+static inline struct socket_smack *smack_sock(const struct sock *sock)
+{
+	return sock->sk_security + smack_blob_sizes.lbs_sock;
+}
+
 /*
  * Is the directory transmuting?
  */
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index f5cbec1e6a92..a931b44bc959 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1581,7 +1581,7 @@ static int smack_inode_getsecurity(struct mnt_idmap *idmap,
 		if (sock == NULL || sock->sk == NULL)
 			return -EOPNOTSUPP;
 
-		ssp = sock->sk->sk_security;
+		ssp = smack_sock(sock->sk);
 
 		if (strcmp(name, XATTR_SMACK_IPIN) == 0)
 			isp = ssp->smk_in;
@@ -1969,7 +1969,7 @@ static int smack_file_receive(struct file *file)
 
 	if (inode->i_sb->s_magic == SOCKFS_MAGIC) {
 		sock = SOCKET_I(inode);
-		ssp = sock->sk->sk_security;
+		ssp = smack_sock(sock->sk);
 		tsp = smack_cred(current_cred());
 		/*
 		 * If the receiving process can't write to the
@@ -2384,11 +2384,7 @@ static void smack_task_to_inode(struct task_struct *p, struct inode *inode)
 static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t gfp_flags)
 {
 	struct smack_known *skp = smk_of_current();
-	struct socket_smack *ssp;
-
-	ssp = kzalloc(sizeof(struct socket_smack), gfp_flags);
-	if (ssp == NULL)
-		return -ENOMEM;
+	struct socket_smack *ssp = smack_sock(sk);
 
 	/*
 	 * Sockets created by kernel threads receive web label.
@@ -2402,11 +2398,10 @@ static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t gfp_flags)
 	}
 	ssp->smk_packet = NULL;
 
-	sk->sk_security = ssp;
-
 	return 0;
 }
 
+#ifdef SMACK_IPV6_PORT_LABELING
 /**
  * smack_sk_free_security - Free a socket blob
  * @sk: the socket
@@ -2415,7 +2410,6 @@ 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) {
@@ -2428,9 +2422,8 @@ static void smack_sk_free_security(struct sock *sk)
 		}
 		rcu_read_unlock();
 	}
-#endif
-	kfree(sk->sk_security);
 }
+#endif
 
 /**
  * smack_sk_clone_security - Copy security context
@@ -2441,8 +2434,8 @@ static void smack_sk_free_security(struct sock *sk)
  */
 static void smack_sk_clone_security(const struct sock *sk, struct sock *newsk)
 {
-	struct socket_smack *ssp_old = sk->sk_security;
-	struct socket_smack *ssp_new = newsk->sk_security;
+	struct socket_smack *ssp_old = smack_sock(sk);
+	struct socket_smack *ssp_new = smack_sock(newsk);
 
 	*ssp_new = *ssp_old;
 }
@@ -2558,7 +2551,7 @@ static struct smack_known *smack_ipv6host_label(struct sockaddr_in6 *sip)
  */
 static int smack_netlbl_add(struct sock *sk)
 {
-	struct socket_smack *ssp = sk->sk_security;
+	struct socket_smack *ssp = smack_sock(sk);
 	struct smack_known *skp = ssp->smk_out;
 	int rc;
 
@@ -2591,7 +2584,7 @@ static int smack_netlbl_add(struct sock *sk)
  */
 static void smack_netlbl_delete(struct sock *sk)
 {
-	struct socket_smack *ssp = sk->sk_security;
+	struct socket_smack *ssp = smack_sock(sk);
 
 	/*
 	 * Take the label off the socket if one is set.
@@ -2623,7 +2616,7 @@ static int smk_ipv4_check(struct sock *sk, struct sockaddr_in *sap)
 	struct smack_known *skp;
 	int rc = 0;
 	struct smack_known *hkp;
-	struct socket_smack *ssp = sk->sk_security;
+	struct socket_smack *ssp = smack_sock(sk);
 	struct smk_audit_info ad;
 
 	rcu_read_lock();
@@ -2696,7 +2689,7 @@ static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address)
 {
 	struct sock *sk = sock->sk;
 	struct sockaddr_in6 *addr6;
-	struct socket_smack *ssp = sock->sk->sk_security;
+	struct socket_smack *ssp = smack_sock(sock->sk);
 	struct smk_port_label *spp;
 	unsigned short port = 0;
 
@@ -2784,7 +2777,7 @@ static int smk_ipv6_port_check(struct sock *sk, struct sockaddr_in6 *address,
 				int act)
 {
 	struct smk_port_label *spp;
-	struct socket_smack *ssp = sk->sk_security;
+	struct socket_smack *ssp = smack_sock(sk);
 	struct smack_known *skp = NULL;
 	unsigned short port;
 	struct smack_known *object;
@@ -2887,7 +2880,7 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
 	if (sock == NULL || sock->sk == NULL)
 		return -EOPNOTSUPP;
 
-	ssp = sock->sk->sk_security;
+	ssp = smack_sock(sock->sk);
 
 	if (strcmp(name, XATTR_SMACK_IPIN) == 0)
 		ssp->smk_in = skp;
@@ -2935,7 +2928,7 @@ static int smack_socket_post_create(struct socket *sock, int family,
 	 * Sockets created by kernel threads receive web label.
 	 */
 	if (unlikely(current->flags & PF_KTHREAD)) {
-		ssp = sock->sk->sk_security;
+		ssp = smack_sock(sock->sk);
 		ssp->smk_in = &smack_known_web;
 		ssp->smk_out = &smack_known_web;
 	}
@@ -2960,8 +2953,8 @@ static int smack_socket_post_create(struct socket *sock, int family,
 static int smack_socket_socketpair(struct socket *socka,
 		                   struct socket *sockb)
 {
-	struct socket_smack *asp = socka->sk->sk_security;
-	struct socket_smack *bsp = sockb->sk->sk_security;
+	struct socket_smack *asp = smack_sock(socka->sk);
+	struct socket_smack *bsp = smack_sock(sockb->sk);
 
 	asp->smk_packet = bsp->smk_out;
 	bsp->smk_packet = asp->smk_out;
@@ -3024,7 +3017,7 @@ static int smack_socket_connect(struct socket *sock, struct sockaddr *sap,
 		if (__is_defined(SMACK_IPV6_SECMARK_LABELING))
 			rsp = smack_ipv6host_label(sip);
 		if (rsp != NULL) {
-			struct socket_smack *ssp = sock->sk->sk_security;
+			struct socket_smack *ssp = smack_sock(sock->sk);
 
 			rc = smk_ipv6_check(ssp->smk_out, rsp, sip,
 					    SMK_CONNECTING);
@@ -3819,9 +3812,9 @@ static int smack_unix_stream_connect(struct sock *sock,
 {
 	struct smack_known *skp;
 	struct smack_known *okp;
-	struct socket_smack *ssp = sock->sk_security;
-	struct socket_smack *osp = other->sk_security;
-	struct socket_smack *nsp = newsk->sk_security;
+	struct socket_smack *ssp = smack_sock(sock);
+	struct socket_smack *osp = smack_sock(other);
+	struct socket_smack *nsp = smack_sock(newsk);
 	struct smk_audit_info ad;
 	int rc = 0;
 #ifdef CONFIG_AUDIT
@@ -3867,8 +3860,8 @@ static int smack_unix_stream_connect(struct sock *sock,
  */
 static int smack_unix_may_send(struct socket *sock, struct socket *other)
 {
-	struct socket_smack *ssp = sock->sk->sk_security;
-	struct socket_smack *osp = other->sk->sk_security;
+	struct socket_smack *ssp = smack_sock(sock->sk);
+	struct socket_smack *osp = smack_sock(other->sk);
 	struct smk_audit_info ad;
 	int rc;
 
@@ -3905,7 +3898,7 @@ static int smack_socket_sendmsg(struct socket *sock, struct msghdr *msg,
 	struct sockaddr_in6 *sap = (struct sockaddr_in6 *) msg->msg_name;
 #endif
 #ifdef SMACK_IPV6_SECMARK_LABELING
-	struct socket_smack *ssp = sock->sk->sk_security;
+	struct socket_smack *ssp = smack_sock(sock->sk);
 	struct smack_known *rsp;
 #endif
 	int rc = 0;
@@ -4117,7 +4110,7 @@ static struct smack_known *smack_from_netlbl(const struct sock *sk, u16 family,
 	netlbl_secattr_init(&secattr);
 
 	if (sk)
-		ssp = sk->sk_security;
+		ssp = smack_sock(sk);
 
 	if (netlbl_skbuff_getattr(skb, family, &secattr) == 0) {
 		skp = smack_from_secattr(&secattr, ssp);
@@ -4139,7 +4132,7 @@ static struct smack_known *smack_from_netlbl(const struct sock *sk, u16 family,
  */
 static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 {
-	struct socket_smack *ssp = sk->sk_security;
+	struct socket_smack *ssp = smack_sock(sk);
 	struct smack_known *skp = NULL;
 	int rc = 0;
 	struct smk_audit_info ad;
@@ -4243,7 +4236,7 @@ static int smack_socket_getpeersec_stream(struct socket *sock,
 	u32 slen = 1;
 	int rc = 0;
 
-	ssp = sock->sk->sk_security;
+	ssp = smack_sock(sock->sk);
 	if (ssp->smk_packet != NULL) {
 		rcp = ssp->smk_packet->smk_known;
 		slen = strlen(rcp) + 1;
@@ -4293,7 +4286,7 @@ static int smack_socket_getpeersec_dgram(struct socket *sock,
 
 	switch (family) {
 	case PF_UNIX:
-		ssp = sock->sk->sk_security;
+		ssp = smack_sock(sock->sk);
 		s = ssp->smk_out->smk_secid;
 		break;
 	case PF_INET:
@@ -4342,7 +4335,7 @@ static void smack_sock_graft(struct sock *sk, struct socket *parent)
 	    (sk->sk_family != PF_INET && sk->sk_family != PF_INET6))
 		return;
 
-	ssp = sk->sk_security;
+	ssp = smack_sock(sk);
 	ssp->smk_in = skp;
 	ssp->smk_out = skp;
 	/* cssp->smk_packet is already set in smack_inet_csk_clone() */
@@ -4362,7 +4355,7 @@ static int smack_inet_conn_request(const struct sock *sk, struct sk_buff *skb,
 {
 	u16 family = sk->sk_family;
 	struct smack_known *skp;
-	struct socket_smack *ssp = sk->sk_security;
+	struct socket_smack *ssp = smack_sock(sk);
 	struct sockaddr_in addr;
 	struct iphdr *hdr;
 	struct smack_known *hskp;
@@ -4448,7 +4441,7 @@ static int smack_inet_conn_request(const struct sock *sk, struct sk_buff *skb,
 static void smack_inet_csk_clone(struct sock *sk,
 				 const struct request_sock *req)
 {
-	struct socket_smack *ssp = sk->sk_security;
+	struct socket_smack *ssp = smack_sock(sk);
 	struct smack_known *skp;
 
 	if (req->peer_secid != 0) {
@@ -5018,6 +5011,7 @@ struct lsm_blob_sizes smack_blob_sizes __ro_after_init = {
 	.lbs_inode = sizeof(struct inode_smack),
 	.lbs_ipc = sizeof(struct smack_known *),
 	.lbs_msg_msg = sizeof(struct smack_known *),
+	.lbs_sock = sizeof(struct socket_smack),
 	.lbs_superblock = sizeof(struct superblock_smack),
 	.lbs_xattr_count = SMACK_INODE_INIT_XATTRS,
 };
@@ -5141,7 +5135,9 @@ static struct security_hook_list smack_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(socket_getpeersec_stream, smack_socket_getpeersec_stream),
 	LSM_HOOK_INIT(socket_getpeersec_dgram, smack_socket_getpeersec_dgram),
 	LSM_HOOK_INIT(sk_alloc_security, smack_sk_alloc_security),
+#ifdef SMACK_IPV6_PORT_LABELING
 	LSM_HOOK_INIT(sk_free_security, smack_sk_free_security),
+#endif
 	LSM_HOOK_INIT(sk_clone_security, smack_sk_clone_security),
 	LSM_HOOK_INIT(sock_graft, smack_sock_graft),
 	LSM_HOOK_INIT(inet_conn_request, smack_inet_conn_request),
diff --git a/security/smack/smack_netfilter.c b/security/smack/smack_netfilter.c
index b945c1d3a743..bad71b7e648d 100644
--- a/security/smack/smack_netfilter.c
+++ b/security/smack/smack_netfilter.c
@@ -26,8 +26,8 @@ static unsigned int smack_ip_output(void *priv,
 	struct socket_smack *ssp;
 	struct smack_known *skp;
 
-	if (sk && sk->sk_security) {
-		ssp = sk->sk_security;
+	if (sk) {
+		ssp = smack_sock(sk);
 		skp = ssp->smk_out;
 		skb->secmark = skp->smk_secid;
 	}
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/6] LSM: Infrastructure management of the sock security
  2024-07-08 21:39   ` [PATCH 1/6] LSM: Infrastructure management of the sock security Casey Schaufler
@ 2024-07-09 19:15     ` Paul Moore
  2024-07-09 23:00       ` Casey Schaufler
  2024-07-09 22:08     ` Paul Moore
  2024-07-09 22:32     ` John Johansen
  2 siblings, 1 reply; 28+ messages in thread
From: Paul Moore @ 2024-07-09 19:15 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: linux-security-module, jmorris, serge, keescook, john.johansen,
	penguin-kernel, stephen.smalley.work, mic

On Mon, Jul 8, 2024 at 5:40 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Move management of the sock->sk_security blob out
> of the individual security modules and into the security
> infrastructure. Instead of allocating the blobs from within
> the modules the modules tell the infrastructure how much
> space is required, and the space is allocated there.
>
> Acked-by: Paul Moore <paul@paul-moore.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Reviewed-by: John Johansen <john.johansen@canonical.com>
> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>  include/linux/lsm_hooks.h         |  1 +
>  security/apparmor/include/net.h   |  3 +-
>  security/apparmor/lsm.c           | 17 +------
>  security/apparmor/net.c           |  2 +-
>  security/security.c               | 36 +++++++++++++-
>  security/selinux/hooks.c          | 80 ++++++++++++++-----------------
>  security/selinux/include/objsec.h |  5 ++
>  security/selinux/netlabel.c       | 23 ++++-----
>  security/smack/smack.h            |  5 ++
>  security/smack/smack_lsm.c        | 70 +++++++++++++--------------
>  security/smack/smack_netfilter.c  |  4 +-
>  11 files changed, 133 insertions(+), 113 deletions(-)

...

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 7eed331e90f0..19346e1817ff 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -5495,8 +5488,8 @@ static void selinux_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk
>
>  static int selinux_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
>  {
> -       struct sk_security_struct *ssksec = ssk->sk_security;
> -       struct sk_security_struct *sksec = sk->sk_security;
> +       struct sk_security_struct *ssksec = selinux_sock(ssk);
> +       struct sk_security_struct *sksec = selinux_sock(sk);
>
>         ssksec->sclass = sksec->sclass;
>         ssksec->sid = sksec->sid;

That's new :)

Unfortunately I merged a previous version of this patch into lsm/dev a
couple of weeks ago (see below) which appears to have a bug based on
the changes in this revision (lore link below).  While I'm generally
adverse to popping patches off the lsm/dev branch so as to not upset
any ongoing development work, given that we are at -rc7 it's probably
okay and much cleaner than doing a full revert; I'll remove that
commit now.

https://lore.kernel.org/linux-security-module/CAHC9VhQeWF814h8+ho3uKuz+NvvFApwJo4FkdmoRvYpuTcrk4A@mail.gmail.com

-- 
paul-moore.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/6] LSM: Infrastructure management of the sock security
  2024-07-09 19:15     ` Paul Moore
@ 2024-07-09 23:00       ` Casey Schaufler
  2024-07-09 23:05         ` Paul Moore
  0 siblings, 1 reply; 28+ messages in thread
From: Casey Schaufler @ 2024-07-09 23:00 UTC (permalink / raw)
  To: Paul Moore
  Cc: linux-security-module, jmorris, serge, keescook, john.johansen,
	penguin-kernel, stephen.smalley.work, mic, Casey Schaufler

On 7/9/2024 12:15 PM, Paul Moore wrote:
> On Mon, Jul 8, 2024 at 5:40 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> Move management of the sock->sk_security blob out
>> of the individual security modules and into the security
>> infrastructure. Instead of allocating the blobs from within
>> the modules the modules tell the infrastructure how much
>> space is required, and the space is allocated there.
>>
>> Acked-by: Paul Moore <paul@paul-moore.com>
>> Reviewed-by: Kees Cook <keescook@chromium.org>
>> Reviewed-by: John Johansen <john.johansen@canonical.com>
>> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>>  include/linux/lsm_hooks.h         |  1 +
>>  security/apparmor/include/net.h   |  3 +-
>>  security/apparmor/lsm.c           | 17 +------
>>  security/apparmor/net.c           |  2 +-
>>  security/security.c               | 36 +++++++++++++-
>>  security/selinux/hooks.c          | 80 ++++++++++++++-----------------
>>  security/selinux/include/objsec.h |  5 ++
>>  security/selinux/netlabel.c       | 23 ++++-----
>>  security/smack/smack.h            |  5 ++
>>  security/smack/smack_lsm.c        | 70 +++++++++++++--------------
>>  security/smack/smack_netfilter.c  |  4 +-
>>  11 files changed, 133 insertions(+), 113 deletions(-)
> ..
>
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 7eed331e90f0..19346e1817ff 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -5495,8 +5488,8 @@ static void selinux_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk
>>
>>  static int selinux_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
>>  {
>> -       struct sk_security_struct *ssksec = ssk->sk_security;
>> -       struct sk_security_struct *sksec = sk->sk_security;
>> +       struct sk_security_struct *ssksec = selinux_sock(ssk);
>> +       struct sk_security_struct *sksec = selinux_sock(sk);
>>
>>         ssksec->sclass = sksec->sclass;
>>         ssksec->sid = sksec->sid;
> That's new :)
>
> Unfortunately I merged a previous version of this patch into lsm/dev a
> couple of weeks ago (see below) which appears to have a bug based on
> the changes in this revision (lore link below).  While I'm generally
> adverse to popping patches off the lsm/dev branch so as to not upset
> any ongoing development work, given that we are at -rc7 it's probably
> okay and much cleaner than doing a full revert; I'll remove that
> commit now.

Sorry 'bout that. I had troubles with kernels built from lsm/dev crashing,
so I switched to linus' tree. 

>
> https://lore.kernel.org/linux-security-module/CAHC9VhQeWF814h8+ho3uKuz+NvvFApwJo4FkdmoRvYpuTcrk4A@mail.gmail.com
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/6] LSM: Infrastructure management of the sock security
  2024-07-09 23:00       ` Casey Schaufler
@ 2024-07-09 23:05         ` Paul Moore
  2024-07-09 23:29           ` Casey Schaufler
  0 siblings, 1 reply; 28+ messages in thread
From: Paul Moore @ 2024-07-09 23:05 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: linux-security-module, jmorris, serge, keescook, john.johansen,
	penguin-kernel, stephen.smalley.work, mic

On Tue, Jul 9, 2024 at 7:00 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 7/9/2024 12:15 PM, Paul Moore wrote:
> > On Mon, Jul 8, 2024 at 5:40 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> Move management of the sock->sk_security blob out
> >> of the individual security modules and into the security
> >> infrastructure. Instead of allocating the blobs from within
> >> the modules the modules tell the infrastructure how much
> >> space is required, and the space is allocated there.
> >>
> >> Acked-by: Paul Moore <paul@paul-moore.com>
> >> Reviewed-by: Kees Cook <keescook@chromium.org>
> >> Reviewed-by: John Johansen <john.johansen@canonical.com>
> >> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> >> ---
> >>  include/linux/lsm_hooks.h         |  1 +
> >>  security/apparmor/include/net.h   |  3 +-
> >>  security/apparmor/lsm.c           | 17 +------
> >>  security/apparmor/net.c           |  2 +-
> >>  security/security.c               | 36 +++++++++++++-
> >>  security/selinux/hooks.c          | 80 ++++++++++++++-----------------
> >>  security/selinux/include/objsec.h |  5 ++
> >>  security/selinux/netlabel.c       | 23 ++++-----
> >>  security/smack/smack.h            |  5 ++
> >>  security/smack/smack_lsm.c        | 70 +++++++++++++--------------
> >>  security/smack/smack_netfilter.c  |  4 +-
> >>  11 files changed, 133 insertions(+), 113 deletions(-)
> > ..
> >
> >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> >> index 7eed331e90f0..19346e1817ff 100644
> >> --- a/security/selinux/hooks.c
> >> +++ b/security/selinux/hooks.c
> >> @@ -5495,8 +5488,8 @@ static void selinux_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk
> >>
> >>  static int selinux_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
> >>  {
> >> -       struct sk_security_struct *ssksec = ssk->sk_security;
> >> -       struct sk_security_struct *sksec = sk->sk_security;
> >> +       struct sk_security_struct *ssksec = selinux_sock(ssk);
> >> +       struct sk_security_struct *sksec = selinux_sock(sk);
> >>
> >>         ssksec->sclass = sksec->sclass;
> >>         ssksec->sid = sksec->sid;
> > That's new :)
> >
> > Unfortunately I merged a previous version of this patch into lsm/dev a
> > couple of weeks ago (see below) which appears to have a bug based on
> > the changes in this revision (lore link below).  While I'm generally
> > adverse to popping patches off the lsm/dev branch so as to not upset
> > any ongoing development work, given that we are at -rc7 it's probably
> > okay and much cleaner than doing a full revert; I'll remove that
> > commit now.
>
> Sorry 'bout that. I had troubles with kernels built from lsm/dev crashing,
> so I switched to linus' tree.

No worries, that's fine, my policy is that I'll typically resolve
merge conflicts so long as the patches are based on either Linus' or
the subsystem tree.  In this case it turned out to be a good thing as
it highlighted the MPTCP omission in the commit merged into lsm/dev.

However, do you have any more detail on the lsm/dev crashes you are
seeing?  I wonder if it is general v6.10-rc1 instability ...

-- 
paul-moore.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/6] LSM: Infrastructure management of the sock security
  2024-07-09 23:05         ` Paul Moore
@ 2024-07-09 23:29           ` Casey Schaufler
  2024-07-10  0:00             ` Paul Moore
  0 siblings, 1 reply; 28+ messages in thread
From: Casey Schaufler @ 2024-07-09 23:29 UTC (permalink / raw)
  To: Paul Moore
  Cc: linux-security-module, jmorris, serge, keescook, john.johansen,
	penguin-kernel, stephen.smalley.work, mic, Casey Schaufler

On 7/9/2024 4:05 PM, Paul Moore wrote:
> On Tue, Jul 9, 2024 at 7:00 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 7/9/2024 12:15 PM, Paul Moore wrote:
>>> On Mon, Jul 8, 2024 at 5:40 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>> Move management of the sock->sk_security blob out
>>>> of the individual security modules and into the security
>>>> infrastructure. Instead of allocating the blobs from within
>>>> the modules the modules tell the infrastructure how much
>>>> space is required, and the space is allocated there.
>>>>
>>>> Acked-by: Paul Moore <paul@paul-moore.com>
>>>> Reviewed-by: Kees Cook <keescook@chromium.org>
>>>> Reviewed-by: John Johansen <john.johansen@canonical.com>
>>>> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
>>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>>>> ---
>>>>  include/linux/lsm_hooks.h         |  1 +
>>>>  security/apparmor/include/net.h   |  3 +-
>>>>  security/apparmor/lsm.c           | 17 +------
>>>>  security/apparmor/net.c           |  2 +-
>>>>  security/security.c               | 36 +++++++++++++-
>>>>  security/selinux/hooks.c          | 80 ++++++++++++++-----------------
>>>>  security/selinux/include/objsec.h |  5 ++
>>>>  security/selinux/netlabel.c       | 23 ++++-----
>>>>  security/smack/smack.h            |  5 ++
>>>>  security/smack/smack_lsm.c        | 70 +++++++++++++--------------
>>>>  security/smack/smack_netfilter.c  |  4 +-
>>>>  11 files changed, 133 insertions(+), 113 deletions(-)
>>> ..
>>>
>>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>>>> index 7eed331e90f0..19346e1817ff 100644
>>>> --- a/security/selinux/hooks.c
>>>> +++ b/security/selinux/hooks.c
>>>> @@ -5495,8 +5488,8 @@ static void selinux_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk
>>>>
>>>>  static int selinux_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
>>>>  {
>>>> -       struct sk_security_struct *ssksec = ssk->sk_security;
>>>> -       struct sk_security_struct *sksec = sk->sk_security;
>>>> +       struct sk_security_struct *ssksec = selinux_sock(ssk);
>>>> +       struct sk_security_struct *sksec = selinux_sock(sk);
>>>>
>>>>         ssksec->sclass = sksec->sclass;
>>>>         ssksec->sid = sksec->sid;
>>> That's new :)
>>>
>>> Unfortunately I merged a previous version of this patch into lsm/dev a
>>> couple of weeks ago (see below) which appears to have a bug based on
>>> the changes in this revision (lore link below).  While I'm generally
>>> adverse to popping patches off the lsm/dev branch so as to not upset
>>> any ongoing development work, given that we are at -rc7 it's probably
>>> okay and much cleaner than doing a full revert; I'll remove that
>>> commit now.
>> Sorry 'bout that. I had troubles with kernels built from lsm/dev crashing,
>> so I switched to linus' tree.
> No worries, that's fine, my policy is that I'll typically resolve
> merge conflicts so long as the patches are based on either Linus' or
> the subsystem tree.  In this case it turned out to be a good thing as
> it highlighted the MPTCP omission in the commit merged into lsm/dev.
>
> However, do you have any more detail on the lsm/dev crashes you are
> seeing?  I wonder if it is general v6.10-rc1 instability ...

Alas, no. My VMs just stopped hard, with no panic or traces.
The problem went away with rc3 (I did almost nothing with rc2)
so I shrugged it off and moved on.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/6] LSM: Infrastructure management of the sock security
  2024-07-09 23:29           ` Casey Schaufler
@ 2024-07-10  0:00             ` Paul Moore
  0 siblings, 0 replies; 28+ messages in thread
From: Paul Moore @ 2024-07-10  0:00 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: linux-security-module, jmorris, serge, keescook, john.johansen,
	penguin-kernel, stephen.smalley.work, mic

On Tue, Jul 9, 2024 at 7:29 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 7/9/2024 4:05 PM, Paul Moore wrote:
> > On Tue, Jul 9, 2024 at 7:00 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> On 7/9/2024 12:15 PM, Paul Moore wrote:
> >>> On Mon, Jul 8, 2024 at 5:40 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >>>> Move management of the sock->sk_security blob out
> >>>> of the individual security modules and into the security
> >>>> infrastructure. Instead of allocating the blobs from within
> >>>> the modules the modules tell the infrastructure how much
> >>>> space is required, and the space is allocated there.
> >>>>
> >>>> Acked-by: Paul Moore <paul@paul-moore.com>
> >>>> Reviewed-by: Kees Cook <keescook@chromium.org>
> >>>> Reviewed-by: John Johansen <john.johansen@canonical.com>
> >>>> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> >>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> >>>> ---
> >>>>  include/linux/lsm_hooks.h         |  1 +
> >>>>  security/apparmor/include/net.h   |  3 +-
> >>>>  security/apparmor/lsm.c           | 17 +------
> >>>>  security/apparmor/net.c           |  2 +-
> >>>>  security/security.c               | 36 +++++++++++++-
> >>>>  security/selinux/hooks.c          | 80 ++++++++++++++-----------------
> >>>>  security/selinux/include/objsec.h |  5 ++
> >>>>  security/selinux/netlabel.c       | 23 ++++-----
> >>>>  security/smack/smack.h            |  5 ++
> >>>>  security/smack/smack_lsm.c        | 70 +++++++++++++--------------
> >>>>  security/smack/smack_netfilter.c  |  4 +-
> >>>>  11 files changed, 133 insertions(+), 113 deletions(-)
> >>> ..
> >>>
> >>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> >>>> index 7eed331e90f0..19346e1817ff 100644
> >>>> --- a/security/selinux/hooks.c
> >>>> +++ b/security/selinux/hooks.c
> >>>> @@ -5495,8 +5488,8 @@ static void selinux_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk
> >>>>
> >>>>  static int selinux_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
> >>>>  {
> >>>> -       struct sk_security_struct *ssksec = ssk->sk_security;
> >>>> -       struct sk_security_struct *sksec = sk->sk_security;
> >>>> +       struct sk_security_struct *ssksec = selinux_sock(ssk);
> >>>> +       struct sk_security_struct *sksec = selinux_sock(sk);
> >>>>
> >>>>         ssksec->sclass = sksec->sclass;
> >>>>         ssksec->sid = sksec->sid;
> >>> That's new :)
> >>>
> >>> Unfortunately I merged a previous version of this patch into lsm/dev a
> >>> couple of weeks ago (see below) which appears to have a bug based on
> >>> the changes in this revision (lore link below).  While I'm generally
> >>> adverse to popping patches off the lsm/dev branch so as to not upset
> >>> any ongoing development work, given that we are at -rc7 it's probably
> >>> okay and much cleaner than doing a full revert; I'll remove that
> >>> commit now.
> >> Sorry 'bout that. I had troubles with kernels built from lsm/dev crashing,
> >> so I switched to linus' tree.
> > No worries, that's fine, my policy is that I'll typically resolve
> > merge conflicts so long as the patches are based on either Linus' or
> > the subsystem tree.  In this case it turned out to be a good thing as
> > it highlighted the MPTCP omission in the commit merged into lsm/dev.
> >
> > However, do you have any more detail on the lsm/dev crashes you are
> > seeing?  I wonder if it is general v6.10-rc1 instability ...
>
> Alas, no. My VMs just stopped hard, with no panic or traces.
> The problem went away with rc3 (I did almost nothing with rc2)
> so I shrugged it off and moved on.

Okay, fair enough.  I haven't seen anything in my testing (although
that is lsm/dev+others merged on top of a Rawhide kernel) so I guess I
won't lose too much sleep over this right now.

-- 
paul-moore.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/6] LSM: Infrastructure management of the sock security
  2024-07-08 21:39   ` [PATCH 1/6] LSM: Infrastructure management of the sock security Casey Schaufler
  2024-07-09 19:15     ` Paul Moore
@ 2024-07-09 22:08     ` Paul Moore
  2024-07-09 22:32     ` John Johansen
  2 siblings, 0 replies; 28+ messages in thread
From: Paul Moore @ 2024-07-09 22:08 UTC (permalink / raw)
  To: Casey Schaufler, casey, linux-security-module
  Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
	stephen.smalley.work, mic

On Jul  8, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote:
> 
> Move management of the sock->sk_security blob out
> of the individual security modules and into the security
> infrastructure. Instead of allocating the blobs from within
> the modules the modules tell the infrastructure how much
> space is required, and the space is allocated there.
> 
> Acked-by: Paul Moore <paul@paul-moore.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Reviewed-by: John Johansen <john.johansen@canonical.com>
> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>  include/linux/lsm_hooks.h         |  1 +
>  security/apparmor/include/net.h   |  3 +-
>  security/apparmor/lsm.c           | 17 +------
>  security/apparmor/net.c           |  2 +-
>  security/security.c               | 36 +++++++++++++-
>  security/selinux/hooks.c          | 80 ++++++++++++++-----------------
>  security/selinux/include/objsec.h |  5 ++
>  security/selinux/netlabel.c       | 23 ++++-----
>  security/smack/smack.h            |  5 ++
>  security/smack/smack_lsm.c        | 70 +++++++++++++--------------
>  security/smack/smack_netfilter.c  |  4 +-
>  11 files changed, 133 insertions(+), 113 deletions(-)

This looked fine before and with the only change being the additional
conversion in the SELinux MPTCP code it still looks good.  I'm going to
merge this into lsm/dev-staging for testing with the idea of moving it
into lsm/dev after the upcoming merge window.

If you respin you don't need to resend this patch.

--
paul-moore.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/6] LSM: Infrastructure management of the sock security
  2024-07-08 21:39   ` [PATCH 1/6] LSM: Infrastructure management of the sock security Casey Schaufler
  2024-07-09 19:15     ` Paul Moore
  2024-07-09 22:08     ` Paul Moore
@ 2024-07-09 22:32     ` John Johansen
  2 siblings, 0 replies; 28+ messages in thread
From: John Johansen @ 2024-07-09 22:32 UTC (permalink / raw)
  To: Casey Schaufler, paul, linux-security-module
  Cc: jmorris, serge, keescook, penguin-kernel, stephen.smalley.work,
	mic

On 7/8/24 14:39, Casey Schaufler wrote:
> Move management of the sock->sk_security blob out
> of the individual security modules and into the security
> infrastructure. Instead of allocating the blobs from within
> the modules the modules tell the infrastructure how much
> space is required, and the space is allocated there.
> 
> Acked-by: Paul Moore <paul@paul-moore.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Reviewed-by: John Johansen <john.johansen@canonical.com>
> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>   include/linux/lsm_hooks.h         |  1 +
>   security/apparmor/include/net.h   |  3 +-
>   security/apparmor/lsm.c           | 17 +------
>   security/apparmor/net.c           |  2 +-
>   security/security.c               | 36 +++++++++++++-
>   security/selinux/hooks.c          | 80 ++++++++++++++-----------------
>   security/selinux/include/objsec.h |  5 ++
>   security/selinux/netlabel.c       | 23 ++++-----
>   security/smack/smack.h            |  5 ++
>   security/smack/smack_lsm.c        | 70 +++++++++++++--------------
>   security/smack/smack_netfilter.c  |  4 +-
>   11 files changed, 133 insertions(+), 113 deletions(-)

still looks good

> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index a2ade0ffe9e7..efd4a0655159 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -73,6 +73,7 @@ struct lsm_blob_sizes {
>   	int	lbs_cred;
>   	int	lbs_file;
>   	int	lbs_inode;
> +	int	lbs_sock;
>   	int	lbs_superblock;
>   	int	lbs_ipc;
>   	int	lbs_msg_msg;
> diff --git a/security/apparmor/include/net.h b/security/apparmor/include/net.h
> index 67bf888c3bd6..c42ed8a73f1c 100644
> --- a/security/apparmor/include/net.h
> +++ b/security/apparmor/include/net.h
> @@ -51,10 +51,9 @@ struct aa_sk_ctx {
>   	struct aa_label *peer;
>   };
>   
> -#define SK_CTX(X) ((X)->sk_security)
>   static inline struct aa_sk_ctx *aa_sock(const struct sock *sk)
>   {
> -	return sk->sk_security;
> +	return sk->sk_security + apparmor_blob_sizes.lbs_sock;
>   }
>   
>   #define DEFINE_AUDIT_NET(NAME, OP, SK, F, T, P)				  \
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 6239777090c4..b3eb0a2f999a 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -1057,27 +1057,12 @@ static int apparmor_userns_create(const struct cred *cred)
>   	return error;
>   }
>   
> -static int apparmor_sk_alloc_security(struct sock *sk, int family, gfp_t flags)
> -{
> -	struct aa_sk_ctx *ctx;
> -
> -	ctx = kzalloc(sizeof(*ctx), flags);
> -	if (!ctx)
> -		return -ENOMEM;
> -
> -	sk->sk_security = ctx;
> -
> -	return 0;
> -}
> -
>   static void apparmor_sk_free_security(struct sock *sk)
>   {
>   	struct aa_sk_ctx *ctx = aa_sock(sk);
>   
> -	sk->sk_security = NULL;
>   	aa_put_label(ctx->label);
>   	aa_put_label(ctx->peer);
> -	kfree(ctx);
>   }
>   
>   /**
> @@ -1425,6 +1410,7 @@ struct lsm_blob_sizes apparmor_blob_sizes __ro_after_init = {
>   	.lbs_cred = sizeof(struct aa_label *),
>   	.lbs_file = sizeof(struct aa_file_ctx),
>   	.lbs_task = sizeof(struct aa_task_ctx),
> +	.lbs_sock = sizeof(struct aa_sk_ctx),
>   };
>   
>   static const struct lsm_id apparmor_lsmid = {
> @@ -1470,7 +1456,6 @@ static struct security_hook_list apparmor_hooks[] __ro_after_init = {
>   	LSM_HOOK_INIT(getprocattr, apparmor_getprocattr),
>   	LSM_HOOK_INIT(setprocattr, apparmor_setprocattr),
>   
> -	LSM_HOOK_INIT(sk_alloc_security, apparmor_sk_alloc_security),
>   	LSM_HOOK_INIT(sk_free_security, apparmor_sk_free_security),
>   	LSM_HOOK_INIT(sk_clone_security, apparmor_sk_clone_security),
>   
> diff --git a/security/apparmor/net.c b/security/apparmor/net.c
> index 87e934b2b548..77413a519117 100644
> --- a/security/apparmor/net.c
> +++ b/security/apparmor/net.c
> @@ -151,7 +151,7 @@ static int aa_label_sk_perm(const struct cred *subj_cred,
>   			    const char *op, u32 request,
>   			    struct sock *sk)
>   {
> -	struct aa_sk_ctx *ctx = SK_CTX(sk);
> +	struct aa_sk_ctx *ctx = aa_sock(sk);
>   	int error = 0;
>   
>   	AA_BUG(!label);
> diff --git a/security/security.c b/security/security.c
> index e5ca08789f74..5e93a72bdca6 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -29,6 +29,7 @@
>   #include <linux/msg.h>
>   #include <linux/overflow.h>
>   #include <net/flow.h>
> +#include <net/sock.h>
>   
>   /* How many LSMs were built into the kernel? */
>   #define LSM_COUNT (__end_lsm_info - __start_lsm_info)
> @@ -227,6 +228,7 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
>   	lsm_set_blob_size(&needed->lbs_inode, &blob_sizes.lbs_inode);
>   	lsm_set_blob_size(&needed->lbs_ipc, &blob_sizes.lbs_ipc);
>   	lsm_set_blob_size(&needed->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
> +	lsm_set_blob_size(&needed->lbs_sock, &blob_sizes.lbs_sock);
>   	lsm_set_blob_size(&needed->lbs_superblock, &blob_sizes.lbs_superblock);
>   	lsm_set_blob_size(&needed->lbs_task, &blob_sizes.lbs_task);
>   	lsm_set_blob_size(&needed->lbs_xattr_count,
> @@ -401,6 +403,7 @@ static void __init ordered_lsm_init(void)
>   	init_debug("inode blob size      = %d\n", blob_sizes.lbs_inode);
>   	init_debug("ipc blob size        = %d\n", blob_sizes.lbs_ipc);
>   	init_debug("msg_msg blob size    = %d\n", blob_sizes.lbs_msg_msg);
> +	init_debug("sock blob size       = %d\n", blob_sizes.lbs_sock);
>   	init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
>   	init_debug("task blob size       = %d\n", blob_sizes.lbs_task);
>   	init_debug("xattr slots          = %d\n", blob_sizes.lbs_xattr_count);
> @@ -4647,6 +4650,28 @@ int security_socket_getpeersec_dgram(struct socket *sock,
>   }
>   EXPORT_SYMBOL(security_socket_getpeersec_dgram);
>   
> +/**
> + * lsm_sock_alloc - allocate a composite sock blob
> + * @sock: the sock that needs a blob
> + * @priority: allocation mode
> + *
> + * Allocate the sock blob for all the modules
> + *
> + * Returns 0, or -ENOMEM if memory can't be allocated.
> + */
> +static int lsm_sock_alloc(struct sock *sock, gfp_t priority)
> +{
> +	if (blob_sizes.lbs_sock == 0) {
> +		sock->sk_security = NULL;
> +		return 0;
> +	}
> +
> +	sock->sk_security = kzalloc(blob_sizes.lbs_sock, priority);
> +	if (sock->sk_security == NULL)
> +		return -ENOMEM;
> +	return 0;
> +}
> +
>   /**
>    * security_sk_alloc() - Allocate and initialize a sock's LSM blob
>    * @sk: sock
> @@ -4660,7 +4685,14 @@ EXPORT_SYMBOL(security_socket_getpeersec_dgram);
>    */
>   int security_sk_alloc(struct sock *sk, int family, gfp_t priority)
>   {
> -	return call_int_hook(sk_alloc_security, sk, family, priority);
> +	int rc = lsm_sock_alloc(sk, priority);
> +
> +	if (unlikely(rc))
> +		return rc;
> +	rc = call_int_hook(sk_alloc_security, sk, family, priority);
> +	if (unlikely(rc))
> +		security_sk_free(sk);
> +	return rc;
>   }
>   
>   /**
> @@ -4672,6 +4704,8 @@ int security_sk_alloc(struct sock *sk, int family, gfp_t priority)
>   void security_sk_free(struct sock *sk)
>   {
>   	call_void_hook(sk_free_security, sk);
> +	kfree(sk->sk_security);
> +	sk->sk_security = NULL;
>   }
>   
>   /**
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 7eed331e90f0..19346e1817ff 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4579,7 +4579,7 @@ static int socket_sockcreate_sid(const struct task_security_struct *tsec,
>   
>   static int sock_has_perm(struct sock *sk, u32 perms)
>   {
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   	struct common_audit_data ad;
>   	struct lsm_network_audit net;
>   
> @@ -4647,7 +4647,7 @@ static int selinux_socket_post_create(struct socket *sock, int family,
>   	isec->initialized = LABEL_INITIALIZED;
>   
>   	if (sock->sk) {
> -		sksec = sock->sk->sk_security;
> +		sksec = selinux_sock(sock->sk);
>   		sksec->sclass = sclass;
>   		sksec->sid = sid;
>   		/* Allows detection of the first association on this socket */
> @@ -4663,8 +4663,8 @@ static int selinux_socket_post_create(struct socket *sock, int family,
>   static int selinux_socket_socketpair(struct socket *socka,
>   				     struct socket *sockb)
>   {
> -	struct sk_security_struct *sksec_a = socka->sk->sk_security;
> -	struct sk_security_struct *sksec_b = sockb->sk->sk_security;
> +	struct sk_security_struct *sksec_a = selinux_sock(socka->sk);
> +	struct sk_security_struct *sksec_b = selinux_sock(sockb->sk);
>   
>   	sksec_a->peer_sid = sksec_b->sid;
>   	sksec_b->peer_sid = sksec_a->sid;
> @@ -4679,7 +4679,7 @@ static int selinux_socket_socketpair(struct socket *socka,
>   static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, int addrlen)
>   {
>   	struct sock *sk = sock->sk;
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   	u16 family;
>   	int err;
>   
> @@ -4819,7 +4819,7 @@ static int selinux_socket_connect_helper(struct socket *sock,
>   					 struct sockaddr *address, int addrlen)
>   {
>   	struct sock *sk = sock->sk;
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   	int err;
>   
>   	err = sock_has_perm(sk, SOCKET__CONNECT);
> @@ -4997,9 +4997,9 @@ static int selinux_socket_unix_stream_connect(struct sock *sock,
>   					      struct sock *other,
>   					      struct sock *newsk)
>   {
> -	struct sk_security_struct *sksec_sock = sock->sk_security;
> -	struct sk_security_struct *sksec_other = other->sk_security;
> -	struct sk_security_struct *sksec_new = newsk->sk_security;
> +	struct sk_security_struct *sksec_sock = selinux_sock(sock);
> +	struct sk_security_struct *sksec_other = selinux_sock(other);
> +	struct sk_security_struct *sksec_new = selinux_sock(newsk);
>   	struct common_audit_data ad;
>   	struct lsm_network_audit net;
>   	int err;
> @@ -5028,8 +5028,8 @@ static int selinux_socket_unix_stream_connect(struct sock *sock,
>   static int selinux_socket_unix_may_send(struct socket *sock,
>   					struct socket *other)
>   {
> -	struct sk_security_struct *ssec = sock->sk->sk_security;
> -	struct sk_security_struct *osec = other->sk->sk_security;
> +	struct sk_security_struct *ssec = selinux_sock(sock->sk);
> +	struct sk_security_struct *osec = selinux_sock(other->sk);
>   	struct common_audit_data ad;
>   	struct lsm_network_audit net;
>   
> @@ -5066,7 +5066,7 @@ static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb,
>   				       u16 family)
>   {
>   	int err = 0;
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   	u32 sk_sid = sksec->sid;
>   	struct common_audit_data ad;
>   	struct lsm_network_audit net;
> @@ -5095,7 +5095,7 @@ static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb,
>   static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
>   {
>   	int err, peerlbl_active, secmark_active;
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   	u16 family = sk->sk_family;
>   	u32 sk_sid = sksec->sid;
>   	struct common_audit_data ad;
> @@ -5163,7 +5163,7 @@ static int selinux_socket_getpeersec_stream(struct socket *sock,
>   	int err = 0;
>   	char *scontext = NULL;
>   	u32 scontext_len;
> -	struct sk_security_struct *sksec = sock->sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sock->sk);
>   	u32 peer_sid = SECSID_NULL;
>   
>   	if (sksec->sclass == SECCLASS_UNIX_STREAM_SOCKET ||
> @@ -5223,34 +5223,27 @@ static int selinux_socket_getpeersec_dgram(struct socket *sock,
>   
>   static int selinux_sk_alloc_security(struct sock *sk, int family, gfp_t priority)
>   {
> -	struct sk_security_struct *sksec;
> -
> -	sksec = kzalloc(sizeof(*sksec), priority);
> -	if (!sksec)
> -		return -ENOMEM;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   
>   	sksec->peer_sid = SECINITSID_UNLABELED;
>   	sksec->sid = SECINITSID_UNLABELED;
>   	sksec->sclass = SECCLASS_SOCKET;
>   	selinux_netlbl_sk_security_reset(sksec);
> -	sk->sk_security = sksec;
>   
>   	return 0;
>   }
>   
>   static void selinux_sk_free_security(struct sock *sk)
>   {
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   
> -	sk->sk_security = NULL;
>   	selinux_netlbl_sk_security_free(sksec);
> -	kfree(sksec);
>   }
>   
>   static void selinux_sk_clone_security(const struct sock *sk, struct sock *newsk)
>   {
> -	struct sk_security_struct *sksec = sk->sk_security;
> -	struct sk_security_struct *newsksec = newsk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
> +	struct sk_security_struct *newsksec = selinux_sock(newsk);
>   
>   	newsksec->sid = sksec->sid;
>   	newsksec->peer_sid = sksec->peer_sid;
> @@ -5264,7 +5257,7 @@ static void selinux_sk_getsecid(const struct sock *sk, u32 *secid)
>   	if (!sk)
>   		*secid = SECINITSID_ANY_SOCKET;
>   	else {
> -		const struct sk_security_struct *sksec = sk->sk_security;
> +		const struct sk_security_struct *sksec = selinux_sock(sk);
>   
>   		*secid = sksec->sid;
>   	}
> @@ -5274,7 +5267,7 @@ static void selinux_sock_graft(struct sock *sk, struct socket *parent)
>   {
>   	struct inode_security_struct *isec =
>   		inode_security_novalidate(SOCK_INODE(parent));
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   
>   	if (sk->sk_family == PF_INET || sk->sk_family == PF_INET6 ||
>   	    sk->sk_family == PF_UNIX)
> @@ -5291,7 +5284,7 @@ static int selinux_sctp_process_new_assoc(struct sctp_association *asoc,
>   {
>   	struct sock *sk = asoc->base.sk;
>   	u16 family = sk->sk_family;
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   	struct common_audit_data ad;
>   	struct lsm_network_audit net;
>   	int err;
> @@ -5346,7 +5339,7 @@ static int selinux_sctp_process_new_assoc(struct sctp_association *asoc,
>   static int selinux_sctp_assoc_request(struct sctp_association *asoc,
>   				      struct sk_buff *skb)
>   {
> -	struct sk_security_struct *sksec = asoc->base.sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(asoc->base.sk);
>   	u32 conn_sid;
>   	int err;
>   
> @@ -5379,7 +5372,7 @@ static int selinux_sctp_assoc_request(struct sctp_association *asoc,
>   static int selinux_sctp_assoc_established(struct sctp_association *asoc,
>   					  struct sk_buff *skb)
>   {
> -	struct sk_security_struct *sksec = asoc->base.sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(asoc->base.sk);
>   
>   	if (!selinux_policycap_extsockclass())
>   		return 0;
> @@ -5478,8 +5471,8 @@ static int selinux_sctp_bind_connect(struct sock *sk, int optname,
>   static void selinux_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk,
>   				  struct sock *newsk)
>   {
> -	struct sk_security_struct *sksec = sk->sk_security;
> -	struct sk_security_struct *newsksec = newsk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
> +	struct sk_security_struct *newsksec = selinux_sock(newsk);
>   
>   	/* If policy does not support SECCLASS_SCTP_SOCKET then call
>   	 * the non-sctp clone version.
> @@ -5495,8 +5488,8 @@ static void selinux_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk
>   
>   static int selinux_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
>   {
> -	struct sk_security_struct *ssksec = ssk->sk_security;
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct sk_security_struct *ssksec = selinux_sock(ssk);
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   
>   	ssksec->sclass = sksec->sclass;
>   	ssksec->sid = sksec->sid;
> @@ -5511,7 +5504,7 @@ static int selinux_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
>   static int selinux_inet_conn_request(const struct sock *sk, struct sk_buff *skb,
>   				     struct request_sock *req)
>   {
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   	int err;
>   	u16 family = req->rsk_ops->family;
>   	u32 connsid;
> @@ -5532,7 +5525,7 @@ static int selinux_inet_conn_request(const struct sock *sk, struct sk_buff *skb,
>   static void selinux_inet_csk_clone(struct sock *newsk,
>   				   const struct request_sock *req)
>   {
> -	struct sk_security_struct *newsksec = newsk->sk_security;
> +	struct sk_security_struct *newsksec = selinux_sock(newsk);
>   
>   	newsksec->sid = req->secid;
>   	newsksec->peer_sid = req->peer_secid;
> @@ -5549,7 +5542,7 @@ static void selinux_inet_csk_clone(struct sock *newsk,
>   static void selinux_inet_conn_established(struct sock *sk, struct sk_buff *skb)
>   {
>   	u16 family = sk->sk_family;
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   
>   	/* handle mapped IPv4 packets arriving via IPv6 sockets */
>   	if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP))
> @@ -5624,7 +5617,7 @@ static int selinux_tun_dev_attach_queue(void *security)
>   static int selinux_tun_dev_attach(struct sock *sk, void *security)
>   {
>   	struct tun_security_struct *tunsec = security;
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   
>   	/* we don't currently perform any NetLabel based labeling here and it
>   	 * isn't clear that we would want to do so anyway; while we could apply
> @@ -5747,7 +5740,7 @@ static unsigned int selinux_ip_output(void *priv, struct sk_buff *skb,
>   			return NF_ACCEPT;
>   
>   		/* standard practice, label using the parent socket */
> -		sksec = sk->sk_security;
> +		sksec = selinux_sock(sk);
>   		sid = sksec->sid;
>   	} else
>   		sid = SECINITSID_KERNEL;
> @@ -5770,7 +5763,7 @@ static unsigned int selinux_ip_postroute_compat(struct sk_buff *skb,
>   	sk = skb_to_full_sk(skb);
>   	if (sk == NULL)
>   		return NF_ACCEPT;
> -	sksec = sk->sk_security;
> +	sksec = selinux_sock(sk);
>   
>   	ad_net_init_from_iif(&ad, &net, state->out->ifindex, state->pf);
>   	if (selinux_parse_skb(skb, &ad, NULL, 0, &proto))
> @@ -5859,7 +5852,7 @@ static unsigned int selinux_ip_postroute(void *priv,
>   		u32 skb_sid;
>   		struct sk_security_struct *sksec;
>   
> -		sksec = sk->sk_security;
> +		sksec = selinux_sock(sk);
>   		if (selinux_skb_peerlbl_sid(skb, family, &skb_sid))
>   			return NF_DROP;
>   		/* At this point, if the returned skb peerlbl is SECSID_NULL
> @@ -5888,7 +5881,7 @@ static unsigned int selinux_ip_postroute(void *priv,
>   	} else {
>   		/* Locally generated packet, fetch the security label from the
>   		 * associated socket. */
> -		struct sk_security_struct *sksec = sk->sk_security;
> +		struct sk_security_struct *sksec = selinux_sock(sk);
>   		peer_sid = sksec->sid;
>   		secmark_perm = PACKET__SEND;
>   	}
> @@ -5931,7 +5924,7 @@ static int selinux_netlink_send(struct sock *sk, struct sk_buff *skb)
>   	unsigned int data_len = skb->len;
>   	unsigned char *data = skb->data;
>   	struct nlmsghdr *nlh;
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   	u16 sclass = sksec->sclass;
>   	u32 perm;
>   
> @@ -6989,6 +6982,7 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = {
>   	.lbs_inode = sizeof(struct inode_security_struct),
>   	.lbs_ipc = sizeof(struct ipc_security_struct),
>   	.lbs_msg_msg = sizeof(struct msg_security_struct),
> +	.lbs_sock = sizeof(struct sk_security_struct),
>   	.lbs_superblock = sizeof(struct superblock_security_struct),
>   	.lbs_xattr_count = SELINUX_INODE_INIT_XATTRS,
>   };
> diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
> index dea1d6f3ed2d..b074099acbaf 100644
> --- a/security/selinux/include/objsec.h
> +++ b/security/selinux/include/objsec.h
> @@ -195,4 +195,9 @@ selinux_superblock(const struct super_block *superblock)
>   	return superblock->s_security + selinux_blob_sizes.lbs_superblock;
>   }
>   
> +static inline struct sk_security_struct *selinux_sock(const struct sock *sock)
> +{
> +	return sock->sk_security + selinux_blob_sizes.lbs_sock;
> +}
> +
>   #endif /* _SELINUX_OBJSEC_H_ */
> diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c
> index 55885634e880..fbe5f8c29f81 100644
> --- a/security/selinux/netlabel.c
> +++ b/security/selinux/netlabel.c
> @@ -17,6 +17,7 @@
>   #include <linux/gfp.h>
>   #include <linux/ip.h>
>   #include <linux/ipv6.h>
> +#include <linux/lsm_hooks.h>
>   #include <net/sock.h>
>   #include <net/netlabel.h>
>   #include <net/ip.h>
> @@ -68,7 +69,7 @@ static int selinux_netlbl_sidlookup_cached(struct sk_buff *skb,
>   static struct netlbl_lsm_secattr *selinux_netlbl_sock_genattr(struct sock *sk)
>   {
>   	int rc;
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   	struct netlbl_lsm_secattr *secattr;
>   
>   	if (sksec->nlbl_secattr != NULL)
> @@ -100,7 +101,7 @@ static struct netlbl_lsm_secattr *selinux_netlbl_sock_getattr(
>   							const struct sock *sk,
>   							u32 sid)
>   {
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   	struct netlbl_lsm_secattr *secattr = sksec->nlbl_secattr;
>   
>   	if (secattr == NULL)
> @@ -240,7 +241,7 @@ int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
>   	 * being labeled by it's parent socket, if it is just exit */
>   	sk = skb_to_full_sk(skb);
>   	if (sk != NULL) {
> -		struct sk_security_struct *sksec = sk->sk_security;
> +		struct sk_security_struct *sksec = selinux_sock(sk);
>   
>   		if (sksec->nlbl_state != NLBL_REQSKB)
>   			return 0;
> @@ -277,7 +278,7 @@ int selinux_netlbl_sctp_assoc_request(struct sctp_association *asoc,
>   {
>   	int rc;
>   	struct netlbl_lsm_secattr secattr;
> -	struct sk_security_struct *sksec = asoc->base.sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(asoc->base.sk);
>   	struct sockaddr_in addr4;
>   	struct sockaddr_in6 addr6;
>   
> @@ -356,7 +357,7 @@ int selinux_netlbl_inet_conn_request(struct request_sock *req, u16 family)
>    */
>   void selinux_netlbl_inet_csk_clone(struct sock *sk, u16 family)
>   {
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   
>   	if (family == PF_INET)
>   		sksec->nlbl_state = NLBL_LABELED;
> @@ -374,8 +375,8 @@ void selinux_netlbl_inet_csk_clone(struct sock *sk, u16 family)
>    */
>   void selinux_netlbl_sctp_sk_clone(struct sock *sk, struct sock *newsk)
>   {
> -	struct sk_security_struct *sksec = sk->sk_security;
> -	struct sk_security_struct *newsksec = newsk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
> +	struct sk_security_struct *newsksec = selinux_sock(newsk);
>   
>   	newsksec->nlbl_state = sksec->nlbl_state;
>   }
> @@ -393,7 +394,7 @@ void selinux_netlbl_sctp_sk_clone(struct sock *sk, struct sock *newsk)
>   int selinux_netlbl_socket_post_create(struct sock *sk, u16 family)
>   {
>   	int rc;
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   	struct netlbl_lsm_secattr *secattr;
>   
>   	if (family != PF_INET && family != PF_INET6)
> @@ -510,7 +511,7 @@ int selinux_netlbl_socket_setsockopt(struct socket *sock,
>   {
>   	int rc = 0;
>   	struct sock *sk = sock->sk;
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   	struct netlbl_lsm_secattr secattr;
>   
>   	if (selinux_netlbl_option(level, optname) &&
> @@ -548,7 +549,7 @@ static int selinux_netlbl_socket_connect_helper(struct sock *sk,
>   						struct sockaddr *addr)
>   {
>   	int rc;
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   	struct netlbl_lsm_secattr *secattr;
>   
>   	/* connected sockets are allowed to disconnect when the address family
> @@ -587,7 +588,7 @@ static int selinux_netlbl_socket_connect_helper(struct sock *sk,
>   int selinux_netlbl_socket_connect_locked(struct sock *sk,
>   					 struct sockaddr *addr)
>   {
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct sk_security_struct *sksec = selinux_sock(sk);
>   
>   	if (sksec->nlbl_state != NLBL_REQSKB &&
>   	    sksec->nlbl_state != NLBL_CONNLABELED)
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index 041688e5a77a..297f21446f45 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -355,6 +355,11 @@ static inline struct superblock_smack *smack_superblock(
>   	return superblock->s_security + smack_blob_sizes.lbs_superblock;
>   }
>   
> +static inline struct socket_smack *smack_sock(const struct sock *sock)
> +{
> +	return sock->sk_security + smack_blob_sizes.lbs_sock;
> +}
> +
>   /*
>    * Is the directory transmuting?
>    */
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index f5cbec1e6a92..a931b44bc959 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -1581,7 +1581,7 @@ static int smack_inode_getsecurity(struct mnt_idmap *idmap,
>   		if (sock == NULL || sock->sk == NULL)
>   			return -EOPNOTSUPP;
>   
> -		ssp = sock->sk->sk_security;
> +		ssp = smack_sock(sock->sk);
>   
>   		if (strcmp(name, XATTR_SMACK_IPIN) == 0)
>   			isp = ssp->smk_in;
> @@ -1969,7 +1969,7 @@ static int smack_file_receive(struct file *file)
>   
>   	if (inode->i_sb->s_magic == SOCKFS_MAGIC) {
>   		sock = SOCKET_I(inode);
> -		ssp = sock->sk->sk_security;
> +		ssp = smack_sock(sock->sk);
>   		tsp = smack_cred(current_cred());
>   		/*
>   		 * If the receiving process can't write to the
> @@ -2384,11 +2384,7 @@ static void smack_task_to_inode(struct task_struct *p, struct inode *inode)
>   static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t gfp_flags)
>   {
>   	struct smack_known *skp = smk_of_current();
> -	struct socket_smack *ssp;
> -
> -	ssp = kzalloc(sizeof(struct socket_smack), gfp_flags);
> -	if (ssp == NULL)
> -		return -ENOMEM;
> +	struct socket_smack *ssp = smack_sock(sk);
>   
>   	/*
>   	 * Sockets created by kernel threads receive web label.
> @@ -2402,11 +2398,10 @@ static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t gfp_flags)
>   	}
>   	ssp->smk_packet = NULL;
>   
> -	sk->sk_security = ssp;
> -
>   	return 0;
>   }
>   
> +#ifdef SMACK_IPV6_PORT_LABELING
>   /**
>    * smack_sk_free_security - Free a socket blob
>    * @sk: the socket
> @@ -2415,7 +2410,6 @@ 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) {
> @@ -2428,9 +2422,8 @@ static void smack_sk_free_security(struct sock *sk)
>   		}
>   		rcu_read_unlock();
>   	}
> -#endif
> -	kfree(sk->sk_security);
>   }
> +#endif
>   
>   /**
>    * smack_sk_clone_security - Copy security context
> @@ -2441,8 +2434,8 @@ static void smack_sk_free_security(struct sock *sk)
>    */
>   static void smack_sk_clone_security(const struct sock *sk, struct sock *newsk)
>   {
> -	struct socket_smack *ssp_old = sk->sk_security;
> -	struct socket_smack *ssp_new = newsk->sk_security;
> +	struct socket_smack *ssp_old = smack_sock(sk);
> +	struct socket_smack *ssp_new = smack_sock(newsk);
>   
>   	*ssp_new = *ssp_old;
>   }
> @@ -2558,7 +2551,7 @@ static struct smack_known *smack_ipv6host_label(struct sockaddr_in6 *sip)
>    */
>   static int smack_netlbl_add(struct sock *sk)
>   {
> -	struct socket_smack *ssp = sk->sk_security;
> +	struct socket_smack *ssp = smack_sock(sk);
>   	struct smack_known *skp = ssp->smk_out;
>   	int rc;
>   
> @@ -2591,7 +2584,7 @@ static int smack_netlbl_add(struct sock *sk)
>    */
>   static void smack_netlbl_delete(struct sock *sk)
>   {
> -	struct socket_smack *ssp = sk->sk_security;
> +	struct socket_smack *ssp = smack_sock(sk);
>   
>   	/*
>   	 * Take the label off the socket if one is set.
> @@ -2623,7 +2616,7 @@ static int smk_ipv4_check(struct sock *sk, struct sockaddr_in *sap)
>   	struct smack_known *skp;
>   	int rc = 0;
>   	struct smack_known *hkp;
> -	struct socket_smack *ssp = sk->sk_security;
> +	struct socket_smack *ssp = smack_sock(sk);
>   	struct smk_audit_info ad;
>   
>   	rcu_read_lock();
> @@ -2696,7 +2689,7 @@ static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address)
>   {
>   	struct sock *sk = sock->sk;
>   	struct sockaddr_in6 *addr6;
> -	struct socket_smack *ssp = sock->sk->sk_security;
> +	struct socket_smack *ssp = smack_sock(sock->sk);
>   	struct smk_port_label *spp;
>   	unsigned short port = 0;
>   
> @@ -2784,7 +2777,7 @@ static int smk_ipv6_port_check(struct sock *sk, struct sockaddr_in6 *address,
>   				int act)
>   {
>   	struct smk_port_label *spp;
> -	struct socket_smack *ssp = sk->sk_security;
> +	struct socket_smack *ssp = smack_sock(sk);
>   	struct smack_known *skp = NULL;
>   	unsigned short port;
>   	struct smack_known *object;
> @@ -2887,7 +2880,7 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
>   	if (sock == NULL || sock->sk == NULL)
>   		return -EOPNOTSUPP;
>   
> -	ssp = sock->sk->sk_security;
> +	ssp = smack_sock(sock->sk);
>   
>   	if (strcmp(name, XATTR_SMACK_IPIN) == 0)
>   		ssp->smk_in = skp;
> @@ -2935,7 +2928,7 @@ static int smack_socket_post_create(struct socket *sock, int family,
>   	 * Sockets created by kernel threads receive web label.
>   	 */
>   	if (unlikely(current->flags & PF_KTHREAD)) {
> -		ssp = sock->sk->sk_security;
> +		ssp = smack_sock(sock->sk);
>   		ssp->smk_in = &smack_known_web;
>   		ssp->smk_out = &smack_known_web;
>   	}
> @@ -2960,8 +2953,8 @@ static int smack_socket_post_create(struct socket *sock, int family,
>   static int smack_socket_socketpair(struct socket *socka,
>   		                   struct socket *sockb)
>   {
> -	struct socket_smack *asp = socka->sk->sk_security;
> -	struct socket_smack *bsp = sockb->sk->sk_security;
> +	struct socket_smack *asp = smack_sock(socka->sk);
> +	struct socket_smack *bsp = smack_sock(sockb->sk);
>   
>   	asp->smk_packet = bsp->smk_out;
>   	bsp->smk_packet = asp->smk_out;
> @@ -3024,7 +3017,7 @@ static int smack_socket_connect(struct socket *sock, struct sockaddr *sap,
>   		if (__is_defined(SMACK_IPV6_SECMARK_LABELING))
>   			rsp = smack_ipv6host_label(sip);
>   		if (rsp != NULL) {
> -			struct socket_smack *ssp = sock->sk->sk_security;
> +			struct socket_smack *ssp = smack_sock(sock->sk);
>   
>   			rc = smk_ipv6_check(ssp->smk_out, rsp, sip,
>   					    SMK_CONNECTING);
> @@ -3819,9 +3812,9 @@ static int smack_unix_stream_connect(struct sock *sock,
>   {
>   	struct smack_known *skp;
>   	struct smack_known *okp;
> -	struct socket_smack *ssp = sock->sk_security;
> -	struct socket_smack *osp = other->sk_security;
> -	struct socket_smack *nsp = newsk->sk_security;
> +	struct socket_smack *ssp = smack_sock(sock);
> +	struct socket_smack *osp = smack_sock(other);
> +	struct socket_smack *nsp = smack_sock(newsk);
>   	struct smk_audit_info ad;
>   	int rc = 0;
>   #ifdef CONFIG_AUDIT
> @@ -3867,8 +3860,8 @@ static int smack_unix_stream_connect(struct sock *sock,
>    */
>   static int smack_unix_may_send(struct socket *sock, struct socket *other)
>   {
> -	struct socket_smack *ssp = sock->sk->sk_security;
> -	struct socket_smack *osp = other->sk->sk_security;
> +	struct socket_smack *ssp = smack_sock(sock->sk);
> +	struct socket_smack *osp = smack_sock(other->sk);
>   	struct smk_audit_info ad;
>   	int rc;
>   
> @@ -3905,7 +3898,7 @@ static int smack_socket_sendmsg(struct socket *sock, struct msghdr *msg,
>   	struct sockaddr_in6 *sap = (struct sockaddr_in6 *) msg->msg_name;
>   #endif
>   #ifdef SMACK_IPV6_SECMARK_LABELING
> -	struct socket_smack *ssp = sock->sk->sk_security;
> +	struct socket_smack *ssp = smack_sock(sock->sk);
>   	struct smack_known *rsp;
>   #endif
>   	int rc = 0;
> @@ -4117,7 +4110,7 @@ static struct smack_known *smack_from_netlbl(const struct sock *sk, u16 family,
>   	netlbl_secattr_init(&secattr);
>   
>   	if (sk)
> -		ssp = sk->sk_security;
> +		ssp = smack_sock(sk);
>   
>   	if (netlbl_skbuff_getattr(skb, family, &secattr) == 0) {
>   		skp = smack_from_secattr(&secattr, ssp);
> @@ -4139,7 +4132,7 @@ static struct smack_known *smack_from_netlbl(const struct sock *sk, u16 family,
>    */
>   static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
>   {
> -	struct socket_smack *ssp = sk->sk_security;
> +	struct socket_smack *ssp = smack_sock(sk);
>   	struct smack_known *skp = NULL;
>   	int rc = 0;
>   	struct smk_audit_info ad;
> @@ -4243,7 +4236,7 @@ static int smack_socket_getpeersec_stream(struct socket *sock,
>   	u32 slen = 1;
>   	int rc = 0;
>   
> -	ssp = sock->sk->sk_security;
> +	ssp = smack_sock(sock->sk);
>   	if (ssp->smk_packet != NULL) {
>   		rcp = ssp->smk_packet->smk_known;
>   		slen = strlen(rcp) + 1;
> @@ -4293,7 +4286,7 @@ static int smack_socket_getpeersec_dgram(struct socket *sock,
>   
>   	switch (family) {
>   	case PF_UNIX:
> -		ssp = sock->sk->sk_security;
> +		ssp = smack_sock(sock->sk);
>   		s = ssp->smk_out->smk_secid;
>   		break;
>   	case PF_INET:
> @@ -4342,7 +4335,7 @@ static void smack_sock_graft(struct sock *sk, struct socket *parent)
>   	    (sk->sk_family != PF_INET && sk->sk_family != PF_INET6))
>   		return;
>   
> -	ssp = sk->sk_security;
> +	ssp = smack_sock(sk);
>   	ssp->smk_in = skp;
>   	ssp->smk_out = skp;
>   	/* cssp->smk_packet is already set in smack_inet_csk_clone() */
> @@ -4362,7 +4355,7 @@ static int smack_inet_conn_request(const struct sock *sk, struct sk_buff *skb,
>   {
>   	u16 family = sk->sk_family;
>   	struct smack_known *skp;
> -	struct socket_smack *ssp = sk->sk_security;
> +	struct socket_smack *ssp = smack_sock(sk);
>   	struct sockaddr_in addr;
>   	struct iphdr *hdr;
>   	struct smack_known *hskp;
> @@ -4448,7 +4441,7 @@ static int smack_inet_conn_request(const struct sock *sk, struct sk_buff *skb,
>   static void smack_inet_csk_clone(struct sock *sk,
>   				 const struct request_sock *req)
>   {
> -	struct socket_smack *ssp = sk->sk_security;
> +	struct socket_smack *ssp = smack_sock(sk);
>   	struct smack_known *skp;
>   
>   	if (req->peer_secid != 0) {
> @@ -5018,6 +5011,7 @@ struct lsm_blob_sizes smack_blob_sizes __ro_after_init = {
>   	.lbs_inode = sizeof(struct inode_smack),
>   	.lbs_ipc = sizeof(struct smack_known *),
>   	.lbs_msg_msg = sizeof(struct smack_known *),
> +	.lbs_sock = sizeof(struct socket_smack),
>   	.lbs_superblock = sizeof(struct superblock_smack),
>   	.lbs_xattr_count = SMACK_INODE_INIT_XATTRS,
>   };
> @@ -5141,7 +5135,9 @@ static struct security_hook_list smack_hooks[] __ro_after_init = {
>   	LSM_HOOK_INIT(socket_getpeersec_stream, smack_socket_getpeersec_stream),
>   	LSM_HOOK_INIT(socket_getpeersec_dgram, smack_socket_getpeersec_dgram),
>   	LSM_HOOK_INIT(sk_alloc_security, smack_sk_alloc_security),
> +#ifdef SMACK_IPV6_PORT_LABELING
>   	LSM_HOOK_INIT(sk_free_security, smack_sk_free_security),
> +#endif
>   	LSM_HOOK_INIT(sk_clone_security, smack_sk_clone_security),
>   	LSM_HOOK_INIT(sock_graft, smack_sock_graft),
>   	LSM_HOOK_INIT(inet_conn_request, smack_inet_conn_request),
> diff --git a/security/smack/smack_netfilter.c b/security/smack/smack_netfilter.c
> index b945c1d3a743..bad71b7e648d 100644
> --- a/security/smack/smack_netfilter.c
> +++ b/security/smack/smack_netfilter.c
> @@ -26,8 +26,8 @@ static unsigned int smack_ip_output(void *priv,
>   	struct socket_smack *ssp;
>   	struct smack_known *skp;
>   
> -	if (sk && sk->sk_security) {
> -		ssp = sk->sk_security;
> +	if (sk) {
> +		ssp = smack_sock(sk);
>   		skp = ssp->smk_out;
>   		skb->secmark = skp->smk_secid;
>   	}


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 2/6] LSM: Infrastructure management of the key security blob
  2024-07-08 21:39 ` [PATCH 0/6] LSM: Infrastructure blob allocation Casey Schaufler
  2024-07-08 21:39   ` [PATCH 1/6] LSM: Infrastructure management of the sock security Casey Schaufler
@ 2024-07-08 21:39   ` Casey Schaufler
  2024-07-09 22:08     ` Paul Moore
  2024-07-08 21:39   ` [PATCH 3/6] LSM: Add helper for blob allocations Casey Schaufler
                     ` (3 subsequent siblings)
  5 siblings, 1 reply; 28+ messages in thread
From: Casey Schaufler @ 2024-07-08 21:39 UTC (permalink / raw)
  To: casey, paul, linux-security-module
  Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
	stephen.smalley.work, mic

Move management of the key->security blob out of the
individual security modules and into the security
infrastructure. Instead of allocating the blobs from within
the modules the modules tell the infrastructure how much
space is required, and the space is allocated there.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 include/linux/lsm_hooks.h         |  1 +
 security/security.c               | 41 +++++++++++++++++++++++++++++--
 security/selinux/hooks.c          | 23 +++++------------
 security/selinux/include/objsec.h |  7 ++++++
 security/smack/smack.h            |  7 ++++++
 security/smack/smack_lsm.c        | 33 +++++++++++--------------
 6 files changed, 75 insertions(+), 37 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index efd4a0655159..7233bc0737be 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -76,6 +76,7 @@ struct lsm_blob_sizes {
 	int	lbs_sock;
 	int	lbs_superblock;
 	int	lbs_ipc;
+	int	lbs_key;
 	int	lbs_msg_msg;
 	int	lbs_task;
 	int	lbs_xattr_count; /* number of xattr slots in new_xattrs array */
diff --git a/security/security.c b/security/security.c
index 5e93a72bdca6..aae37481b7be 100644
--- a/security/security.c
+++ b/security/security.c
@@ -227,6 +227,9 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
 		blob_sizes.lbs_inode = sizeof(struct rcu_head);
 	lsm_set_blob_size(&needed->lbs_inode, &blob_sizes.lbs_inode);
 	lsm_set_blob_size(&needed->lbs_ipc, &blob_sizes.lbs_ipc);
+#ifdef CONFIG_KEYS
+	lsm_set_blob_size(&needed->lbs_key, &blob_sizes.lbs_key);
+#endif
 	lsm_set_blob_size(&needed->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
 	lsm_set_blob_size(&needed->lbs_sock, &blob_sizes.lbs_sock);
 	lsm_set_blob_size(&needed->lbs_superblock, &blob_sizes.lbs_superblock);
@@ -402,6 +405,9 @@ static void __init ordered_lsm_init(void)
 	init_debug("file blob size       = %d\n", blob_sizes.lbs_file);
 	init_debug("inode blob size      = %d\n", blob_sizes.lbs_inode);
 	init_debug("ipc blob size        = %d\n", blob_sizes.lbs_ipc);
+#ifdef CONFIG_KEYS
+	init_debug("key blob size        = %d\n", blob_sizes.lbs_key);
+#endif /* CONFIG_KEYS */
 	init_debug("msg_msg blob size    = %d\n", blob_sizes.lbs_msg_msg);
 	init_debug("sock blob size       = %d\n", blob_sizes.lbs_sock);
 	init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
@@ -718,6 +724,29 @@ static int lsm_ipc_alloc(struct kern_ipc_perm *kip)
 	return 0;
 }
 
+#ifdef CONFIG_KEYS
+/**
+ * lsm_key_alloc - allocate a composite key blob
+ * @key: the key that needs a blob
+ *
+ * Allocate the key blob for all the modules
+ *
+ * Returns 0, or -ENOMEM if memory can't be allocated.
+ */
+static int lsm_key_alloc(struct key *key)
+{
+	if (blob_sizes.lbs_key == 0) {
+		key->security = NULL;
+		return 0;
+	}
+
+	key->security = kzalloc(blob_sizes.lbs_key, GFP_KERNEL);
+	if (key->security == NULL)
+		return -ENOMEM;
+	return 0;
+}
+#endif /* CONFIG_KEYS */
+
 /**
  * lsm_msg_msg_alloc - allocate a composite msg_msg blob
  * @mp: the msg_msg that needs a blob
@@ -5290,7 +5319,14 @@ EXPORT_SYMBOL(security_skb_classify_flow);
 int security_key_alloc(struct key *key, const struct cred *cred,
 		       unsigned long flags)
 {
-	return call_int_hook(key_alloc, key, cred, flags);
+	int rc = lsm_key_alloc(key);
+
+	if (unlikely(rc))
+		return rc;
+	rc = call_int_hook(key_alloc, key, cred, flags);
+	if (unlikely(rc))
+		security_key_free(key);
+	return rc;
 }
 
 /**
@@ -5301,7 +5337,8 @@ int security_key_alloc(struct key *key, const struct cred *cred,
  */
 void security_key_free(struct key *key)
 {
-	call_void_hook(key_free, key);
+	kfree(key->security);
+	key->security = NULL;
 }
 
 /**
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 19346e1817ff..b3de2e941ef7 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6658,11 +6658,7 @@ static int selinux_key_alloc(struct key *k, const struct cred *cred,
 			     unsigned long flags)
 {
 	const struct task_security_struct *tsec;
-	struct key_security_struct *ksec;
-
-	ksec = kzalloc(sizeof(struct key_security_struct), GFP_KERNEL);
-	if (!ksec)
-		return -ENOMEM;
+	struct key_security_struct *ksec = selinux_key(k);
 
 	tsec = selinux_cred(cred);
 	if (tsec->keycreate_sid)
@@ -6670,18 +6666,9 @@ static int selinux_key_alloc(struct key *k, const struct cred *cred,
 	else
 		ksec->sid = tsec->sid;
 
-	k->security = ksec;
 	return 0;
 }
 
-static void selinux_key_free(struct key *k)
-{
-	struct key_security_struct *ksec = k->security;
-
-	k->security = NULL;
-	kfree(ksec);
-}
-
 static int selinux_key_permission(key_ref_t key_ref,
 				  const struct cred *cred,
 				  enum key_need_perm need_perm)
@@ -6722,14 +6709,14 @@ static int selinux_key_permission(key_ref_t key_ref,
 
 	sid = cred_sid(cred);
 	key = key_ref_to_ptr(key_ref);
-	ksec = key->security;
+	ksec = selinux_key(key);
 
 	return avc_has_perm(sid, ksec->sid, SECCLASS_KEY, perm, NULL);
 }
 
 static int selinux_key_getsecurity(struct key *key, char **_buffer)
 {
-	struct key_security_struct *ksec = key->security;
+	struct key_security_struct *ksec = selinux_key(key);
 	char *context = NULL;
 	unsigned len;
 	int rc;
@@ -6981,6 +6968,9 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = {
 	.lbs_file = sizeof(struct file_security_struct),
 	.lbs_inode = sizeof(struct inode_security_struct),
 	.lbs_ipc = sizeof(struct ipc_security_struct),
+#ifdef CONFIG_KEYS
+	.lbs_key = sizeof(struct key_security_struct),
+#endif /* CONFIG_KEYS */
 	.lbs_msg_msg = sizeof(struct msg_security_struct),
 	.lbs_sock = sizeof(struct sk_security_struct),
 	.lbs_superblock = sizeof(struct superblock_security_struct),
@@ -7318,7 +7308,6 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
 #endif
 
 #ifdef CONFIG_KEYS
-	LSM_HOOK_INIT(key_free, selinux_key_free),
 	LSM_HOOK_INIT(key_permission, selinux_key_permission),
 	LSM_HOOK_INIT(key_getsecurity, selinux_key_getsecurity),
 #ifdef CONFIG_KEY_NOTIFICATIONS
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index b074099acbaf..83b9443d6919 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -195,6 +195,13 @@ selinux_superblock(const struct super_block *superblock)
 	return superblock->s_security + selinux_blob_sizes.lbs_superblock;
 }
 
+#ifdef CONFIG_KEYS
+static inline struct key_security_struct *selinux_key(const struct key *key)
+{
+	return key->security + selinux_blob_sizes.lbs_key;
+}
+#endif /* CONFIG_KEYS */
+
 static inline struct sk_security_struct *selinux_sock(const struct sock *sock)
 {
 	return sock->sk_security + selinux_blob_sizes.lbs_sock;
diff --git a/security/smack/smack.h b/security/smack/smack.h
index 297f21446f45..dbf8d7226eb5 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -360,6 +360,13 @@ static inline struct socket_smack *smack_sock(const struct sock *sock)
 	return sock->sk_security + smack_blob_sizes.lbs_sock;
 }
 
+#ifdef CONFIG_KEYS
+static inline struct smack_known **smack_key(const struct key *key)
+{
+	return key->security + smack_blob_sizes.lbs_key;
+}
+#endif /* CONFIG_KEYS */
+
 /*
  * Is the directory transmuting?
  */
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index a931b44bc959..17bcc9cbf584 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4473,23 +4473,13 @@ static void smack_inet_csk_clone(struct sock *sk,
 static int smack_key_alloc(struct key *key, const struct cred *cred,
 			   unsigned long flags)
 {
+	struct smack_known **blob = smack_key(key);
 	struct smack_known *skp = smk_of_task(smack_cred(cred));
 
-	key->security = skp;
+	*blob = skp;
 	return 0;
 }
 
-/**
- * smack_key_free - Clear the key security blob
- * @key: the object
- *
- * Clear the blob pointer
- */
-static void smack_key_free(struct key *key)
-{
-	key->security = NULL;
-}
-
 /**
  * smack_key_permission - Smack access on a key
  * @key_ref: gets to the object
@@ -4503,6 +4493,8 @@ static int smack_key_permission(key_ref_t key_ref,
 				const struct cred *cred,
 				enum key_need_perm need_perm)
 {
+	struct smack_known **blob;
+	struct smack_known *skp;
 	struct key *keyp;
 	struct smk_audit_info ad;
 	struct smack_known *tkp = smk_of_task(smack_cred(cred));
@@ -4540,7 +4532,9 @@ static int smack_key_permission(key_ref_t key_ref,
 	 * If the key hasn't been initialized give it access so that
 	 * it may do so.
 	 */
-	if (keyp->security == NULL)
+	blob = smack_key(keyp);
+	skp = *blob;
+	if (skp == NULL)
 		return 0;
 	/*
 	 * This should not occur
@@ -4556,8 +4550,8 @@ static int smack_key_permission(key_ref_t key_ref,
 	ad.a.u.key_struct.key = keyp->serial;
 	ad.a.u.key_struct.key_desc = keyp->description;
 #endif
-	rc = smk_access(tkp, keyp->security, request, &ad);
-	rc = smk_bu_note("key access", tkp, keyp->security, request, rc);
+	rc = smk_access(tkp, skp, request, &ad);
+	rc = smk_bu_note("key access", tkp, skp, request, rc);
 	return rc;
 }
 
@@ -4572,11 +4566,12 @@ static int smack_key_permission(key_ref_t key_ref,
  */
 static int smack_key_getsecurity(struct key *key, char **_buffer)
 {
-	struct smack_known *skp = key->security;
+	struct smack_known **blob = smack_key(key);
+	struct smack_known *skp = *blob;
 	size_t length;
 	char *copy;
 
-	if (key->security == NULL) {
+	if (skp == NULL) {
 		*_buffer = NULL;
 		return 0;
 	}
@@ -5010,6 +5005,9 @@ struct lsm_blob_sizes smack_blob_sizes __ro_after_init = {
 	.lbs_file = sizeof(struct smack_known *),
 	.lbs_inode = sizeof(struct inode_smack),
 	.lbs_ipc = sizeof(struct smack_known *),
+#ifdef CONFIG_KEYS
+	.lbs_key = sizeof(struct smack_known *),
+#endif /* CONFIG_KEYS */
 	.lbs_msg_msg = sizeof(struct smack_known *),
 	.lbs_sock = sizeof(struct socket_smack),
 	.lbs_superblock = sizeof(struct superblock_smack),
@@ -5146,7 +5144,6 @@ static struct security_hook_list smack_hooks[] __ro_after_init = {
  /* key management security hooks */
 #ifdef CONFIG_KEYS
 	LSM_HOOK_INIT(key_alloc, smack_key_alloc),
-	LSM_HOOK_INIT(key_free, smack_key_free),
 	LSM_HOOK_INIT(key_permission, smack_key_permission),
 	LSM_HOOK_INIT(key_getsecurity, smack_key_getsecurity),
 #ifdef CONFIG_KEY_NOTIFICATIONS
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/6] LSM: Infrastructure management of the key security  blob
  2024-07-08 21:39   ` [PATCH 2/6] LSM: Infrastructure management of the key security blob Casey Schaufler
@ 2024-07-09 22:08     ` Paul Moore
  2024-07-09 22:47       ` John Johansen
  0 siblings, 1 reply; 28+ messages in thread
From: Paul Moore @ 2024-07-09 22:08 UTC (permalink / raw)
  To: Casey Schaufler, casey, linux-security-module
  Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
	stephen.smalley.work, mic

On Jul  8, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote:
> 
> Move management of the key->security blob out of the
> individual security modules and into the security
> infrastructure. Instead of allocating the blobs from within
> the modules the modules tell the infrastructure how much
> space is required, and the space is allocated there.

Perhaps mention that the key_free hook is being removed as it is not
currently needed after this change?

> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>  include/linux/lsm_hooks.h         |  1 +
>  security/security.c               | 41 +++++++++++++++++++++++++++++--
>  security/selinux/hooks.c          | 23 +++++------------
>  security/selinux/include/objsec.h |  7 ++++++
>  security/smack/smack.h            |  7 ++++++
>  security/smack/smack_lsm.c        | 33 +++++++++++--------------
>  6 files changed, 75 insertions(+), 37 deletions(-)

...

> diff --git a/security/security.c b/security/security.c
> index 5e93a72bdca6..aae37481b7be 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -227,6 +227,9 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
>  		blob_sizes.lbs_inode = sizeof(struct rcu_head);
>  	lsm_set_blob_size(&needed->lbs_inode, &blob_sizes.lbs_inode);
>  	lsm_set_blob_size(&needed->lbs_ipc, &blob_sizes.lbs_ipc);
> +#ifdef CONFIG_KEYS
> +	lsm_set_blob_size(&needed->lbs_key, &blob_sizes.lbs_key);
> +#endif

Since the lsm_blob_sizes struct is going to have the lsb_key field
regardless of CONFIG_KEYS (which is good, I'm not arguing that), we
should be okay to call lsm_set_blob_size() on the lsb_key field, right?

>  	lsm_set_blob_size(&needed->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
>  	lsm_set_blob_size(&needed->lbs_sock, &blob_sizes.lbs_sock);
>  	lsm_set_blob_size(&needed->lbs_superblock, &blob_sizes.lbs_superblock);
> @@ -402,6 +405,9 @@ static void __init ordered_lsm_init(void)
>  	init_debug("file blob size       = %d\n", blob_sizes.lbs_file);
>  	init_debug("inode blob size      = %d\n", blob_sizes.lbs_inode);
>  	init_debug("ipc blob size        = %d\n", blob_sizes.lbs_ipc);
> +#ifdef CONFIG_KEYS
> +	init_debug("key blob size        = %d\n", blob_sizes.lbs_key);
> +#endif /* CONFIG_KEYS */

This one makes sense.

> @@ -5301,7 +5337,8 @@ int security_key_alloc(struct key *key, const struct cred *cred,
>   */
>  void security_key_free(struct key *key)
>  {
> -	call_void_hook(key_free, key);
> +	kfree(key->security);
> +	key->security = NULL;
>  }

A note to future devs, we can add the key_free hook back if needed, but
currently nobody is using it beyond basic memory management.

>  /**
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 19346e1817ff..b3de2e941ef7 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6981,6 +6968,9 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = {
>  	.lbs_file = sizeof(struct file_security_struct),
>  	.lbs_inode = sizeof(struct inode_security_struct),
>  	.lbs_ipc = sizeof(struct ipc_security_struct),
> +#ifdef CONFIG_KEYS
> +	.lbs_key = sizeof(struct key_security_struct),
> +#endif /* CONFIG_KEYS */

We can probably get rid of the Kconfig conditional.  I understand the
desire to keep this to only what is needed, but since this only really
has an impact on boot, and the impact is some basic math, I'd rather
not run the risk of rot due to conditional compilation.

>  	.lbs_msg_msg = sizeof(struct msg_security_struct),
>  	.lbs_sock = sizeof(struct sk_security_struct),
>  	.lbs_superblock = sizeof(struct superblock_security_struct),

...

> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index a931b44bc959..17bcc9cbf584 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -5010,6 +5005,9 @@ struct lsm_blob_sizes smack_blob_sizes __ro_after_init = {
>  	.lbs_file = sizeof(struct smack_known *),
>  	.lbs_inode = sizeof(struct inode_smack),
>  	.lbs_ipc = sizeof(struct smack_known *),
> +#ifdef CONFIG_KEYS
> +	.lbs_key = sizeof(struct smack_known *),
> +#endif /* CONFIG_KEYS */

See above, but ultimately this is Smack code so it's your call.

>  	.lbs_msg_msg = sizeof(struct smack_known *),
>  	.lbs_sock = sizeof(struct socket_smack),
>  	.lbs_superblock = sizeof(struct superblock_smack),

--
paul-moore.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/6] LSM: Infrastructure management of the key security blob
  2024-07-09 22:08     ` Paul Moore
@ 2024-07-09 22:47       ` John Johansen
  2024-07-09 23:01         ` Paul Moore
  0 siblings, 1 reply; 28+ messages in thread
From: John Johansen @ 2024-07-09 22:47 UTC (permalink / raw)
  To: Paul Moore, Casey Schaufler, linux-security-module
  Cc: jmorris, serge, keescook, penguin-kernel, stephen.smalley.work,
	mic

On 7/9/24 15:08, Paul Moore wrote:
> On Jul  8, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote:
>>
>> Move management of the key->security blob out of the
>> individual security modules and into the security
>> infrastructure. Instead of allocating the blobs from within
>> the modules the modules tell the infrastructure how much
>> space is required, and the space is allocated there.
> 
> Perhaps mention that the key_free hook is being removed as it is not
> currently needed after this change?
> 
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>>   include/linux/lsm_hooks.h         |  1 +
>>   security/security.c               | 41 +++++++++++++++++++++++++++++--
>>   security/selinux/hooks.c          | 23 +++++------------
>>   security/selinux/include/objsec.h |  7 ++++++
>>   security/smack/smack.h            |  7 ++++++
>>   security/smack/smack_lsm.c        | 33 +++++++++++--------------
>>   6 files changed, 75 insertions(+), 37 deletions(-)
> 
> ...
> 
>> diff --git a/security/security.c b/security/security.c
>> index 5e93a72bdca6..aae37481b7be 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -227,6 +227,9 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
>>   		blob_sizes.lbs_inode = sizeof(struct rcu_head);
>>   	lsm_set_blob_size(&needed->lbs_inode, &blob_sizes.lbs_inode);
>>   	lsm_set_blob_size(&needed->lbs_ipc, &blob_sizes.lbs_ipc);
>> +#ifdef CONFIG_KEYS
>> +	lsm_set_blob_size(&needed->lbs_key, &blob_sizes.lbs_key);
>> +#endif
> 
> Since the lsm_blob_sizes struct is going to have the lsb_key field
> regardless of CONFIG_KEYS (which is good, I'm not arguing that), we
> should be okay to call lsm_set_blob_size() on the lsb_key field, right?

yes

> 
>>   	lsm_set_blob_size(&needed->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
>>   	lsm_set_blob_size(&needed->lbs_sock, &blob_sizes.lbs_sock);
>>   	lsm_set_blob_size(&needed->lbs_superblock, &blob_sizes.lbs_superblock);
>> @@ -402,6 +405,9 @@ static void __init ordered_lsm_init(void)
>>   	init_debug("file blob size       = %d\n", blob_sizes.lbs_file);
>>   	init_debug("inode blob size      = %d\n", blob_sizes.lbs_inode);
>>   	init_debug("ipc blob size        = %d\n", blob_sizes.lbs_ipc);
>> +#ifdef CONFIG_KEYS
>> +	init_debug("key blob size        = %d\n", blob_sizes.lbs_key);
>> +#endif /* CONFIG_KEYS */
> 
> This one makes sense.
> 
>> @@ -5301,7 +5337,8 @@ int security_key_alloc(struct key *key, const struct cred *cred,
>>    */
>>   void security_key_free(struct key *key)
>>   {
>> -	call_void_hook(key_free, key);
>> +	kfree(key->security);
>> +	key->security = NULL;
>>   }
> 
> A note to future devs, we can add the key_free hook back if needed, but
> currently nobody is using it beyond basic memory management.
> 
>>   /**
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 19346e1817ff..b3de2e941ef7 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -6981,6 +6968,9 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = {
>>   	.lbs_file = sizeof(struct file_security_struct),
>>   	.lbs_inode = sizeof(struct inode_security_struct),
>>   	.lbs_ipc = sizeof(struct ipc_security_struct),
>> +#ifdef CONFIG_KEYS
>> +	.lbs_key = sizeof(struct key_security_struct),
>> +#endif /* CONFIG_KEYS */
> 
> We can probably get rid of the Kconfig conditional.  I understand the
> desire to keep this to only what is needed, but since this only really
> has an impact on boot, and the impact is some basic math, I'd rather
> not run the risk of rot due to conditional compilation.
> 

sure, the counter argument is why isn't this conditional when other parts
for keys is. That inconsistency introduces it own issues that can cause
problems down the road. I really don't have an opinion on which way
is better, just playing devils advocate.


>>   	.lbs_msg_msg = sizeof(struct msg_security_struct),
>>   	.lbs_sock = sizeof(struct sk_security_struct),
>>   	.lbs_superblock = sizeof(struct superblock_security_struct),
> 
> ...
> 
>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>> index a931b44bc959..17bcc9cbf584 100644
>> --- a/security/smack/smack_lsm.c
>> +++ b/security/smack/smack_lsm.c
>> @@ -5010,6 +5005,9 @@ struct lsm_blob_sizes smack_blob_sizes __ro_after_init = {
>>   	.lbs_file = sizeof(struct smack_known *),
>>   	.lbs_inode = sizeof(struct inode_smack),
>>   	.lbs_ipc = sizeof(struct smack_known *),
>> +#ifdef CONFIG_KEYS
>> +	.lbs_key = sizeof(struct smack_known *),
>> +#endif /* CONFIG_KEYS */
> 
> See above, but ultimately this is Smack code so it's your call.

I think we should be consistent, either it goes everywhere or it stays
everywhere. It is going to be confusing for other people coming in and
looking at the code as to why one is conditional and one isn't.

> 
>>   	.lbs_msg_msg = sizeof(struct smack_known *),
>>   	.lbs_sock = sizeof(struct socket_smack),
>>   	.lbs_superblock = sizeof(struct superblock_smack),
> 
> --
> paul-moore.com


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/6] LSM: Infrastructure management of the key security blob
  2024-07-09 22:47       ` John Johansen
@ 2024-07-09 23:01         ` Paul Moore
  0 siblings, 0 replies; 28+ messages in thread
From: Paul Moore @ 2024-07-09 23:01 UTC (permalink / raw)
  To: John Johansen
  Cc: Casey Schaufler, linux-security-module, jmorris, serge, keescook,
	penguin-kernel, stephen.smalley.work, mic

On Tue, Jul 9, 2024 at 6:47 PM John Johansen
<john.johansen@canonical.com> wrote:
> On 7/9/24 15:08, Paul Moore wrote:
> > On Jul  8, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote:
> >>
> >> Move management of the key->security blob out of the
> >> individual security modules and into the security
> >> infrastructure. Instead of allocating the blobs from within
> >> the modules the modules tell the infrastructure how much
> >> space is required, and the space is allocated there.
> >
> > Perhaps mention that the key_free hook is being removed as it is not
> > currently needed after this change?
> >
> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> >> ---
> >>   include/linux/lsm_hooks.h         |  1 +
> >>   security/security.c               | 41 +++++++++++++++++++++++++++++--
> >>   security/selinux/hooks.c          | 23 +++++------------
> >>   security/selinux/include/objsec.h |  7 ++++++
> >>   security/smack/smack.h            |  7 ++++++
> >>   security/smack/smack_lsm.c        | 33 +++++++++++--------------
> >>   6 files changed, 75 insertions(+), 37 deletions(-)

...

> >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> >> index 19346e1817ff..b3de2e941ef7 100644
> >> --- a/security/selinux/hooks.c
> >> +++ b/security/selinux/hooks.c
> >> @@ -6981,6 +6968,9 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = {
> >>      .lbs_file = sizeof(struct file_security_struct),
> >>      .lbs_inode = sizeof(struct inode_security_struct),
> >>      .lbs_ipc = sizeof(struct ipc_security_struct),
> >> +#ifdef CONFIG_KEYS
> >> +    .lbs_key = sizeof(struct key_security_struct),
> >> +#endif /* CONFIG_KEYS */
> >
> > We can probably get rid of the Kconfig conditional.  I understand the
> > desire to keep this to only what is needed, but since this only really
> > has an impact on boot, and the impact is some basic math, I'd rather
> > not run the risk of rot due to conditional compilation.
>
> sure, the counter argument is why isn't this conditional when other parts
> for keys is. That inconsistency introduces it own issues that can cause
> problems down the road. I really don't have an opinion on which way
> is better, just playing devils advocate.

That's fair, since it's Casey's patch I guess he can be the tie break.
While I generally prefer to limit conditional code blocks, I'm not
overly concerned about this one.

> >>      .lbs_msg_msg = sizeof(struct msg_security_struct),
> >>      .lbs_sock = sizeof(struct sk_security_struct),
> >>      .lbs_superblock = sizeof(struct superblock_security_struct),
> >
> > ...
> >
> >> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> >> index a931b44bc959..17bcc9cbf584 100644
> >> --- a/security/smack/smack_lsm.c
> >> +++ b/security/smack/smack_lsm.c
> >> @@ -5010,6 +5005,9 @@ struct lsm_blob_sizes smack_blob_sizes __ro_after_init = {
> >>      .lbs_file = sizeof(struct smack_known *),
> >>      .lbs_inode = sizeof(struct inode_smack),
> >>      .lbs_ipc = sizeof(struct smack_known *),
> >> +#ifdef CONFIG_KEYS
> >> +    .lbs_key = sizeof(struct smack_known *),
> >> +#endif /* CONFIG_KEYS */
> >
> > See above, but ultimately this is Smack code so it's your call.
>
> I think we should be consistent, either it goes everywhere or it stays
> everywhere. It is going to be confusing for other people coming in and
> looking at the code as to why one is conditional and one isn't.

I agree with everything you've said, but I personally don't want to be
in the business of demanding things from the individual LSMs so long
as they aren't hurting anything else.  Hopefully Casey will go for
consistency ;)

-- 
paul-moore.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 3/6] LSM: Add helper for blob allocations
  2024-07-08 21:39 ` [PATCH 0/6] LSM: Infrastructure blob allocation Casey Schaufler
  2024-07-08 21:39   ` [PATCH 1/6] LSM: Infrastructure management of the sock security Casey Schaufler
  2024-07-08 21:39   ` [PATCH 2/6] LSM: Infrastructure management of the key security blob Casey Schaufler
@ 2024-07-08 21:39   ` Casey Schaufler
  2024-07-09 22:08     ` Paul Moore
  2024-07-09 22:51     ` John Johansen
  2024-07-08 21:39   ` [PATCH 4/6] LSM: Infrastructure management of the dev_tun blob Casey Schaufler
                     ` (2 subsequent siblings)
  5 siblings, 2 replies; 28+ messages in thread
From: Casey Schaufler @ 2024-07-08 21:39 UTC (permalink / raw)
  To: casey, paul, linux-security-module
  Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
	stephen.smalley.work, mic

Create a helper function lsm_blob_alloc() for general use in the hook
specific functions that allocate LSM blobs. Change the hook specific
functions to use this helper. This reduces the code size by a small
amount and will make adding new instances of infrastructure managed
security blobs easier.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/security.c | 97 +++++++++++++++------------------------------
 1 file changed, 33 insertions(+), 64 deletions(-)

diff --git a/security/security.c b/security/security.c
index aae37481b7be..438ec6708eb3 100644
--- a/security/security.c
+++ b/security/security.c
@@ -605,27 +605,42 @@ int unregister_blocking_lsm_notifier(struct notifier_block *nb)
 EXPORT_SYMBOL(unregister_blocking_lsm_notifier);
 
 /**
- * lsm_cred_alloc - allocate a composite cred blob
- * @cred: the cred that needs a blob
+ * lsm_blob_alloc - allocate a composite blob
+ * @dest: the destination for the blob
+ * @size: the size of the blob
  * @gfp: allocation type
  *
- * Allocate the cred blob for all the modules
+ * Allocate a blob for all the modules
  *
  * Returns 0, or -ENOMEM if memory can't be allocated.
  */
-static int lsm_cred_alloc(struct cred *cred, gfp_t gfp)
+static int lsm_blob_alloc(void **dest, size_t size, gfp_t gfp)
 {
-	if (blob_sizes.lbs_cred == 0) {
-		cred->security = NULL;
+	if (size == 0) {
+		*dest = NULL;
 		return 0;
 	}
 
-	cred->security = kzalloc(blob_sizes.lbs_cred, gfp);
-	if (cred->security == NULL)
+	*dest = kzalloc(size, gfp);
+	if (*dest == NULL)
 		return -ENOMEM;
 	return 0;
 }
 
+/**
+ * lsm_cred_alloc - allocate a composite cred blob
+ * @cred: the cred that needs a blob
+ * @gfp: allocation type
+ *
+ * Allocate the cred blob for all the modules
+ *
+ * Returns 0, or -ENOMEM if memory can't be allocated.
+ */
+static int lsm_cred_alloc(struct cred *cred, gfp_t gfp)
+{
+	return lsm_blob_alloc(&cred->security, blob_sizes.lbs_cred, gfp);
+}
+
 /**
  * lsm_early_cred - during initialization allocate a composite cred blob
  * @cred: the cred that needs a blob
@@ -692,15 +707,7 @@ int lsm_inode_alloc(struct inode *inode)
  */
 static int lsm_task_alloc(struct task_struct *task)
 {
-	if (blob_sizes.lbs_task == 0) {
-		task->security = NULL;
-		return 0;
-	}
-
-	task->security = kzalloc(blob_sizes.lbs_task, GFP_KERNEL);
-	if (task->security == NULL)
-		return -ENOMEM;
-	return 0;
+	return lsm_blob_alloc(&task->security, blob_sizes.lbs_task, GFP_KERNEL);
 }
 
 /**
@@ -713,15 +720,7 @@ static int lsm_task_alloc(struct task_struct *task)
  */
 static int lsm_ipc_alloc(struct kern_ipc_perm *kip)
 {
-	if (blob_sizes.lbs_ipc == 0) {
-		kip->security = NULL;
-		return 0;
-	}
-
-	kip->security = kzalloc(blob_sizes.lbs_ipc, GFP_KERNEL);
-	if (kip->security == NULL)
-		return -ENOMEM;
-	return 0;
+	return lsm_blob_alloc(&kip->security, blob_sizes.lbs_ipc, GFP_KERNEL);
 }
 
 #ifdef CONFIG_KEYS
@@ -735,15 +734,7 @@ static int lsm_ipc_alloc(struct kern_ipc_perm *kip)
  */
 static int lsm_key_alloc(struct key *key)
 {
-	if (blob_sizes.lbs_key == 0) {
-		key->security = NULL;
-		return 0;
-	}
-
-	key->security = kzalloc(blob_sizes.lbs_key, GFP_KERNEL);
-	if (key->security == NULL)
-		return -ENOMEM;
-	return 0;
+	return lsm_blob_alloc(&key->security, blob_sizes.lbs_key, GFP_KERNEL);
 }
 #endif /* CONFIG_KEYS */
 
@@ -757,15 +748,8 @@ static int lsm_key_alloc(struct key *key)
  */
 static int lsm_msg_msg_alloc(struct msg_msg *mp)
 {
-	if (blob_sizes.lbs_msg_msg == 0) {
-		mp->security = NULL;
-		return 0;
-	}
-
-	mp->security = kzalloc(blob_sizes.lbs_msg_msg, GFP_KERNEL);
-	if (mp->security == NULL)
-		return -ENOMEM;
-	return 0;
+	return lsm_blob_alloc(&mp->security, blob_sizes.lbs_msg_msg,
+			      GFP_KERNEL);
 }
 
 /**
@@ -792,15 +776,8 @@ static void __init lsm_early_task(struct task_struct *task)
  */
 static int lsm_superblock_alloc(struct super_block *sb)
 {
-	if (blob_sizes.lbs_superblock == 0) {
-		sb->s_security = NULL;
-		return 0;
-	}
-
-	sb->s_security = kzalloc(blob_sizes.lbs_superblock, GFP_KERNEL);
-	if (sb->s_security == NULL)
-		return -ENOMEM;
-	return 0;
+	return lsm_blob_alloc(&sb->s_security, blob_sizes.lbs_superblock,
+			      GFP_KERNEL);
 }
 
 /**
@@ -4682,23 +4659,15 @@ EXPORT_SYMBOL(security_socket_getpeersec_dgram);
 /**
  * lsm_sock_alloc - allocate a composite sock blob
  * @sock: the sock that needs a blob
- * @priority: allocation mode
+ * @gfp: allocation mode
  *
  * Allocate the sock blob for all the modules
  *
  * Returns 0, or -ENOMEM if memory can't be allocated.
  */
-static int lsm_sock_alloc(struct sock *sock, gfp_t priority)
+static int lsm_sock_alloc(struct sock *sock, gfp_t gfp)
 {
-	if (blob_sizes.lbs_sock == 0) {
-		sock->sk_security = NULL;
-		return 0;
-	}
-
-	sock->sk_security = kzalloc(blob_sizes.lbs_sock, priority);
-	if (sock->sk_security == NULL)
-		return -ENOMEM;
-	return 0;
+	return lsm_blob_alloc(&sock->sk_security, blob_sizes.lbs_sock, gfp);
 }
 
 /**
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/6] LSM: Add helper for blob allocations
  2024-07-08 21:39   ` [PATCH 3/6] LSM: Add helper for blob allocations Casey Schaufler
@ 2024-07-09 22:08     ` Paul Moore
  2024-07-09 23:09       ` Casey Schaufler
  2024-07-09 22:51     ` John Johansen
  1 sibling, 1 reply; 28+ messages in thread
From: Paul Moore @ 2024-07-09 22:08 UTC (permalink / raw)
  To: Casey Schaufler, casey, linux-security-module
  Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
	stephen.smalley.work, mic

On Jul  8, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote:
> 
> Create a helper function lsm_blob_alloc() for general use in the hook
> specific functions that allocate LSM blobs. Change the hook specific
> functions to use this helper. This reduces the code size by a small
> amount and will make adding new instances of infrastructure managed
> security blobs easier.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>  security/security.c | 97 +++++++++++++++------------------------------
>  1 file changed, 33 insertions(+), 64 deletions(-)

Looks good to me, but as it is dependent on 2/6 I'm unable to apply
it anywhere right now.

--
paul-moore.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/6] LSM: Add helper for blob allocations
  2024-07-09 22:08     ` Paul Moore
@ 2024-07-09 23:09       ` Casey Schaufler
  2024-07-10  0:01         ` Paul Moore
  0 siblings, 1 reply; 28+ messages in thread
From: Casey Schaufler @ 2024-07-09 23:09 UTC (permalink / raw)
  To: Paul Moore, linux-security-module
  Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
	stephen.smalley.work, mic, Casey Schaufler

On 7/9/2024 3:08 PM, Paul Moore wrote:
> On Jul  8, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote:
>> Create a helper function lsm_blob_alloc() for general use in the hook
>> specific functions that allocate LSM blobs. Change the hook specific
>> functions to use this helper. This reduces the code size by a small
>> amount and will make adding new instances of infrastructure managed
>> security blobs easier.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>>  security/security.c | 97 +++++++++++++++------------------------------
>>  1 file changed, 33 insertions(+), 64 deletions(-)
> Looks good to me, but as it is dependent on 2/6 I'm unable to apply
> it anywhere right now.

I can reorder 2/6 and 3/6 if that would help.

>
> --
> paul-moore.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/6] LSM: Add helper for blob allocations
  2024-07-09 23:09       ` Casey Schaufler
@ 2024-07-10  0:01         ` Paul Moore
  0 siblings, 0 replies; 28+ messages in thread
From: Paul Moore @ 2024-07-10  0:01 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: linux-security-module, jmorris, serge, keescook, john.johansen,
	penguin-kernel, stephen.smalley.work, mic

On Tue, Jul 9, 2024 at 7:09 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 7/9/2024 3:08 PM, Paul Moore wrote:
> > On Jul  8, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> Create a helper function lsm_blob_alloc() for general use in the hook
> >> specific functions that allocate LSM blobs. Change the hook specific
> >> functions to use this helper. This reduces the code size by a small
> >> amount and will make adding new instances of infrastructure managed
> >> security blobs easier.
> >>
> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> >> ---
> >>  security/security.c | 97 +++++++++++++++------------------------------
> >>  1 file changed, 33 insertions(+), 64 deletions(-)
> > Looks good to me, but as it is dependent on 2/6 I'm unable to apply
> > it anywhere right now.
>
> I can reorder 2/6 and 3/6 if that would help.

No ordering is fine, it was simply a matter of I didn't merge 2/6 so I
couldn't merge 3/6.

-- 
paul-moore.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/6] LSM: Add helper for blob allocations
  2024-07-08 21:39   ` [PATCH 3/6] LSM: Add helper for blob allocations Casey Schaufler
  2024-07-09 22:08     ` Paul Moore
@ 2024-07-09 22:51     ` John Johansen
  1 sibling, 0 replies; 28+ messages in thread
From: John Johansen @ 2024-07-09 22:51 UTC (permalink / raw)
  To: Casey Schaufler, paul, linux-security-module
  Cc: jmorris, serge, keescook, penguin-kernel, stephen.smalley.work,
	mic

On 7/8/24 14:39, Casey Schaufler wrote:
> Create a helper function lsm_blob_alloc() for general use in the hook
> specific functions that allocate LSM blobs. Change the hook specific
> functions to use this helper. This reduces the code size by a small
> amount and will make adding new instances of infrastructure managed
> security blobs easier.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

looks good

Reviewed-by: John Johansen <john.johansen@canonical.com>

> ---
>   security/security.c | 97 +++++++++++++++------------------------------
>   1 file changed, 33 insertions(+), 64 deletions(-)
> 
> diff --git a/security/security.c b/security/security.c
> index aae37481b7be..438ec6708eb3 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -605,27 +605,42 @@ int unregister_blocking_lsm_notifier(struct notifier_block *nb)
>   EXPORT_SYMBOL(unregister_blocking_lsm_notifier);
>   
>   /**
> - * lsm_cred_alloc - allocate a composite cred blob
> - * @cred: the cred that needs a blob
> + * lsm_blob_alloc - allocate a composite blob
> + * @dest: the destination for the blob
> + * @size: the size of the blob
>    * @gfp: allocation type
>    *
> - * Allocate the cred blob for all the modules
> + * Allocate a blob for all the modules
>    *
>    * Returns 0, or -ENOMEM if memory can't be allocated.
>    */
> -static int lsm_cred_alloc(struct cred *cred, gfp_t gfp)
> +static int lsm_blob_alloc(void **dest, size_t size, gfp_t gfp)
>   {
> -	if (blob_sizes.lbs_cred == 0) {
> -		cred->security = NULL;
> +	if (size == 0) {
> +		*dest = NULL;
>   		return 0;
>   	}
>   
> -	cred->security = kzalloc(blob_sizes.lbs_cred, gfp);
> -	if (cred->security == NULL)
> +	*dest = kzalloc(size, gfp);
> +	if (*dest == NULL)
>   		return -ENOMEM;
>   	return 0;
>   }
>   
> +/**
> + * lsm_cred_alloc - allocate a composite cred blob
> + * @cred: the cred that needs a blob
> + * @gfp: allocation type
> + *
> + * Allocate the cred blob for all the modules
> + *
> + * Returns 0, or -ENOMEM if memory can't be allocated.
> + */
> +static int lsm_cred_alloc(struct cred *cred, gfp_t gfp)
> +{
> +	return lsm_blob_alloc(&cred->security, blob_sizes.lbs_cred, gfp);
> +}
> +
>   /**
>    * lsm_early_cred - during initialization allocate a composite cred blob
>    * @cred: the cred that needs a blob
> @@ -692,15 +707,7 @@ int lsm_inode_alloc(struct inode *inode)
>    */
>   static int lsm_task_alloc(struct task_struct *task)
>   {
> -	if (blob_sizes.lbs_task == 0) {
> -		task->security = NULL;
> -		return 0;
> -	}
> -
> -	task->security = kzalloc(blob_sizes.lbs_task, GFP_KERNEL);
> -	if (task->security == NULL)
> -		return -ENOMEM;
> -	return 0;
> +	return lsm_blob_alloc(&task->security, blob_sizes.lbs_task, GFP_KERNEL);
>   }
>   
>   /**
> @@ -713,15 +720,7 @@ static int lsm_task_alloc(struct task_struct *task)
>    */
>   static int lsm_ipc_alloc(struct kern_ipc_perm *kip)
>   {
> -	if (blob_sizes.lbs_ipc == 0) {
> -		kip->security = NULL;
> -		return 0;
> -	}
> -
> -	kip->security = kzalloc(blob_sizes.lbs_ipc, GFP_KERNEL);
> -	if (kip->security == NULL)
> -		return -ENOMEM;
> -	return 0;
> +	return lsm_blob_alloc(&kip->security, blob_sizes.lbs_ipc, GFP_KERNEL);
>   }
>   
>   #ifdef CONFIG_KEYS
> @@ -735,15 +734,7 @@ static int lsm_ipc_alloc(struct kern_ipc_perm *kip)
>    */
>   static int lsm_key_alloc(struct key *key)
>   {
> -	if (blob_sizes.lbs_key == 0) {
> -		key->security = NULL;
> -		return 0;
> -	}
> -
> -	key->security = kzalloc(blob_sizes.lbs_key, GFP_KERNEL);
> -	if (key->security == NULL)
> -		return -ENOMEM;
> -	return 0;
> +	return lsm_blob_alloc(&key->security, blob_sizes.lbs_key, GFP_KERNEL);
>   }
>   #endif /* CONFIG_KEYS */
>   
> @@ -757,15 +748,8 @@ static int lsm_key_alloc(struct key *key)
>    */
>   static int lsm_msg_msg_alloc(struct msg_msg *mp)
>   {
> -	if (blob_sizes.lbs_msg_msg == 0) {
> -		mp->security = NULL;
> -		return 0;
> -	}
> -
> -	mp->security = kzalloc(blob_sizes.lbs_msg_msg, GFP_KERNEL);
> -	if (mp->security == NULL)
> -		return -ENOMEM;
> -	return 0;
> +	return lsm_blob_alloc(&mp->security, blob_sizes.lbs_msg_msg,
> +			      GFP_KERNEL);
>   }
>   
>   /**
> @@ -792,15 +776,8 @@ static void __init lsm_early_task(struct task_struct *task)
>    */
>   static int lsm_superblock_alloc(struct super_block *sb)
>   {
> -	if (blob_sizes.lbs_superblock == 0) {
> -		sb->s_security = NULL;
> -		return 0;
> -	}
> -
> -	sb->s_security = kzalloc(blob_sizes.lbs_superblock, GFP_KERNEL);
> -	if (sb->s_security == NULL)
> -		return -ENOMEM;
> -	return 0;
> +	return lsm_blob_alloc(&sb->s_security, blob_sizes.lbs_superblock,
> +			      GFP_KERNEL);
>   }
>   
>   /**
> @@ -4682,23 +4659,15 @@ EXPORT_SYMBOL(security_socket_getpeersec_dgram);
>   /**
>    * lsm_sock_alloc - allocate a composite sock blob
>    * @sock: the sock that needs a blob
> - * @priority: allocation mode
> + * @gfp: allocation mode
>    *
>    * Allocate the sock blob for all the modules
>    *
>    * Returns 0, or -ENOMEM if memory can't be allocated.
>    */
> -static int lsm_sock_alloc(struct sock *sock, gfp_t priority)
> +static int lsm_sock_alloc(struct sock *sock, gfp_t gfp)
>   {
> -	if (blob_sizes.lbs_sock == 0) {
> -		sock->sk_security = NULL;
> -		return 0;
> -	}
> -
> -	sock->sk_security = kzalloc(blob_sizes.lbs_sock, priority);
> -	if (sock->sk_security == NULL)
> -		return -ENOMEM;
> -	return 0;
> +	return lsm_blob_alloc(&sock->sk_security, blob_sizes.lbs_sock, gfp);
>   }
>   
>   /**


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 4/6] LSM: Infrastructure management of the dev_tun blob
  2024-07-08 21:39 ` [PATCH 0/6] LSM: Infrastructure blob allocation Casey Schaufler
                     ` (2 preceding siblings ...)
  2024-07-08 21:39   ` [PATCH 3/6] LSM: Add helper for blob allocations Casey Schaufler
@ 2024-07-08 21:39   ` Casey Schaufler
  2024-07-09 22:08     ` Paul Moore
  2024-07-09 23:01     ` John Johansen
  2024-07-08 21:39   ` [PATCH 5/6] LSM: Infrastructure management of the infiniband blob Casey Schaufler
  2024-07-08 21:39   ` [PATCH 6/6] LSM: Infrastructure management of the perf_event security blob Casey Schaufler
  5 siblings, 2 replies; 28+ messages in thread
From: Casey Schaufler @ 2024-07-08 21:39 UTC (permalink / raw)
  To: casey, paul, linux-security-module
  Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
	stephen.smalley.work, mic

Move management of the dev_tun security blob out of the individual
security modules and into the LSM infrastructure.  The security modules
tell the infrastructure how much space they require at initialization.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 include/linux/lsm_hook_defs.h     |  2 +-
 include/linux/lsm_hooks.h         |  1 +
 security/security.c               | 11 ++++++++++-
 security/selinux/hooks.c          | 22 ++++++----------------
 security/selinux/include/objsec.h |  6 ++++++
 5 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 44488b1ab9a9..601e3092a7ed 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -352,7 +352,7 @@ LSM_HOOK(void, LSM_RET_VOID, secmark_refcount_inc, void)
 LSM_HOOK(void, LSM_RET_VOID, secmark_refcount_dec, void)
 LSM_HOOK(void, LSM_RET_VOID, req_classify_flow, const struct request_sock *req,
 	 struct flowi_common *flic)
-LSM_HOOK(int, 0, tun_dev_alloc_security, void **security)
+LSM_HOOK(int, 0, tun_dev_alloc_security, void *security)
 LSM_HOOK(void, LSM_RET_VOID, tun_dev_free_security, void *security)
 LSM_HOOK(int, 0, tun_dev_create, void)
 LSM_HOOK(int, 0, tun_dev_attach_queue, void *security)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 7233bc0737be..0ff14ff128c8 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -80,6 +80,7 @@ struct lsm_blob_sizes {
 	int	lbs_msg_msg;
 	int	lbs_task;
 	int	lbs_xattr_count; /* number of xattr slots in new_xattrs array */
+	int	lbs_tun_dev;
 };
 
 /**
diff --git a/security/security.c b/security/security.c
index 438ec6708eb3..2c9d075f5f92 100644
--- a/security/security.c
+++ b/security/security.c
@@ -234,6 +234,7 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
 	lsm_set_blob_size(&needed->lbs_sock, &blob_sizes.lbs_sock);
 	lsm_set_blob_size(&needed->lbs_superblock, &blob_sizes.lbs_superblock);
 	lsm_set_blob_size(&needed->lbs_task, &blob_sizes.lbs_task);
+	lsm_set_blob_size(&needed->lbs_tun_dev, &blob_sizes.lbs_tun_dev);
 	lsm_set_blob_size(&needed->lbs_xattr_count,
 			  &blob_sizes.lbs_xattr_count);
 }
@@ -412,6 +413,7 @@ static void __init ordered_lsm_init(void)
 	init_debug("sock blob size       = %d\n", blob_sizes.lbs_sock);
 	init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
 	init_debug("task blob size       = %d\n", blob_sizes.lbs_task);
+	init_debug("tun device blob size = %d\n", blob_sizes.lbs_tun_dev);
 	init_debug("xattr slots          = %d\n", blob_sizes.lbs_xattr_count);
 
 	/*
@@ -4851,7 +4853,13 @@ EXPORT_SYMBOL(security_secmark_refcount_dec);
  */
 int security_tun_dev_alloc_security(void **security)
 {
-	return call_int_hook(tun_dev_alloc_security, security);
+	int rc;
+
+	rc = lsm_blob_alloc(security, blob_sizes.lbs_tun_dev, GFP_KERNEL);
+	if (rc)
+		return rc;
+
+	return call_int_hook(tun_dev_alloc_security, *security);
 }
 EXPORT_SYMBOL(security_tun_dev_alloc_security);
 
@@ -4864,6 +4872,7 @@ EXPORT_SYMBOL(security_tun_dev_alloc_security);
 void security_tun_dev_free_security(void *security)
 {
 	call_void_hook(tun_dev_free_security, security);
+	kfree(security);
 }
 EXPORT_SYMBOL(security_tun_dev_free_security);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index b3de2e941ef7..7f1d8358922a 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5573,24 +5573,14 @@ static void selinux_req_classify_flow(const struct request_sock *req,
 	flic->flowic_secid = req->secid;
 }
 
-static int selinux_tun_dev_alloc_security(void **security)
+static int selinux_tun_dev_alloc_security(void *security)
 {
-	struct tun_security_struct *tunsec;
+	struct tun_security_struct *tunsec = selinux_tun_dev(security);
 
-	tunsec = kzalloc(sizeof(*tunsec), GFP_KERNEL);
-	if (!tunsec)
-		return -ENOMEM;
 	tunsec->sid = current_sid();
-
-	*security = tunsec;
 	return 0;
 }
 
-static void selinux_tun_dev_free_security(void *security)
-{
-	kfree(security);
-}
-
 static int selinux_tun_dev_create(void)
 {
 	u32 sid = current_sid();
@@ -5608,7 +5598,7 @@ static int selinux_tun_dev_create(void)
 
 static int selinux_tun_dev_attach_queue(void *security)
 {
-	struct tun_security_struct *tunsec = security;
+	struct tun_security_struct *tunsec = selinux_tun_dev(security);
 
 	return avc_has_perm(current_sid(), tunsec->sid, SECCLASS_TUN_SOCKET,
 			    TUN_SOCKET__ATTACH_QUEUE, NULL);
@@ -5616,7 +5606,7 @@ static int selinux_tun_dev_attach_queue(void *security)
 
 static int selinux_tun_dev_attach(struct sock *sk, void *security)
 {
-	struct tun_security_struct *tunsec = security;
+	struct tun_security_struct *tunsec = selinux_tun_dev(security);
 	struct sk_security_struct *sksec = selinux_sock(sk);
 
 	/* we don't currently perform any NetLabel based labeling here and it
@@ -5634,7 +5624,7 @@ static int selinux_tun_dev_attach(struct sock *sk, void *security)
 
 static int selinux_tun_dev_open(void *security)
 {
-	struct tun_security_struct *tunsec = security;
+	struct tun_security_struct *tunsec = selinux_tun_dev(security);
 	u32 sid = current_sid();
 	int err;
 
@@ -6975,6 +6965,7 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = {
 	.lbs_sock = sizeof(struct sk_security_struct),
 	.lbs_superblock = sizeof(struct superblock_security_struct),
 	.lbs_xattr_count = SELINUX_INODE_INIT_XATTRS,
+	.lbs_tun_dev = sizeof(struct tun_security_struct),
 };
 
 #ifdef CONFIG_PERF_EVENTS
@@ -7285,7 +7276,6 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(secmark_refcount_inc, selinux_secmark_refcount_inc),
 	LSM_HOOK_INIT(secmark_refcount_dec, selinux_secmark_refcount_dec),
 	LSM_HOOK_INIT(req_classify_flow, selinux_req_classify_flow),
-	LSM_HOOK_INIT(tun_dev_free_security, selinux_tun_dev_free_security),
 	LSM_HOOK_INIT(tun_dev_create, selinux_tun_dev_create),
 	LSM_HOOK_INIT(tun_dev_attach_queue, selinux_tun_dev_attach_queue),
 	LSM_HOOK_INIT(tun_dev_attach, selinux_tun_dev_attach),
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index 83b9443d6919..461c6985977d 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -207,4 +207,10 @@ static inline struct sk_security_struct *selinux_sock(const struct sock *sock)
 	return sock->sk_security + selinux_blob_sizes.lbs_sock;
 }
 
+static inline struct tun_security_struct *
+selinux_tun_dev(void *security)
+{
+	return security + selinux_blob_sizes.lbs_tun_dev;
+}
+
 #endif /* _SELINUX_OBJSEC_H_ */
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH 4/6] LSM: Infrastructure management of the dev_tun blob
  2024-07-08 21:39   ` [PATCH 4/6] LSM: Infrastructure management of the dev_tun blob Casey Schaufler
@ 2024-07-09 22:08     ` Paul Moore
  2024-07-09 23:01     ` John Johansen
  1 sibling, 0 replies; 28+ messages in thread
From: Paul Moore @ 2024-07-09 22:08 UTC (permalink / raw)
  To: Casey Schaufler, casey, linux-security-module
  Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
	stephen.smalley.work, mic

On Jul  8, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote:
> 
> Move management of the dev_tun security blob out of the individual
> security modules and into the LSM infrastructure.  The security modules
> tell the infrastructure how much space they require at initialization.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>  include/linux/lsm_hook_defs.h     |  2 +-
>  include/linux/lsm_hooks.h         |  1 +
>  security/security.c               | 11 ++++++++++-
>  security/selinux/hooks.c          | 22 ++++++----------------
>  security/selinux/include/objsec.h |  6 ++++++
>  5 files changed, 24 insertions(+), 18 deletions(-)

...

> diff --git a/security/security.c b/security/security.c
> index 438ec6708eb3..2c9d075f5f92 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -4864,6 +4872,7 @@ EXPORT_SYMBOL(security_tun_dev_alloc_security);
>  void security_tun_dev_free_security(void *security)
>  {
>  	call_void_hook(tun_dev_free_security, security);
> +	kfree(security);
>  }
>  EXPORT_SYMBOL(security_tun_dev_free_security);

If we removed key_free in patch 2/6, we should probably do the same
here for the same reasons.  Don't forget to mention it in the commit
description.

--
paul-moore.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 4/6] LSM: Infrastructure management of the dev_tun blob
  2024-07-08 21:39   ` [PATCH 4/6] LSM: Infrastructure management of the dev_tun blob Casey Schaufler
  2024-07-09 22:08     ` Paul Moore
@ 2024-07-09 23:01     ` John Johansen
  2024-07-09 23:11       ` Casey Schaufler
  1 sibling, 1 reply; 28+ messages in thread
From: John Johansen @ 2024-07-09 23:01 UTC (permalink / raw)
  To: Casey Schaufler, paul, linux-security-module
  Cc: jmorris, serge, keescook, penguin-kernel, stephen.smalley.work,
	mic

On 7/8/24 14:39, Casey Schaufler wrote:
> Move management of the dev_tun security blob out of the individual
> security modules and into the LSM infrastructure.  The security modules
> tell the infrastructure how much space they require at initialization.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

one issue below, besides Paul's point the rest of it looks good

> ---
>   include/linux/lsm_hook_defs.h     |  2 +-
>   include/linux/lsm_hooks.h         |  1 +
>   security/security.c               | 11 ++++++++++-
>   security/selinux/hooks.c          | 22 ++++++----------------
>   security/selinux/include/objsec.h |  6 ++++++
>   5 files changed, 24 insertions(+), 18 deletions(-)
> 
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 44488b1ab9a9..601e3092a7ed 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -352,7 +352,7 @@ LSM_HOOK(void, LSM_RET_VOID, secmark_refcount_inc, void)
>   LSM_HOOK(void, LSM_RET_VOID, secmark_refcount_dec, void)
>   LSM_HOOK(void, LSM_RET_VOID, req_classify_flow, const struct request_sock *req,
>   	 struct flowi_common *flic)
> -LSM_HOOK(int, 0, tun_dev_alloc_security, void **security)
> +LSM_HOOK(int, 0, tun_dev_alloc_security, void *security)
>   LSM_HOOK(void, LSM_RET_VOID, tun_dev_free_security, void *security)
>   LSM_HOOK(int, 0, tun_dev_create, void)
>   LSM_HOOK(int, 0, tun_dev_attach_queue, void *security)
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 7233bc0737be..0ff14ff128c8 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -80,6 +80,7 @@ struct lsm_blob_sizes {
>   	int	lbs_msg_msg;
>   	int	lbs_task;
>   	int	lbs_xattr_count; /* number of xattr slots in new_xattrs array */
> +	int	lbs_tun_dev;
>   };
>   
>   /**
> diff --git a/security/security.c b/security/security.c
> index 438ec6708eb3..2c9d075f5f92 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -234,6 +234,7 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
>   	lsm_set_blob_size(&needed->lbs_sock, &blob_sizes.lbs_sock);
>   	lsm_set_blob_size(&needed->lbs_superblock, &blob_sizes.lbs_superblock);
>   	lsm_set_blob_size(&needed->lbs_task, &blob_sizes.lbs_task);
> +	lsm_set_blob_size(&needed->lbs_tun_dev, &blob_sizes.lbs_tun_dev);
>   	lsm_set_blob_size(&needed->lbs_xattr_count,
>   			  &blob_sizes.lbs_xattr_count);
>   }
> @@ -412,6 +413,7 @@ static void __init ordered_lsm_init(void)
>   	init_debug("sock blob size       = %d\n", blob_sizes.lbs_sock);
>   	init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
>   	init_debug("task blob size       = %d\n", blob_sizes.lbs_task);
> +	init_debug("tun device blob size = %d\n", blob_sizes.lbs_tun_dev);
>   	init_debug("xattr slots          = %d\n", blob_sizes.lbs_xattr_count);
>   
>   	/*
> @@ -4851,7 +4853,13 @@ EXPORT_SYMBOL(security_secmark_refcount_dec);
>    */
>   int security_tun_dev_alloc_security(void **security)
>   {
> -	return call_int_hook(tun_dev_alloc_security, security);
> +	int rc;
> +
> +	rc = lsm_blob_alloc(security, blob_sizes.lbs_tun_dev, GFP_KERNEL);
> +	if (rc)
> +		return rc;
> +
> +	return call_int_hook(tun_dev_alloc_security, *security);

we either need to free the blob here or make a change to tun_net_init.

if security_tun_dev_alloc fails it is not calling the corresponding
security_tun_dev_free_security()

>   }
>   EXPORT_SYMBOL(security_tun_dev_alloc_security);
>   
> @@ -4864,6 +4872,7 @@ EXPORT_SYMBOL(security_tun_dev_alloc_security);
>   void security_tun_dev_free_security(void *security)
>   {
>   	call_void_hook(tun_dev_free_security, security);
> +	kfree(security);
>   }
>   EXPORT_SYMBOL(security_tun_dev_free_security);
>   
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index b3de2e941ef7..7f1d8358922a 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -5573,24 +5573,14 @@ static void selinux_req_classify_flow(const struct request_sock *req,
>   	flic->flowic_secid = req->secid;
>   }
>   
> -static int selinux_tun_dev_alloc_security(void **security)
> +static int selinux_tun_dev_alloc_security(void *security)
>   {
> -	struct tun_security_struct *tunsec;
> +	struct tun_security_struct *tunsec = selinux_tun_dev(security);
>   
> -	tunsec = kzalloc(sizeof(*tunsec), GFP_KERNEL);
> -	if (!tunsec)
> -		return -ENOMEM;
>   	tunsec->sid = current_sid();
> -
> -	*security = tunsec;
>   	return 0;
>   }
>   
> -static void selinux_tun_dev_free_security(void *security)
> -{
> -	kfree(security);
> -}
> -
>   static int selinux_tun_dev_create(void)
>   {
>   	u32 sid = current_sid();
> @@ -5608,7 +5598,7 @@ static int selinux_tun_dev_create(void)
>   
>   static int selinux_tun_dev_attach_queue(void *security)
>   {
> -	struct tun_security_struct *tunsec = security;
> +	struct tun_security_struct *tunsec = selinux_tun_dev(security);
>   
>   	return avc_has_perm(current_sid(), tunsec->sid, SECCLASS_TUN_SOCKET,
>   			    TUN_SOCKET__ATTACH_QUEUE, NULL);
> @@ -5616,7 +5606,7 @@ static int selinux_tun_dev_attach_queue(void *security)
>   
>   static int selinux_tun_dev_attach(struct sock *sk, void *security)
>   {
> -	struct tun_security_struct *tunsec = security;
> +	struct tun_security_struct *tunsec = selinux_tun_dev(security);
>   	struct sk_security_struct *sksec = selinux_sock(sk);
>   
>   	/* we don't currently perform any NetLabel based labeling here and it
> @@ -5634,7 +5624,7 @@ static int selinux_tun_dev_attach(struct sock *sk, void *security)
>   
>   static int selinux_tun_dev_open(void *security)
>   {
> -	struct tun_security_struct *tunsec = security;
> +	struct tun_security_struct *tunsec = selinux_tun_dev(security);
>   	u32 sid = current_sid();
>   	int err;
>   
> @@ -6975,6 +6965,7 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = {
>   	.lbs_sock = sizeof(struct sk_security_struct),
>   	.lbs_superblock = sizeof(struct superblock_security_struct),
>   	.lbs_xattr_count = SELINUX_INODE_INIT_XATTRS,
> +	.lbs_tun_dev = sizeof(struct tun_security_struct),
>   };
>   
>   #ifdef CONFIG_PERF_EVENTS
> @@ -7285,7 +7276,6 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
>   	LSM_HOOK_INIT(secmark_refcount_inc, selinux_secmark_refcount_inc),
>   	LSM_HOOK_INIT(secmark_refcount_dec, selinux_secmark_refcount_dec),
>   	LSM_HOOK_INIT(req_classify_flow, selinux_req_classify_flow),
> -	LSM_HOOK_INIT(tun_dev_free_security, selinux_tun_dev_free_security),
>   	LSM_HOOK_INIT(tun_dev_create, selinux_tun_dev_create),
>   	LSM_HOOK_INIT(tun_dev_attach_queue, selinux_tun_dev_attach_queue),
>   	LSM_HOOK_INIT(tun_dev_attach, selinux_tun_dev_attach),
> diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
> index 83b9443d6919..461c6985977d 100644
> --- a/security/selinux/include/objsec.h
> +++ b/security/selinux/include/objsec.h
> @@ -207,4 +207,10 @@ static inline struct sk_security_struct *selinux_sock(const struct sock *sock)
>   	return sock->sk_security + selinux_blob_sizes.lbs_sock;
>   }
>   
> +static inline struct tun_security_struct *
> +selinux_tun_dev(void *security)
> +{
> +	return security + selinux_blob_sizes.lbs_tun_dev;
> +}
> +
>   #endif /* _SELINUX_OBJSEC_H_ */


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 4/6] LSM: Infrastructure management of the dev_tun blob
  2024-07-09 23:01     ` John Johansen
@ 2024-07-09 23:11       ` Casey Schaufler
  0 siblings, 0 replies; 28+ messages in thread
From: Casey Schaufler @ 2024-07-09 23:11 UTC (permalink / raw)
  To: John Johansen, paul, linux-security-module
  Cc: jmorris, serge, keescook, penguin-kernel, stephen.smalley.work,
	mic, Casey Schaufler

On 7/9/2024 4:01 PM, John Johansen wrote:
> On 7/8/24 14:39, Casey Schaufler wrote:
>> Move management of the dev_tun security blob out of the individual
>> security modules and into the LSM infrastructure.  The security modules
>> tell the infrastructure how much space they require at initialization.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>
> one issue below, besides Paul's point the rest of it looks good
>
>> ---
>>   include/linux/lsm_hook_defs.h     |  2 +-
>>   include/linux/lsm_hooks.h         |  1 +
>>   security/security.c               | 11 ++++++++++-
>>   security/selinux/hooks.c          | 22 ++++++----------------
>>   security/selinux/include/objsec.h |  6 ++++++
>>   5 files changed, 24 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/linux/lsm_hook_defs.h
>> b/include/linux/lsm_hook_defs.h
>> index 44488b1ab9a9..601e3092a7ed 100644
>> --- a/include/linux/lsm_hook_defs.h
>> +++ b/include/linux/lsm_hook_defs.h
>> @@ -352,7 +352,7 @@ LSM_HOOK(void, LSM_RET_VOID,
>> secmark_refcount_inc, void)
>>   LSM_HOOK(void, LSM_RET_VOID, secmark_refcount_dec, void)
>>   LSM_HOOK(void, LSM_RET_VOID, req_classify_flow, const struct
>> request_sock *req,
>>        struct flowi_common *flic)
>> -LSM_HOOK(int, 0, tun_dev_alloc_security, void **security)
>> +LSM_HOOK(int, 0, tun_dev_alloc_security, void *security)
>>   LSM_HOOK(void, LSM_RET_VOID, tun_dev_free_security, void *security)
>>   LSM_HOOK(int, 0, tun_dev_create, void)
>>   LSM_HOOK(int, 0, tun_dev_attach_queue, void *security)
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 7233bc0737be..0ff14ff128c8 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -80,6 +80,7 @@ struct lsm_blob_sizes {
>>       int    lbs_msg_msg;
>>       int    lbs_task;
>>       int    lbs_xattr_count; /* number of xattr slots in new_xattrs
>> array */
>> +    int    lbs_tun_dev;
>>   };
>>     /**
>> diff --git a/security/security.c b/security/security.c
>> index 438ec6708eb3..2c9d075f5f92 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -234,6 +234,7 @@ static void __init lsm_set_blob_sizes(struct
>> lsm_blob_sizes *needed)
>>       lsm_set_blob_size(&needed->lbs_sock, &blob_sizes.lbs_sock);
>>       lsm_set_blob_size(&needed->lbs_superblock,
>> &blob_sizes.lbs_superblock);
>>       lsm_set_blob_size(&needed->lbs_task, &blob_sizes.lbs_task);
>> +    lsm_set_blob_size(&needed->lbs_tun_dev, &blob_sizes.lbs_tun_dev);
>>       lsm_set_blob_size(&needed->lbs_xattr_count,
>>                 &blob_sizes.lbs_xattr_count);
>>   }
>> @@ -412,6 +413,7 @@ static void __init ordered_lsm_init(void)
>>       init_debug("sock blob size       = %d\n", blob_sizes.lbs_sock);
>>       init_debug("superblock blob size = %d\n",
>> blob_sizes.lbs_superblock);
>>       init_debug("task blob size       = %d\n", blob_sizes.lbs_task);
>> +    init_debug("tun device blob size = %d\n", blob_sizes.lbs_tun_dev);
>>       init_debug("xattr slots          = %d\n",
>> blob_sizes.lbs_xattr_count);
>>         /*
>> @@ -4851,7 +4853,13 @@ EXPORT_SYMBOL(security_secmark_refcount_dec);
>>    */
>>   int security_tun_dev_alloc_security(void **security)
>>   {
>> -    return call_int_hook(tun_dev_alloc_security, security);
>> +    int rc;
>> +
>> +    rc = lsm_blob_alloc(security, blob_sizes.lbs_tun_dev, GFP_KERNEL);
>> +    if (rc)
>> +        return rc;
>> +
>> +    return call_int_hook(tun_dev_alloc_security, *security);
>
> we either need to free the blob here or make a change to tun_net_init.
>
> if security_tun_dev_alloc fails it is not calling the corresponding
> security_tun_dev_free_security()

Yes indeed, I missed that one. Fix in v2.

>
>>   }
>>   EXPORT_SYMBOL(security_tun_dev_alloc_security);
>>   @@ -4864,6 +4872,7 @@ EXPORT_SYMBOL(security_tun_dev_alloc_security);
>>   void security_tun_dev_free_security(void *security)
>>   {
>>       call_void_hook(tun_dev_free_security, security);
>> +    kfree(security);
>>   }
>>   EXPORT_SYMBOL(security_tun_dev_free_security);
>>   diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index b3de2e941ef7..7f1d8358922a 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -5573,24 +5573,14 @@ static void selinux_req_classify_flow(const
>> struct request_sock *req,
>>       flic->flowic_secid = req->secid;
>>   }
>>   -static int selinux_tun_dev_alloc_security(void **security)
>> +static int selinux_tun_dev_alloc_security(void *security)
>>   {
>> -    struct tun_security_struct *tunsec;
>> +    struct tun_security_struct *tunsec = selinux_tun_dev(security);
>>   -    tunsec = kzalloc(sizeof(*tunsec), GFP_KERNEL);
>> -    if (!tunsec)
>> -        return -ENOMEM;
>>       tunsec->sid = current_sid();
>> -
>> -    *security = tunsec;
>>       return 0;
>>   }
>>   -static void selinux_tun_dev_free_security(void *security)
>> -{
>> -    kfree(security);
>> -}
>> -
>>   static int selinux_tun_dev_create(void)
>>   {
>>       u32 sid = current_sid();
>> @@ -5608,7 +5598,7 @@ static int selinux_tun_dev_create(void)
>>     static int selinux_tun_dev_attach_queue(void *security)
>>   {
>> -    struct tun_security_struct *tunsec = security;
>> +    struct tun_security_struct *tunsec = selinux_tun_dev(security);
>>         return avc_has_perm(current_sid(), tunsec->sid,
>> SECCLASS_TUN_SOCKET,
>>                   TUN_SOCKET__ATTACH_QUEUE, NULL);
>> @@ -5616,7 +5606,7 @@ static int selinux_tun_dev_attach_queue(void
>> *security)
>>     static int selinux_tun_dev_attach(struct sock *sk, void *security)
>>   {
>> -    struct tun_security_struct *tunsec = security;
>> +    struct tun_security_struct *tunsec = selinux_tun_dev(security);
>>       struct sk_security_struct *sksec = selinux_sock(sk);
>>         /* we don't currently perform any NetLabel based labeling
>> here and it
>> @@ -5634,7 +5624,7 @@ static int selinux_tun_dev_attach(struct sock
>> *sk, void *security)
>>     static int selinux_tun_dev_open(void *security)
>>   {
>> -    struct tun_security_struct *tunsec = security;
>> +    struct tun_security_struct *tunsec = selinux_tun_dev(security);
>>       u32 sid = current_sid();
>>       int err;
>>   @@ -6975,6 +6965,7 @@ struct lsm_blob_sizes selinux_blob_sizes
>> __ro_after_init = {
>>       .lbs_sock = sizeof(struct sk_security_struct),
>>       .lbs_superblock = sizeof(struct superblock_security_struct),
>>       .lbs_xattr_count = SELINUX_INODE_INIT_XATTRS,
>> +    .lbs_tun_dev = sizeof(struct tun_security_struct),
>>   };
>>     #ifdef CONFIG_PERF_EVENTS
>> @@ -7285,7 +7276,6 @@ static struct security_hook_list
>> selinux_hooks[] __ro_after_init = {
>>       LSM_HOOK_INIT(secmark_refcount_inc, selinux_secmark_refcount_inc),
>>       LSM_HOOK_INIT(secmark_refcount_dec, selinux_secmark_refcount_dec),
>>       LSM_HOOK_INIT(req_classify_flow, selinux_req_classify_flow),
>> -    LSM_HOOK_INIT(tun_dev_free_security,
>> selinux_tun_dev_free_security),
>>       LSM_HOOK_INIT(tun_dev_create, selinux_tun_dev_create),
>>       LSM_HOOK_INIT(tun_dev_attach_queue, selinux_tun_dev_attach_queue),
>>       LSM_HOOK_INIT(tun_dev_attach, selinux_tun_dev_attach),
>> diff --git a/security/selinux/include/objsec.h
>> b/security/selinux/include/objsec.h
>> index 83b9443d6919..461c6985977d 100644
>> --- a/security/selinux/include/objsec.h
>> +++ b/security/selinux/include/objsec.h
>> @@ -207,4 +207,10 @@ static inline struct sk_security_struct
>> *selinux_sock(const struct sock *sock)
>>       return sock->sk_security + selinux_blob_sizes.lbs_sock;
>>   }
>>   +static inline struct tun_security_struct *
>> +selinux_tun_dev(void *security)
>> +{
>> +    return security + selinux_blob_sizes.lbs_tun_dev;
>> +}
>> +
>>   #endif /* _SELINUX_OBJSEC_H_ */
>
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 5/6] LSM: Infrastructure management of the infiniband blob
  2024-07-08 21:39 ` [PATCH 0/6] LSM: Infrastructure blob allocation Casey Schaufler
                     ` (3 preceding siblings ...)
  2024-07-08 21:39   ` [PATCH 4/6] LSM: Infrastructure management of the dev_tun blob Casey Schaufler
@ 2024-07-08 21:39   ` Casey Schaufler
  2024-07-09 22:08     ` Paul Moore
  2024-07-09 23:38     ` John Johansen
  2024-07-08 21:39   ` [PATCH 6/6] LSM: Infrastructure management of the perf_event security blob Casey Schaufler
  5 siblings, 2 replies; 28+ messages in thread
From: Casey Schaufler @ 2024-07-08 21:39 UTC (permalink / raw)
  To: casey, paul, linux-security-module
  Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
	stephen.smalley.work, mic

Move management of the infiniband security blob out of the individual
security modules and into the LSM infrastructure.  The security modules
tell the infrastructure how much space they require at initialization.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 include/linux/lsm_hook_defs.h     |  2 +-
 include/linux/lsm_hooks.h         |  1 +
 security/security.c               | 11 ++++++++++-
 security/selinux/hooks.c          | 16 +++-------------
 security/selinux/include/objsec.h |  6 ++++++
 5 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 601e3092a7ed..796ab400e992 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -373,7 +373,7 @@ LSM_HOOK(int, 0, mptcp_add_subflow, struct sock *sk, struct sock *ssk)
 LSM_HOOK(int, 0, ib_pkey_access, void *sec, u64 subnet_prefix, u16 pkey)
 LSM_HOOK(int, 0, ib_endport_manage_subnet, void *sec, const char *dev_name,
 	 u8 port_num)
-LSM_HOOK(int, 0, ib_alloc_security, void **sec)
+LSM_HOOK(int, 0, ib_alloc_security, void *sec)
 LSM_HOOK(void, LSM_RET_VOID, ib_free_security, void *sec)
 #endif /* CONFIG_SECURITY_INFINIBAND */
 
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 0ff14ff128c8..b6fc6ac88723 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -72,6 +72,7 @@ struct security_hook_list {
 struct lsm_blob_sizes {
 	int	lbs_cred;
 	int	lbs_file;
+	int	lbs_ib;
 	int	lbs_inode;
 	int	lbs_sock;
 	int	lbs_superblock;
diff --git a/security/security.c b/security/security.c
index 2c9d075f5f92..731a54fabc79 100644
--- a/security/security.c
+++ b/security/security.c
@@ -219,6 +219,7 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
 
 	lsm_set_blob_size(&needed->lbs_cred, &blob_sizes.lbs_cred);
 	lsm_set_blob_size(&needed->lbs_file, &blob_sizes.lbs_file);
+	lsm_set_blob_size(&needed->lbs_ib, &blob_sizes.lbs_ib);
 	/*
 	 * The inode blob gets an rcu_head in addition to
 	 * what the modules might need.
@@ -404,6 +405,7 @@ static void __init ordered_lsm_init(void)
 
 	init_debug("cred blob size       = %d\n", blob_sizes.lbs_cred);
 	init_debug("file blob size       = %d\n", blob_sizes.lbs_file);
+	init_debug("ib blob size         = %d\n", blob_sizes.lbs_ib);
 	init_debug("inode blob size      = %d\n", blob_sizes.lbs_inode);
 	init_debug("ipc blob size        = %d\n", blob_sizes.lbs_ipc);
 #ifdef CONFIG_KEYS
@@ -5068,7 +5070,13 @@ EXPORT_SYMBOL(security_ib_endport_manage_subnet);
  */
 int security_ib_alloc_security(void **sec)
 {
-	return call_int_hook(ib_alloc_security, sec);
+	int rc;
+
+	rc = lsm_blob_alloc(sec, blob_sizes.lbs_ib, GFP_KERNEL);
+	if (rc)
+		return rc;
+
+	return call_int_hook(ib_alloc_security, *sec);
 }
 EXPORT_SYMBOL(security_ib_alloc_security);
 
@@ -5081,6 +5089,7 @@ EXPORT_SYMBOL(security_ib_alloc_security);
 void security_ib_free_security(void *sec)
 {
 	call_void_hook(ib_free_security, sec);
+	kfree(sec);
 }
 EXPORT_SYMBOL(security_ib_free_security);
 #endif	/* CONFIG_SECURITY_INFINIBAND */
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 7f1d8358922a..79fe75603881 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6776,23 +6776,13 @@ static int selinux_ib_endport_manage_subnet(void *ib_sec, const char *dev_name,
 			    INFINIBAND_ENDPORT__MANAGE_SUBNET, &ad);
 }
 
-static int selinux_ib_alloc_security(void **ib_sec)
+static int selinux_ib_alloc_security(void *ib_sec)
 {
-	struct ib_security_struct *sec;
+	struct ib_security_struct *sec = selinux_ib(ib_sec);
 
-	sec = kzalloc(sizeof(*sec), GFP_KERNEL);
-	if (!sec)
-		return -ENOMEM;
 	sec->sid = current_sid();
-
-	*ib_sec = sec;
 	return 0;
 }
-
-static void selinux_ib_free_security(void *ib_sec)
-{
-	kfree(ib_sec);
-}
 #endif
 
 #ifdef CONFIG_BPF_SYSCALL
@@ -6966,6 +6956,7 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = {
 	.lbs_superblock = sizeof(struct superblock_security_struct),
 	.lbs_xattr_count = SELINUX_INODE_INIT_XATTRS,
 	.lbs_tun_dev = sizeof(struct tun_security_struct),
+	.lbs_ib = sizeof(struct ib_security_struct),
 };
 
 #ifdef CONFIG_PERF_EVENTS
@@ -7284,7 +7275,6 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(ib_pkey_access, selinux_ib_pkey_access),
 	LSM_HOOK_INIT(ib_endport_manage_subnet,
 		      selinux_ib_endport_manage_subnet),
-	LSM_HOOK_INIT(ib_free_security, selinux_ib_free_security),
 #endif
 #ifdef CONFIG_SECURITY_NETWORK_XFRM
 	LSM_HOOK_INIT(xfrm_policy_free_security, selinux_xfrm_policy_free),
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index 461c6985977d..b1878f9395b5 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -213,4 +213,10 @@ selinux_tun_dev(void *security)
 	return security + selinux_blob_sizes.lbs_tun_dev;
 }
 
+static inline struct ib_security_struct *
+selinux_ib(void *ib_sec)
+{
+	return ib_sec + selinux_blob_sizes.lbs_ib;
+}
+
 #endif /* _SELINUX_OBJSEC_H_ */
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH 5/6] LSM: Infrastructure management of the infiniband blob
  2024-07-08 21:39   ` [PATCH 5/6] LSM: Infrastructure management of the infiniband blob Casey Schaufler
@ 2024-07-09 22:08     ` Paul Moore
  2024-07-09 23:38     ` John Johansen
  1 sibling, 0 replies; 28+ messages in thread
From: Paul Moore @ 2024-07-09 22:08 UTC (permalink / raw)
  To: Casey Schaufler, casey, linux-security-module
  Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
	stephen.smalley.work, mic

On Jul  8, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote:
> 
> Move management of the infiniband security blob out of the individual
> security modules and into the LSM infrastructure.  The security modules
> tell the infrastructure how much space they require at initialization.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>  include/linux/lsm_hook_defs.h     |  2 +-
>  include/linux/lsm_hooks.h         |  1 +
>  security/security.c               | 11 ++++++++++-
>  security/selinux/hooks.c          | 16 +++-------------
>  security/selinux/include/objsec.h |  6 ++++++
>  5 files changed, 21 insertions(+), 15 deletions(-)

...

> diff --git a/security/security.c b/security/security.c
> index 2c9d075f5f92..731a54fabc79 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -5081,6 +5089,7 @@ EXPORT_SYMBOL(security_ib_alloc_security);
>  void security_ib_free_security(void *sec)
>  {
>  	call_void_hook(ib_free_security, sec);
> +	kfree(sec);
>  }
>  EXPORT_SYMBOL(security_ib_free_security);

Same comment as elsewhere, let's be consistent with the *free() hooks,
either we keep them or we remove them if unused (preferable).

--
paul-moore.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 5/6] LSM: Infrastructure management of the infiniband blob
  2024-07-08 21:39   ` [PATCH 5/6] LSM: Infrastructure management of the infiniband blob Casey Schaufler
  2024-07-09 22:08     ` Paul Moore
@ 2024-07-09 23:38     ` John Johansen
  1 sibling, 0 replies; 28+ messages in thread
From: John Johansen @ 2024-07-09 23:38 UTC (permalink / raw)
  To: Casey Schaufler, paul, linux-security-module
  Cc: jmorris, serge, keescook, penguin-kernel, stephen.smalley.work,
	mic

On 7/8/24 14:39, Casey Schaufler wrote:
> Move management of the infiniband security blob out of the individual
> security modules and into the LSM infrastructure.  The security modules
> tell the infrastructure how much space they require at initialization.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

an issue below, other than that and Paul's point it looks good

> ---
>   include/linux/lsm_hook_defs.h     |  2 +-
>   include/linux/lsm_hooks.h         |  1 +
>   security/security.c               | 11 ++++++++++-
>   security/selinux/hooks.c          | 16 +++-------------
>   security/selinux/include/objsec.h |  6 ++++++
>   5 files changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 601e3092a7ed..796ab400e992 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -373,7 +373,7 @@ LSM_HOOK(int, 0, mptcp_add_subflow, struct sock *sk, struct sock *ssk)
>   LSM_HOOK(int, 0, ib_pkey_access, void *sec, u64 subnet_prefix, u16 pkey)
>   LSM_HOOK(int, 0, ib_endport_manage_subnet, void *sec, const char *dev_name,
>   	 u8 port_num)
> -LSM_HOOK(int, 0, ib_alloc_security, void **sec)
> +LSM_HOOK(int, 0, ib_alloc_security, void *sec)
>   LSM_HOOK(void, LSM_RET_VOID, ib_free_security, void *sec)
>   #endif /* CONFIG_SECURITY_INFINIBAND */
>   
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 0ff14ff128c8..b6fc6ac88723 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -72,6 +72,7 @@ struct security_hook_list {
>   struct lsm_blob_sizes {
>   	int	lbs_cred;
>   	int	lbs_file;
> +	int	lbs_ib;
>   	int	lbs_inode;
>   	int	lbs_sock;
>   	int	lbs_superblock;
> diff --git a/security/security.c b/security/security.c
> index 2c9d075f5f92..731a54fabc79 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -219,6 +219,7 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
>   
>   	lsm_set_blob_size(&needed->lbs_cred, &blob_sizes.lbs_cred);
>   	lsm_set_blob_size(&needed->lbs_file, &blob_sizes.lbs_file);
> +	lsm_set_blob_size(&needed->lbs_ib, &blob_sizes.lbs_ib);
>   	/*
>   	 * The inode blob gets an rcu_head in addition to
>   	 * what the modules might need.
> @@ -404,6 +405,7 @@ static void __init ordered_lsm_init(void)
>   
>   	init_debug("cred blob size       = %d\n", blob_sizes.lbs_cred);
>   	init_debug("file blob size       = %d\n", blob_sizes.lbs_file);
> +	init_debug("ib blob size         = %d\n", blob_sizes.lbs_ib);
>   	init_debug("inode blob size      = %d\n", blob_sizes.lbs_inode);
>   	init_debug("ipc blob size        = %d\n", blob_sizes.lbs_ipc);
>   #ifdef CONFIG_KEYS
> @@ -5068,7 +5070,13 @@ EXPORT_SYMBOL(security_ib_endport_manage_subnet);
>    */
>   int security_ib_alloc_security(void **sec)
>   {
> -	return call_int_hook(ib_alloc_security, sec);
> +	int rc;
> +
> +	rc = lsm_blob_alloc(sec, blob_sizes.lbs_ib, GFP_KERNEL);
> +	if (rc)
> +		return rc;
> +
> +	return call_int_hook(ib_alloc_security, *sec);

this has a similar problem to the tun dev. When security_ib_alloc() fails,
security_ib_free() hook isn't getting called

>   }
>   EXPORT_SYMBOL(security_ib_alloc_security);
>   
> @@ -5081,6 +5089,7 @@ EXPORT_SYMBOL(security_ib_alloc_security);
>   void security_ib_free_security(void *sec)
>   {
>   	call_void_hook(ib_free_security, sec);
> +	kfree(sec);
>   }
>   EXPORT_SYMBOL(security_ib_free_security);
>   #endif	/* CONFIG_SECURITY_INFINIBAND */
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 7f1d8358922a..79fe75603881 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6776,23 +6776,13 @@ static int selinux_ib_endport_manage_subnet(void *ib_sec, const char *dev_name,
>   			    INFINIBAND_ENDPORT__MANAGE_SUBNET, &ad);
>   }
>   
> -static int selinux_ib_alloc_security(void **ib_sec)
> +static int selinux_ib_alloc_security(void *ib_sec)
>   {
> -	struct ib_security_struct *sec;
> +	struct ib_security_struct *sec = selinux_ib(ib_sec);
>   
> -	sec = kzalloc(sizeof(*sec), GFP_KERNEL);
> -	if (!sec)
> -		return -ENOMEM;
>   	sec->sid = current_sid();
> -
> -	*ib_sec = sec;
>   	return 0;
>   }
> -
> -static void selinux_ib_free_security(void *ib_sec)
> -{
> -	kfree(ib_sec);
> -}
>   #endif
>   
>   #ifdef CONFIG_BPF_SYSCALL
> @@ -6966,6 +6956,7 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = {
>   	.lbs_superblock = sizeof(struct superblock_security_struct),
>   	.lbs_xattr_count = SELINUX_INODE_INIT_XATTRS,
>   	.lbs_tun_dev = sizeof(struct tun_security_struct),
> +	.lbs_ib = sizeof(struct ib_security_struct),
>   };
>   
>   #ifdef CONFIG_PERF_EVENTS
> @@ -7284,7 +7275,6 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
>   	LSM_HOOK_INIT(ib_pkey_access, selinux_ib_pkey_access),
>   	LSM_HOOK_INIT(ib_endport_manage_subnet,
>   		      selinux_ib_endport_manage_subnet),
> -	LSM_HOOK_INIT(ib_free_security, selinux_ib_free_security),
>   #endif
>   #ifdef CONFIG_SECURITY_NETWORK_XFRM
>   	LSM_HOOK_INIT(xfrm_policy_free_security, selinux_xfrm_policy_free),
> diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
> index 461c6985977d..b1878f9395b5 100644
> --- a/security/selinux/include/objsec.h
> +++ b/security/selinux/include/objsec.h
> @@ -213,4 +213,10 @@ selinux_tun_dev(void *security)
>   	return security + selinux_blob_sizes.lbs_tun_dev;
>   }
>   
> +static inline struct ib_security_struct *
> +selinux_ib(void *ib_sec)
> +{
> +	return ib_sec + selinux_blob_sizes.lbs_ib;
> +}
> +
>   #endif /* _SELINUX_OBJSEC_H_ */


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 6/6] LSM: Infrastructure management of the perf_event security blob
  2024-07-08 21:39 ` [PATCH 0/6] LSM: Infrastructure blob allocation Casey Schaufler
                     ` (4 preceding siblings ...)
  2024-07-08 21:39   ` [PATCH 5/6] LSM: Infrastructure management of the infiniband blob Casey Schaufler
@ 2024-07-08 21:39   ` Casey Schaufler
  2024-07-09 22:08     ` Paul Moore
  2024-07-09 23:47     ` John Johansen
  5 siblings, 2 replies; 28+ messages in thread
From: Casey Schaufler @ 2024-07-08 21:39 UTC (permalink / raw)
  To: casey, paul, linux-security-module
  Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
	stephen.smalley.work, mic

Move management of the perf_event->security blob out of the individual
security modules and into the security infrastructure. Instead of
allocating the blobs from within the modules the modules tell the
infrastructure how much space is required, and the space is allocated
there.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 include/linux/lsm_hooks.h         |  1 +
 security/security.c               | 12 ++++++++++++
 security/selinux/hooks.c          | 18 ++++--------------
 security/selinux/include/objsec.h |  6 ++++++
 4 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index b6fc6ac88723..f1ca8082075a 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -79,6 +79,7 @@ struct lsm_blob_sizes {
 	int	lbs_ipc;
 	int	lbs_key;
 	int	lbs_msg_msg;
+	int	lbs_perf_event;
 	int	lbs_task;
 	int	lbs_xattr_count; /* number of xattr slots in new_xattrs array */
 	int	lbs_tun_dev;
diff --git a/security/security.c b/security/security.c
index 731a54fabc79..da2111f8d9df 100644
--- a/security/security.c
+++ b/security/security.c
@@ -28,6 +28,7 @@
 #include <linux/xattr.h>
 #include <linux/msg.h>
 #include <linux/overflow.h>
+#include <linux/perf_event.h>
 #include <net/flow.h>
 #include <net/sock.h>
 
@@ -232,6 +233,7 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
 	lsm_set_blob_size(&needed->lbs_key, &blob_sizes.lbs_key);
 #endif
 	lsm_set_blob_size(&needed->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
+	lsm_set_blob_size(&needed->lbs_perf_event, &blob_sizes.lbs_perf_event);
 	lsm_set_blob_size(&needed->lbs_sock, &blob_sizes.lbs_sock);
 	lsm_set_blob_size(&needed->lbs_superblock, &blob_sizes.lbs_superblock);
 	lsm_set_blob_size(&needed->lbs_task, &blob_sizes.lbs_task);
@@ -414,6 +416,7 @@ static void __init ordered_lsm_init(void)
 	init_debug("msg_msg blob size    = %d\n", blob_sizes.lbs_msg_msg);
 	init_debug("sock blob size       = %d\n", blob_sizes.lbs_sock);
 	init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
+	init_debug("perf event blob size = %d\n", blob_sizes.lbs_perf_event);
 	init_debug("task blob size       = %d\n", blob_sizes.lbs_task);
 	init_debug("tun device blob size = %d\n", blob_sizes.lbs_tun_dev);
 	init_debug("xattr slots          = %d\n", blob_sizes.lbs_xattr_count);
@@ -5653,6 +5656,13 @@ int security_perf_event_open(struct perf_event_attr *attr, int type)
  */
 int security_perf_event_alloc(struct perf_event *event)
 {
+	int rc;
+
+	rc = lsm_blob_alloc(&event->security, blob_sizes.lbs_perf_event,
+			    GFP_KERNEL);
+	if (rc)
+		return rc;
+
 	return call_int_hook(perf_event_alloc, event);
 }
 
@@ -5665,6 +5675,8 @@ int security_perf_event_alloc(struct perf_event *event)
 void security_perf_event_free(struct perf_event *event)
 {
 	call_void_hook(perf_event_free, event);
+	kfree(event->security);
+	event->security = NULL;
 }
 
 /**
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 79fe75603881..d1d6adfdfbc7 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6952,6 +6952,9 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = {
 	.lbs_key = sizeof(struct key_security_struct),
 #endif /* CONFIG_KEYS */
 	.lbs_msg_msg = sizeof(struct msg_security_struct),
+#ifdef CONFIG_PERF_EVENTS
+	.lbs_perf_event = sizeof(struct perf_event_security_struct),
+#endif
 	.lbs_sock = sizeof(struct sk_security_struct),
 	.lbs_superblock = sizeof(struct superblock_security_struct),
 	.lbs_xattr_count = SELINUX_INODE_INIT_XATTRS,
@@ -6983,24 +6986,12 @@ static int selinux_perf_event_alloc(struct perf_event *event)
 {
 	struct perf_event_security_struct *perfsec;
 
-	perfsec = kzalloc(sizeof(*perfsec), GFP_KERNEL);
-	if (!perfsec)
-		return -ENOMEM;
-
+	perfsec = selinux_perf_event(event->security);
 	perfsec->sid = current_sid();
-	event->security = perfsec;
 
 	return 0;
 }
 
-static void selinux_perf_event_free(struct perf_event *event)
-{
-	struct perf_event_security_struct *perfsec = event->security;
-
-	event->security = NULL;
-	kfree(perfsec);
-}
-
 static int selinux_perf_event_read(struct perf_event *event)
 {
 	struct perf_event_security_struct *perfsec = event->security;
@@ -7312,7 +7303,6 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
 
 #ifdef CONFIG_PERF_EVENTS
 	LSM_HOOK_INIT(perf_event_open, selinux_perf_event_open),
-	LSM_HOOK_INIT(perf_event_free, selinux_perf_event_free),
 	LSM_HOOK_INIT(perf_event_read, selinux_perf_event_read),
 	LSM_HOOK_INIT(perf_event_write, selinux_perf_event_write),
 #endif
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index b1878f9395b5..d632a9180b41 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -219,4 +219,10 @@ selinux_ib(void *ib_sec)
 	return ib_sec + selinux_blob_sizes.lbs_ib;
 }
 
+static inline struct perf_event_security_struct *
+selinux_perf_event(void *perf_event)
+{
+	return perf_event + selinux_blob_sizes.lbs_perf_event;
+}
+
 #endif /* _SELINUX_OBJSEC_H_ */
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH 6/6] LSM: Infrastructure management of the perf_event  security blob
  2024-07-08 21:39   ` [PATCH 6/6] LSM: Infrastructure management of the perf_event security blob Casey Schaufler
@ 2024-07-09 22:08     ` Paul Moore
  2024-07-09 23:47     ` John Johansen
  1 sibling, 0 replies; 28+ messages in thread
From: Paul Moore @ 2024-07-09 22:08 UTC (permalink / raw)
  To: Casey Schaufler, casey, linux-security-module
  Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
	stephen.smalley.work, mic

On Jul  8, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote:
> 
> Move management of the perf_event->security blob out of the individual
> security modules and into the security infrastructure. Instead of
> allocating the blobs from within the modules the modules tell the
> infrastructure how much space is required, and the space is allocated
> there.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>  include/linux/lsm_hooks.h         |  1 +
>  security/security.c               | 12 ++++++++++++
>  security/selinux/hooks.c          | 18 ++++--------------
>  security/selinux/include/objsec.h |  6 ++++++
>  4 files changed, 23 insertions(+), 14 deletions(-)

...

> @@ -5665,6 +5675,8 @@ int security_perf_event_alloc(struct perf_event *event)
>  void security_perf_event_free(struct perf_event *event)
>  {
>  	call_void_hook(perf_event_free, event);
> +	kfree(event->security);
> +	event->security = NULL;
>  }

See previous comments regarding the *free() hooks.

--
paul-moore.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 6/6] LSM: Infrastructure management of the perf_event security blob
  2024-07-08 21:39   ` [PATCH 6/6] LSM: Infrastructure management of the perf_event security blob Casey Schaufler
  2024-07-09 22:08     ` Paul Moore
@ 2024-07-09 23:47     ` John Johansen
  1 sibling, 0 replies; 28+ messages in thread
From: John Johansen @ 2024-07-09 23:47 UTC (permalink / raw)
  To: Casey Schaufler, paul, linux-security-module
  Cc: jmorris, serge, keescook, penguin-kernel, stephen.smalley.work,
	mic

On 7/8/24 14:39, Casey Schaufler wrote:
> Move management of the perf_event->security blob out of the individual
> security modules and into the security infrastructure. Instead of
> allocating the blobs from within the modules the modules tell the
> infrastructure how much space is required, and the space is allocated
> there.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

again, issue below, and other than Paul's point about the free hook the
rest looks good

> ---
>   include/linux/lsm_hooks.h         |  1 +
>   security/security.c               | 12 ++++++++++++
>   security/selinux/hooks.c          | 18 ++++--------------
>   security/selinux/include/objsec.h |  6 ++++++
>   4 files changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index b6fc6ac88723..f1ca8082075a 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -79,6 +79,7 @@ struct lsm_blob_sizes {
>   	int	lbs_ipc;
>   	int	lbs_key;
>   	int	lbs_msg_msg;
> +	int	lbs_perf_event;
>   	int	lbs_task;
>   	int	lbs_xattr_count; /* number of xattr slots in new_xattrs array */
>   	int	lbs_tun_dev;
> diff --git a/security/security.c b/security/security.c
> index 731a54fabc79..da2111f8d9df 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -28,6 +28,7 @@
>   #include <linux/xattr.h>
>   #include <linux/msg.h>
>   #include <linux/overflow.h>
> +#include <linux/perf_event.h>
>   #include <net/flow.h>
>   #include <net/sock.h>
>   
> @@ -232,6 +233,7 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
>   	lsm_set_blob_size(&needed->lbs_key, &blob_sizes.lbs_key);
>   #endif
>   	lsm_set_blob_size(&needed->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
> +	lsm_set_blob_size(&needed->lbs_perf_event, &blob_sizes.lbs_perf_event);
>   	lsm_set_blob_size(&needed->lbs_sock, &blob_sizes.lbs_sock);
>   	lsm_set_blob_size(&needed->lbs_superblock, &blob_sizes.lbs_superblock);
>   	lsm_set_blob_size(&needed->lbs_task, &blob_sizes.lbs_task);
> @@ -414,6 +416,7 @@ static void __init ordered_lsm_init(void)
>   	init_debug("msg_msg blob size    = %d\n", blob_sizes.lbs_msg_msg);
>   	init_debug("sock blob size       = %d\n", blob_sizes.lbs_sock);
>   	init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
> +	init_debug("perf event blob size = %d\n", blob_sizes.lbs_perf_event);
>   	init_debug("task blob size       = %d\n", blob_sizes.lbs_task);
>   	init_debug("tun device blob size = %d\n", blob_sizes.lbs_tun_dev);
>   	init_debug("xattr slots          = %d\n", blob_sizes.lbs_xattr_count);
> @@ -5653,6 +5656,13 @@ int security_perf_event_open(struct perf_event_attr *attr, int type)
>    */
>   int security_perf_event_alloc(struct perf_event *event)
>   {
> +	int rc;
> +
> +	rc = lsm_blob_alloc(&event->security, blob_sizes.lbs_perf_event,
> +			    GFP_KERNEL);
> +	if (rc)
> +		return rc;
> +

again similar issue. While free_event_rcu() is called that one doesn't
actually take care of the security field

>   	return call_int_hook(perf_event_alloc, event);
>   }
>   
> @@ -5665,6 +5675,8 @@ int security_perf_event_alloc(struct perf_event *event)
>   void security_perf_event_free(struct perf_event *event)
>   {
>   	call_void_hook(perf_event_free, event);
> +	kfree(event->security);
> +	event->security = NULL;
>   }
>   
>   /**
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 79fe75603881..d1d6adfdfbc7 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6952,6 +6952,9 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = {
>   	.lbs_key = sizeof(struct key_security_struct),
>   #endif /* CONFIG_KEYS */
>   	.lbs_msg_msg = sizeof(struct msg_security_struct),
> +#ifdef CONFIG_PERF_EVENTS
> +	.lbs_perf_event = sizeof(struct perf_event_security_struct),
> +#endif
>   	.lbs_sock = sizeof(struct sk_security_struct),
>   	.lbs_superblock = sizeof(struct superblock_security_struct),
>   	.lbs_xattr_count = SELINUX_INODE_INIT_XATTRS,
> @@ -6983,24 +6986,12 @@ static int selinux_perf_event_alloc(struct perf_event *event)
>   {
>   	struct perf_event_security_struct *perfsec;
>   
> -	perfsec = kzalloc(sizeof(*perfsec), GFP_KERNEL);
> -	if (!perfsec)
> -		return -ENOMEM;
> -
> +	perfsec = selinux_perf_event(event->security);
>   	perfsec->sid = current_sid();
> -	event->security = perfsec;
>   
>   	return 0;
>   }
>   
> -static void selinux_perf_event_free(struct perf_event *event)
> -{
> -	struct perf_event_security_struct *perfsec = event->security;
> -
> -	event->security = NULL;
> -	kfree(perfsec);
> -}
> -
>   static int selinux_perf_event_read(struct perf_event *event)
>   {
>   	struct perf_event_security_struct *perfsec = event->security;
> @@ -7312,7 +7303,6 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
>   
>   #ifdef CONFIG_PERF_EVENTS
>   	LSM_HOOK_INIT(perf_event_open, selinux_perf_event_open),
> -	LSM_HOOK_INIT(perf_event_free, selinux_perf_event_free),
>   	LSM_HOOK_INIT(perf_event_read, selinux_perf_event_read),
>   	LSM_HOOK_INIT(perf_event_write, selinux_perf_event_write),
>   #endif
> diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
> index b1878f9395b5..d632a9180b41 100644
> --- a/security/selinux/include/objsec.h
> +++ b/security/selinux/include/objsec.h
> @@ -219,4 +219,10 @@ selinux_ib(void *ib_sec)
>   	return ib_sec + selinux_blob_sizes.lbs_ib;
>   }
>   
> +static inline struct perf_event_security_struct *
> +selinux_perf_event(void *perf_event)
> +{
> +	return perf_event + selinux_blob_sizes.lbs_perf_event;
> +}
> +
>   #endif /* _SELINUX_OBJSEC_H_ */


^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2024-07-10  0:01 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20240708213957.20519-1-casey.ref@schaufler-ca.com>
2024-07-08 21:39 ` [PATCH 0/6] LSM: Infrastructure blob allocation Casey Schaufler
2024-07-08 21:39   ` [PATCH 1/6] LSM: Infrastructure management of the sock security Casey Schaufler
2024-07-09 19:15     ` Paul Moore
2024-07-09 23:00       ` Casey Schaufler
2024-07-09 23:05         ` Paul Moore
2024-07-09 23:29           ` Casey Schaufler
2024-07-10  0:00             ` Paul Moore
2024-07-09 22:08     ` Paul Moore
2024-07-09 22:32     ` John Johansen
2024-07-08 21:39   ` [PATCH 2/6] LSM: Infrastructure management of the key security blob Casey Schaufler
2024-07-09 22:08     ` Paul Moore
2024-07-09 22:47       ` John Johansen
2024-07-09 23:01         ` Paul Moore
2024-07-08 21:39   ` [PATCH 3/6] LSM: Add helper for blob allocations Casey Schaufler
2024-07-09 22:08     ` Paul Moore
2024-07-09 23:09       ` Casey Schaufler
2024-07-10  0:01         ` Paul Moore
2024-07-09 22:51     ` John Johansen
2024-07-08 21:39   ` [PATCH 4/6] LSM: Infrastructure management of the dev_tun blob Casey Schaufler
2024-07-09 22:08     ` Paul Moore
2024-07-09 23:01     ` John Johansen
2024-07-09 23:11       ` Casey Schaufler
2024-07-08 21:39   ` [PATCH 5/6] LSM: Infrastructure management of the infiniband blob Casey Schaufler
2024-07-09 22:08     ` Paul Moore
2024-07-09 23:38     ` John Johansen
2024-07-08 21:39   ` [PATCH 6/6] LSM: Infrastructure management of the perf_event security blob Casey Schaufler
2024-07-09 22:08     ` Paul Moore
2024-07-09 23:47     ` John Johansen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox