[PATCH net 0/4] security: fixups for the security hooks in sctp

There are a couple of problems in the currect security hooks in sctp:

1. The hooks incorrectly treat sctp_endpoint in SCTP as request_sock in
   TCP, while it's in fact no more than an extension of the sock, and
   represents the local host. It is created when sock is created, not
   when a conn request comes. sctp_association is actually the correct
   one to represent the connection, and created when a conn request
   arrives.

2. security_sctp_assoc_request() hook should also be called in processing
   COOKIE ECHO, as that's the place where the real assoc is created and
   used in the future.

The problems above may cause accept sk, peeloff sk or client sk having
the incorrect security labels.

So this patchset is to change some hooks and pass asoc into them and save
these secids into asoc, as well as add the missing sctp_assoc_request
hook into the COOKIE ECHO processing.

Xin Long (4):
  security: pass asoc to sctp_assoc_request and sctp_sk_clone
  security: call security_sctp_assoc_request in sctp_sf_do_5_1D_ce
  security: add sctp_assoc_established hook
  security: implement sctp_assoc_established hook in selinux

 Documentation/security/SCTP.rst     | 65 +++++++++++++++--------------
 include/linux/lsm_hook_defs.h       |  6 ++-
 include/linux/lsm_hooks.h           | 13 ++++--
 include/linux/security.h            | 18 +++++---
 include/net/sctp/structs.h          | 20 ++++-----
 net/sctp/sm_statefuns.c             | 31 ++++++++------
 net/sctp/socket.c                   |  5 +--
 security/security.c                 | 15 +++++--
 security/selinux/hooks.c            | 36 +++++++++++-----
 security/selinux/include/netlabel.h |  4 +-
 security/selinux/netlabel.c         | 14 +++----
 11 files changed, 135 insertions(+), 92 deletions(-)

-- 
2.27.0

This patch is to move secid and peer_secid from endpoint to association,
and pass asoc to sctp_assoc_request and sctp_sk_clone instead of ep. As
ep is the local endpoint and asoc represents a connection, and in SCTP
one sk/ep could have multiple asoc/connection, saving secid/peer_secid
for new asoc will overwrite the old asoc's.

Note that since asoc can be passed as NULL, security_sctp_assoc_request()
is moved to the place right after the new_asoc is created in
sctp_sf_do_5_1B_init() and sctp_sf_do_unexpected_init().

Fixes: 72e89f50084c ("security: Add support for SCTP security hooks")
Reported-by: Prashanth Prahlad <redacted>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 Documentation/security/SCTP.rst     | 28 ++++++++++++++--------------
 include/linux/lsm_hook_defs.h       |  4 ++--
 include/linux/lsm_hooks.h           |  8 ++++----
 include/linux/security.h            | 10 +++++-----
 include/net/sctp/structs.h          | 20 ++++++++++----------
 net/sctp/sm_statefuns.c             | 26 +++++++++++++-------------
 net/sctp/socket.c                   |  5 ++---
 security/security.c                 |  8 ++++----
 security/selinux/hooks.c            | 20 ++++++++++----------
 security/selinux/include/netlabel.h |  4 ++--
 security/selinux/netlabel.c         | 14 +++++++-------
 11 files changed, 73 insertions(+), 74 deletions(-)

diff --git a/Documentation/security/SCTP.rst b/Documentation/security/SCTP.rst
index 0bcf6c1245ee..415b548d9ce0 100644
--- a/Documentation/security/SCTP.rst
+++ b/Documentation/security/SCTP.rst

@@ -26,11 +26,11 @@ described in the `SCTP SELinux Support`_ chapter.
 
 security_sctp_assoc_request()
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-Passes the ``@ep`` and ``@chunk->skb`` of the association INIT packet to the
+Passes the ``@asoc`` and ``@chunk->skb`` of the association INIT packet to the
 security module. Returns 0 on success, error on failure.
 ::
 
-    @ep - pointer to sctp endpoint structure.
+    @asoc - pointer to sctp association structure.
     @skb - pointer to skbuff of association packet.
 
 

@@ -117,9 +117,9 @@ Called whenever a new socket is created by **accept**\(2)
 calls **sctp_peeloff**\(3).
 ::
 
-    @ep - pointer to current sctp endpoint structure.
+    @asoc - pointer to current sctp association structure.
     @sk - pointer to current sock structure.
-    @sk - pointer to new sock structure.
+    @newsk - pointer to new sock structure.
 
 
 security_inet_conn_established()

@@ -200,22 +200,22 @@ hooks with the SELinux specifics expanded below::
 
 security_sctp_assoc_request()
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-Passes the ``@ep`` and ``@chunk->skb`` of the association INIT packet to the
+Passes the ``@asoc`` and ``@chunk->skb`` of the association INIT packet to the
 security module. Returns 0 on success, error on failure.
 ::
 
-    @ep - pointer to sctp endpoint structure.
+    @asoc - pointer to sctp association structure.
     @skb - pointer to skbuff of association packet.
 
 The security module performs the following operations:
-     IF this is the first association on ``@ep->base.sk``, then set the peer
+     IF this is the first association on ``@asoc->base.sk``, then set the peer
      sid to that in ``@skb``. This will ensure there is only one peer sid
-     assigned to ``@ep->base.sk`` that may support multiple associations.
+     assigned to ``@asoc->base.sk`` that may support multiple associations.
 
-     ELSE validate the ``@ep->base.sk peer_sid`` against the ``@skb peer sid``
+     ELSE validate the ``@asoc->base.sk peer_sid`` against the ``@skb peer sid``
      to determine whether the association should be allowed or denied.
 
-     Set the sctp ``@ep sid`` to socket's sid (from ``ep->base.sk``) with
+     Set the sctp ``@asoc sid`` to socket's sid (from ``asoc->base.sk``) with
      MLS portion taken from ``@skb peer sid``. This will be used by SCTP
      TCP style sockets and peeled off connections as they cause a new socket
      to be generated.

@@ -259,13 +259,13 @@ security_sctp_sk_clone()
 Called whenever a new socket is created by **accept**\(2) (i.e. a TCP style
 socket) or when a socket is 'peeled off' e.g userspace calls
 **sctp_peeloff**\(3). ``security_sctp_sk_clone()`` will set the new
-sockets sid and peer sid to that contained in the ``@ep sid`` and
-``@ep peer sid`` respectively.
+sockets sid and peer sid to that contained in the ``@asoc sid`` and
+``@asoc peer sid`` respectively.
 ::
 
-    @ep - pointer to current sctp endpoint structure.
+    @asoc - pointer to current sctp association structure.
     @sk - pointer to current sock structure.
-    @sk - pointer to new sock structure.
+    @newsk - pointer to new sock structure.
 
 
 security_inet_conn_established()

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 2adeea44c0d5..0024273a7382 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h

@@ -328,11 +328,11 @@ LSM_HOOK(int, 0, tun_dev_create, void)
 LSM_HOOK(int, 0, tun_dev_attach_queue, void *security)
 LSM_HOOK(int, 0, tun_dev_attach, struct sock *sk, void *security)
 LSM_HOOK(int, 0, tun_dev_open, void *security)
-LSM_HOOK(int, 0, sctp_assoc_request, struct sctp_endpoint *ep,
+LSM_HOOK(int, 0, sctp_assoc_request, struct sctp_association *asoc,
 	 struct sk_buff *skb)
 LSM_HOOK(int, 0, sctp_bind_connect, struct sock *sk, int optname,
 	 struct sockaddr *address, int addrlen)
-LSM_HOOK(void, LSM_RET_VOID, sctp_sk_clone, struct sctp_endpoint *ep,
+LSM_HOOK(void, LSM_RET_VOID, sctp_sk_clone, struct sctp_association *asoc,
 	 struct sock *sk, struct sock *newsk)
 #endif /* CONFIG_SECURITY_NETWORK */
 

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 5c4c5c0602cb..240b92d89852 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h

@@ -1024,9 +1024,9 @@
  * Security hooks for SCTP
  *
  * @sctp_assoc_request:
- *	Passes the @ep and @chunk->skb of the association INIT packet to
+ *	Passes the @asoc and @chunk->skb of the association INIT packet to
  *	the security module.
- *	@ep pointer to sctp endpoint structure.
+ *	@asoc pointer to sctp association structure.
  *	@skb pointer to skbuff of association packet.
  *	Return 0 on success, error on failure.
  * @sctp_bind_connect:

@@ -1044,9 +1044,9 @@
  *	Called whenever a new socket is created by accept(2) (i.e. a TCP
  *	style socket) or when a socket is 'peeled off' e.g userspace
  *	calls sctp_peeloff(3).
- *	@ep pointer to current sctp endpoint structure.
+ *	@asoc pointer to current sctp association structure.
  *	@sk pointer to current sock structure.
- *	@sk pointer to new sock structure.
+ *	@newsk pointer to new sock structure.
  *
  * Security hooks for Infiniband
  *

diff --git a/include/linux/security.h b/include/linux/security.h
index 5b7288521300..a16407444871 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h

@@ -179,7 +179,7 @@ struct xfrm_policy;
 struct xfrm_state;
 struct xfrm_user_sec_ctx;
 struct seq_file;
-struct sctp_endpoint;
+struct sctp_association;
 
 #ifdef CONFIG_MMU
 extern unsigned long mmap_min_addr;

@@ -1418,10 +1418,10 @@ int security_tun_dev_create(void);
 int security_tun_dev_attach_queue(void *security);
 int security_tun_dev_attach(struct sock *sk, void *security);
 int security_tun_dev_open(void *security);
-int security_sctp_assoc_request(struct sctp_endpoint *ep, struct sk_buff *skb);
+int security_sctp_assoc_request(struct sctp_association *asoc, struct sk_buff *skb);
 int security_sctp_bind_connect(struct sock *sk, int optname,
 			       struct sockaddr *address, int addrlen);
-void security_sctp_sk_clone(struct sctp_endpoint *ep, struct sock *sk,
+void security_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk,
 			    struct sock *newsk);
 
 #else	/* CONFIG_SECURITY_NETWORK */

@@ -1624,7 +1624,7 @@ static inline int security_tun_dev_open(void *security)
 	return 0;
 }
 
-static inline int security_sctp_assoc_request(struct sctp_endpoint *ep,
+static inline int security_sctp_assoc_request(struct sctp_association *asoc,
 					      struct sk_buff *skb)
 {
 	return 0;

@@ -1637,7 +1637,7 @@ static inline int security_sctp_bind_connect(struct sock *sk, int optname,
 	return 0;
 }
 
-static inline void security_sctp_sk_clone(struct sctp_endpoint *ep,
+static inline void security_sctp_sk_clone(struct sctp_association *asoc,
 					  struct sock *sk,
 					  struct sock *newsk)
 {

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 651bba654d77..899c29c326ba 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h

@@ -1355,16 +1355,6 @@ struct sctp_endpoint {
 	      reconf_enable:1;
 
 	__u8  strreset_enable;
-
-	/* Security identifiers from incoming (INIT). These are set by
-	 * security_sctp_assoc_request(). These will only be used by
-	 * SCTP TCP type sockets and peeled off connections as they
-	 * cause a new socket to be generated. security_sctp_sk_clone()
-	 * will then plug these into the new socket.
-	 */
-
-	u32 secid;
-	u32 peer_secid;
 };
 
 /* Recover the outter endpoint structure. */

@@ -2104,6 +2094,16 @@ struct sctp_association {
 	__u64 abandoned_unsent[SCTP_PR_INDEX(MAX) + 1];
 	__u64 abandoned_sent[SCTP_PR_INDEX(MAX) + 1];
 
+	/* Security identifiers from incoming (INIT). These are set by
+	 * security_sctp_assoc_request(). These will only be used by
+	 * SCTP TCP type sockets and peeled off connections as they
+	 * cause a new socket to be generated. security_sctp_sk_clone()
+	 * will then plug these into the new socket.
+	 */
+
+	u32 secid;
+	u32 peer_secid;
+
 	struct rcu_head rcu;
 };
 

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index fb3da4d8f4a3..3206374209bc 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c

@@ -326,11 +326,6 @@ enum sctp_disposition sctp_sf_do_5_1B_init(struct net *net,
 	struct sctp_packet *packet;
 	int len;
 
-	/* Update socket peer label if first association. */
-	if (security_sctp_assoc_request((struct sctp_endpoint *)ep,
-					chunk->skb))
-		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
-
 	/* 6.10 Bundling
 	 * An endpoint MUST NOT bundle INIT, INIT ACK or
 	 * SHUTDOWN COMPLETE with any other chunks.

@@ -415,6 +410,12 @@ enum sctp_disposition sctp_sf_do_5_1B_init(struct net *net,
 	if (!new_asoc)
 		goto nomem;
 
+	/* Update socket peer label if first association. */
+	if (security_sctp_assoc_request(new_asoc, chunk->skb)) {
+		sctp_association_free(new_asoc);
+		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
+	}
+
 	if (sctp_assoc_set_bind_addr_from_ep(new_asoc,
 					     sctp_scope(sctp_source(chunk)),
 					     GFP_ATOMIC) < 0)

@@ -780,7 +781,6 @@ enum sctp_disposition sctp_sf_do_5_1D_ce(struct net *net,
 		}
 	}
 
-
 	/* Delay state machine commands until later.
 	 *
 	 * Re-build the bind address for the association is done in

@@ -1517,11 +1517,6 @@ static enum sctp_disposition sctp_sf_do_unexpected_init(
 	struct sctp_packet *packet;
 	int len;
 
-	/* Update socket peer label if first association. */
-	if (security_sctp_assoc_request((struct sctp_endpoint *)ep,
-					chunk->skb))
-		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
-
 	/* 6.10 Bundling
 	 * An endpoint MUST NOT bundle INIT, INIT ACK or
 	 * SHUTDOWN COMPLETE with any other chunks.

@@ -1594,6 +1589,12 @@ static enum sctp_disposition sctp_sf_do_unexpected_init(
 	if (!new_asoc)
 		goto nomem;
 
+	/* Update socket peer label if first association. */
+	if (security_sctp_assoc_request(new_asoc, chunk->skb)) {
+		sctp_association_free(new_asoc);
+		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
+	}
+
 	if (sctp_assoc_set_bind_addr_from_ep(new_asoc,
 				sctp_scope(sctp_source(chunk)), GFP_ATOMIC) < 0)
 		goto nomem;

@@ -2255,8 +2256,7 @@ enum sctp_disposition sctp_sf_do_5_2_4_dupcook(
 	}
 
 	/* Update socket peer label if first association. */
-	if (security_sctp_assoc_request((struct sctp_endpoint *)ep,
-					chunk->skb)) {
+	if (security_sctp_assoc_request(new_asoc, chunk->skb)) {
 		sctp_association_free(new_asoc);
 		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
 	}

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 6b937bfd4751..33391254fa82 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c

@@ -9412,7 +9412,6 @@ void sctp_copy_sock(struct sock *newsk, struct sock *sk,
 	struct inet_sock *inet = inet_sk(sk);
 	struct inet_sock *newinet;
 	struct sctp_sock *sp = sctp_sk(sk);
-	struct sctp_endpoint *ep = sp->ep;
 
 	newsk->sk_type = sk->sk_type;
 	newsk->sk_bound_dev_if = sk->sk_bound_dev_if;

@@ -9457,9 +9456,9 @@ void sctp_copy_sock(struct sock *newsk, struct sock *sk,
 		net_enable_timestamp();
 
 	/* Set newsk security attributes from original sk and connection
-	 * security attribute from ep.
+	 * security attribute from asoc.
 	 */
-	security_sctp_sk_clone(ep, sk, newsk);
+	security_sctp_sk_clone(asoc, sk, newsk);
 }
 
 static inline void sctp_copy_descendant(struct sock *sk_to,

diff --git a/security/security.c b/security/security.c
index 9ffa9e9c5c55..b0f1c007aa3b 100644
--- a/security/security.c
+++ b/security/security.c

@@ -2366,9 +2366,9 @@ int security_tun_dev_open(void *security)
 }
 EXPORT_SYMBOL(security_tun_dev_open);
 
-int security_sctp_assoc_request(struct sctp_endpoint *ep, struct sk_buff *skb)
+int security_sctp_assoc_request(struct sctp_association *asoc, struct sk_buff *skb)
 {
-	return call_int_hook(sctp_assoc_request, 0, ep, skb);
+	return call_int_hook(sctp_assoc_request, 0, asoc, skb);
 }
 EXPORT_SYMBOL(security_sctp_assoc_request);
 

@@ -2380,10 +2380,10 @@ int security_sctp_bind_connect(struct sock *sk, int optname,
 }
 EXPORT_SYMBOL(security_sctp_bind_connect);
 
-void security_sctp_sk_clone(struct sctp_endpoint *ep, struct sock *sk,
+void security_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk,
 			    struct sock *newsk)
 {
-	call_void_hook(sctp_sk_clone, ep, sk, newsk);
+	call_void_hook(sctp_sk_clone, asoc, sk, newsk);
 }
 EXPORT_SYMBOL(security_sctp_sk_clone);
 

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e7ebd45ca345..f025fc00421b 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c

@@ -5356,10 +5356,10 @@ static void selinux_sock_graft(struct sock *sk, struct socket *parent)
  * connect(2), sctp_connectx(3) or sctp_sendmsg(3) (with no association
  * already present).
  */
-static int selinux_sctp_assoc_request(struct sctp_endpoint *ep,
+static int selinux_sctp_assoc_request(struct sctp_association *asoc,
 				      struct sk_buff *skb)
 {
-	struct sk_security_struct *sksec = ep->base.sk->sk_security;
+	struct sk_security_struct *sksec = asoc->base.sk->sk_security;
 	struct common_audit_data ad;
 	struct lsm_network_audit net = {0,};
 	u8 peerlbl_active;

@@ -5376,7 +5376,7 @@ static int selinux_sctp_assoc_request(struct sctp_endpoint *ep,
 		/* This will return peer_sid = SECSID_NULL if there are
 		 * no peer labels, see security_net_peersid_resolve().
 		 */
-		err = selinux_skb_peerlbl_sid(skb, ep->base.sk->sk_family,
+		err = selinux_skb_peerlbl_sid(skb, asoc->base.sk->sk_family,
 					      &peer_sid);
 		if (err)
 			return err;

@@ -5400,7 +5400,7 @@ static int selinux_sctp_assoc_request(struct sctp_endpoint *ep,
 		 */
 		ad.type = LSM_AUDIT_DATA_NET;
 		ad.u.net = &net;
-		ad.u.net->sk = ep->base.sk;
+		ad.u.net->sk = asoc->base.sk;
 		err = avc_has_perm(&selinux_state,
 				   sksec->peer_sid, peer_sid, sksec->sclass,
 				   SCTP_SOCKET__ASSOCIATION, &ad);

@@ -5418,11 +5418,11 @@ static int selinux_sctp_assoc_request(struct sctp_endpoint *ep,
 	if (err)
 		return err;
 
-	ep->secid = conn_sid;
-	ep->peer_secid = peer_sid;
+	asoc->secid = conn_sid;
+	asoc->peer_secid = peer_sid;
 
 	/* Set any NetLabel labels including CIPSO/CALIPSO options. */
-	return selinux_netlbl_sctp_assoc_request(ep, skb);
+	return selinux_netlbl_sctp_assoc_request(asoc, skb);
 }
 
 /* Check if sctp IPv4/IPv6 addresses are valid for binding or connecting

@@ -5507,7 +5507,7 @@ static int selinux_sctp_bind_connect(struct sock *sk, int optname,
 }
 
 /* Called whenever a new socket is created by accept(2) or sctp_peeloff(3). */
-static void selinux_sctp_sk_clone(struct sctp_endpoint *ep, struct sock *sk,
+static void selinux_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk,
 				  struct sock *newsk)
 {
 	struct sk_security_struct *sksec = sk->sk_security;

@@ -5519,8 +5519,8 @@ static void selinux_sctp_sk_clone(struct sctp_endpoint *ep, struct sock *sk,
 	if (!selinux_policycap_extsockclass())
 		return selinux_sk_clone_security(sk, newsk);
 
-	newsksec->sid = ep->secid;
-	newsksec->peer_sid = ep->peer_secid;
+	newsksec->sid = asoc->secid;
+	newsksec->peer_sid = asoc->peer_secid;
 	newsksec->sclass = sksec->sclass;
 	selinux_netlbl_sctp_sk_clone(sk, newsk);
 }

diff --git a/security/selinux/include/netlabel.h b/security/selinux/include/netlabel.h
index 0c58f62dc6ab..4d0456d3d459 100644
--- a/security/selinux/include/netlabel.h
+++ b/security/selinux/include/netlabel.h

@@ -39,7 +39,7 @@ int selinux_netlbl_skbuff_getsid(struct sk_buff *skb,
 int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
 				 u16 family,
 				 u32 sid);
-int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
+int selinux_netlbl_sctp_assoc_request(struct sctp_association *asoc,
 				     struct sk_buff *skb);
 int selinux_netlbl_inet_conn_request(struct request_sock *req, u16 family);
 void selinux_netlbl_inet_csk_clone(struct sock *sk, u16 family);

@@ -98,7 +98,7 @@ static inline int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
 	return 0;
 }
 
-static inline int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
+static inline int selinux_netlbl_sctp_assoc_request(struct sctp_association *asoc,
 						    struct sk_buff *skb)
 {
 	return 0;

diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c
index abaab7683840..43d72f776a7d 100644
--- a/security/selinux/netlabel.c
+++ b/security/selinux/netlabel.c

@@ -268,22 +268,22 @@ int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
  * Returns zero on success, negative values on failure.
  *
  */
-int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
+int selinux_netlbl_sctp_assoc_request(struct sctp_association *asoc,
 				     struct sk_buff *skb)
 {
 	int rc;
 	struct netlbl_lsm_secattr secattr;
-	struct sk_security_struct *sksec = ep->base.sk->sk_security;
+	struct sk_security_struct *sksec = asoc->base.sk->sk_security;
 	struct sockaddr_in addr4;
 	struct sockaddr_in6 addr6;
 
-	if (ep->base.sk->sk_family != PF_INET &&
-				ep->base.sk->sk_family != PF_INET6)
+	if (asoc->base.sk->sk_family != PF_INET &&
+	    asoc->base.sk->sk_family != PF_INET6)
 		return 0;
 
 	netlbl_secattr_init(&secattr);
 	rc = security_netlbl_sid_to_secattr(&selinux_state,
-					    ep->secid, &secattr);
+					    asoc->secid, &secattr);
 	if (rc != 0)
 		goto assoc_request_return;
 

@@ -293,11 +293,11 @@ int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
 	if (ip_hdr(skb)->version == 4) {
 		addr4.sin_family = AF_INET;
 		addr4.sin_addr.s_addr = ip_hdr(skb)->saddr;
-		rc = netlbl_conn_setattr(ep->base.sk, (void *)&addr4, &secattr);
+		rc = netlbl_conn_setattr(asoc->base.sk, (void *)&addr4, &secattr);
 	} else if (IS_ENABLED(CONFIG_IPV6) && ip_hdr(skb)->version == 6) {
 		addr6.sin6_family = AF_INET6;
 		addr6.sin6_addr = ipv6_hdr(skb)->saddr;
-		rc = netlbl_conn_setattr(ep->base.sk, (void *)&addr6, &secattr);
+		rc = netlbl_conn_setattr(asoc->base.sk, (void *)&addr6, &secattr);
 	} else {
 		rc = -EAFNOSUPPORT;
 	}

-- 
2.27.0

The asoc created when receives the INIT chunk is a temporary one, it
will be delete after INIT_ACK chunk is replied. So for the real asoc
created in sctp_sf_do_5_1D_ce() when receives the COOKIE_ECHO chunk,
security_sctp_assoc_request() should also be called.

Fixes: 72e89f50084c ("security: Add support for SCTP security hooks")
Reported-by: Prashanth Prahlad <redacted>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 Documentation/security/SCTP.rst | 15 +++++++++------
 net/sctp/sm_statefuns.c         |  5 +++++
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/Documentation/security/SCTP.rst b/Documentation/security/SCTP.rst
index 415b548d9ce0..9a38067762e5 100644
--- a/Documentation/security/SCTP.rst
+++ b/Documentation/security/SCTP.rst

@@ -151,9 +151,9 @@ establishing an association.
          INIT --------------------------------------------->
                                                    sctp_sf_do_5_1B_init()
                                                  Respond to an INIT chunk.
-                                             SCTP peer endpoint "A" is
-                                             asking for an association. Call
-                                             security_sctp_assoc_request()
+                                             SCTP peer endpoint "A" is asking
+                                             for an temporary association.
+                                             Call security_sctp_assoc_request()
                                              to set the peer label if first
                                              association.
                                              If not first association, check

@@ -163,9 +163,12 @@ establishing an association.
           |                                       discard the packet.
           |
     COOKIE ECHO ------------------------------------------>
-                                                          |
-                                                          |
-                                                          |
+                                                  sctp_sf_do_5_1D_ce()
+                                             Respond to an COOKIE ECHO chunk.
+                                             Confirm the cookie and create an
+                                             permanent association.
+                                             Call security_sctp_assoc_request() to
+                                             do the same as for INIT chunk Response.
           <------------------------------------------- COOKIE ACK
           |                                               |
     sctp_sf_do_5_1E_ca                                    |

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 3206374209bc..b818532c3fc2 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c

@@ -781,6 +781,11 @@ enum sctp_disposition sctp_sf_do_5_1D_ce(struct net *net,
 		}
 	}
 
+	if (security_sctp_assoc_request(new_asoc, chunk->skb)) {
+		sctp_association_free(new_asoc);
+		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
+	}
+
 	/* Delay state machine commands until later.
 	 *
 	 * Re-build the bind address for the association is done in

-- 
2.27.0

security_sctp_assoc_established() is added to replace
security_inet_conn_established() called in
sctp_sf_do_5_1E_ca(), so that asoc can be accessed in security
subsystem and save the peer secid to asoc->peer_secid.

Fixes: 72e89f50084c ("security: Add support for SCTP security hooks")
Reported-by: Prashanth Prahlad <redacted>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 Documentation/security/SCTP.rst | 22 ++++++++++------------
 include/linux/lsm_hook_defs.h   |  2 ++
 include/linux/lsm_hooks.h       |  5 +++++
 include/linux/security.h        |  8 ++++++++
 net/sctp/sm_statefuns.c         |  2 +-
 security/security.c             |  7 +++++++
 6 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/Documentation/security/SCTP.rst b/Documentation/security/SCTP.rst
index 9a38067762e5..3ebbcd80b3e7 100644
--- a/Documentation/security/SCTP.rst
+++ b/Documentation/security/SCTP.rst

@@ -15,10 +15,7 @@ For security module support, three SCTP specific hooks have been implemented::
     security_sctp_assoc_request()
     security_sctp_bind_connect()
     security_sctp_sk_clone()
-
-Also the following security hook has been utilised::
-
-    security_inet_conn_established()
+    security_sctp_assoc_established()
 
 The usage of these hooks are described below with the SELinux implementation
 described in the `SCTP SELinux Support`_ chapter.

@@ -122,11 +119,12 @@ calls **sctp_peeloff**\(3).
     @newsk - pointer to new sock structure.
 
 
-security_inet_conn_established()
+security_sctp_assoc_established()
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-Called when a COOKIE ACK is received::
+Called when a COOKIE ACK is received, and the peer secid will be
+saved into ``@asoc->peer_secid`` for client::
 
-    @sk  - pointer to sock structure.
+    @asoc - pointer to sctp association structure.
     @skb - pointer to skbuff of the COOKIE ACK packet.
 
 

@@ -134,7 +132,7 @@ Security Hooks used for Association Establishment
 -------------------------------------------------
 
 The following diagram shows the use of ``security_sctp_bind_connect()``,
-``security_sctp_assoc_request()``, ``security_inet_conn_established()`` when
+``security_sctp_assoc_request()``, ``security_sctp_assoc_established()`` when
 establishing an association.
 ::
 

@@ -172,7 +170,7 @@ establishing an association.
           <------------------------------------------- COOKIE ACK
           |                                               |
     sctp_sf_do_5_1E_ca                                    |
- Call security_inet_conn_established()                    |
+ Call security_sctp_assoc_established()                   |
  to set the peer label.                                   |
           |                                               |
           |                               If SCTP_SOCKET_TCP or peeled off

@@ -198,7 +196,7 @@ hooks with the SELinux specifics expanded below::
     security_sctp_assoc_request()
     security_sctp_bind_connect()
     security_sctp_sk_clone()
-    security_inet_conn_established()
+    security_sctp_assoc_established()
 
 
 security_sctp_assoc_request()

@@ -271,12 +269,12 @@ sockets sid and peer sid to that contained in the ``@asoc sid`` and
     @newsk - pointer to new sock structure.
 
 
-security_inet_conn_established()
+security_sctp_assoc_established()
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Called when a COOKIE ACK is received where it sets the connection's peer sid
 to that in ``@skb``::
 
-    @sk  - pointer to sock structure.
+    @asoc - pointer to sctp association structure.
     @skb - pointer to skbuff of the COOKIE ACK packet.
 
 

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 0024273a7382..e9870118cc67 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h

@@ -334,6 +334,8 @@ LSM_HOOK(int, 0, sctp_bind_connect, struct sock *sk, int optname,
 	 struct sockaddr *address, int addrlen)
 LSM_HOOK(void, LSM_RET_VOID, sctp_sk_clone, struct sctp_association *asoc,
 	 struct sock *sk, struct sock *newsk)
+LSM_HOOK(void, LSM_RET_VOID, sctp_assoc_established, struct sctp_association *asoc,
+	 struct sk_buff *skb)
 #endif /* CONFIG_SECURITY_NETWORK */
 
 #ifdef CONFIG_SECURITY_INFINIBAND

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 240b92d89852..ba42c22204e2 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h

@@ -1047,6 +1047,11 @@
  *	@asoc pointer to current sctp association structure.
  *	@sk pointer to current sock structure.
  *	@newsk pointer to new sock structure.
+ * @sctp_assoc_established:
+ *	Passes the @asoc and @chunk->skb of the association COOKIE_ACK packet
+ *	to the security module.
+ *	@asoc pointer to sctp association structure.
+ *	@skb pointer to skbuff of association packet.
  *
  * Security hooks for Infiniband
  *

diff --git a/include/linux/security.h b/include/linux/security.h
index a16407444871..11cdddf9685c 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h

@@ -1423,6 +1423,8 @@ int security_sctp_bind_connect(struct sock *sk, int optname,
 			       struct sockaddr *address, int addrlen);
 void security_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk,
 			    struct sock *newsk);
+void security_sctp_assoc_established(struct sctp_association *asoc,
+				     struct sk_buff *skb);
 
 #else	/* CONFIG_SECURITY_NETWORK */
 static inline int security_unix_stream_connect(struct sock *sock,

@@ -1642,6 +1644,12 @@ static inline void security_sctp_sk_clone(struct sctp_association *asoc,
 					  struct sock *newsk)
 {
 }
+
+static inline void security_sctp_assoc_established(struct sctp_association *asoc,
+						   struct sk_buff *skb)
+{
+	return 0;
+}
 #endif	/* CONFIG_SECURITY_NETWORK */
 
 #ifdef CONFIG_SECURITY_INFINIBAND

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index b818532c3fc2..5fabaa54b77d 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c

@@ -946,7 +946,7 @@ enum sctp_disposition sctp_sf_do_5_1E_ca(struct net *net,
 	sctp_add_cmd_sf(commands, SCTP_CMD_INIT_COUNTER_RESET, SCTP_NULL());
 
 	/* Set peer label for connection. */
-	security_inet_conn_established(ep->base.sk, chunk->skb);
+	security_sctp_assoc_established((struct sctp_association *)asoc, chunk->skb);
 
 	/* RFC 2960 5.1 Normal Establishment of an Association
 	 *

diff --git a/security/security.c b/security/security.c
index b0f1c007aa3b..4b2b4b5beb27 100644
--- a/security/security.c
+++ b/security/security.c

@@ -2387,6 +2387,13 @@ void security_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk,
 }
 EXPORT_SYMBOL(security_sctp_sk_clone);
 
+void security_sctp_assoc_established(struct sctp_association *asoc,
+				     struct sk_buff *skb)
+{
+	call_void_hook(sctp_assoc_established, asoc, skb);
+}
+EXPORT_SYMBOL(security_sctp_assoc_established);
+
 #endif	/* CONFIG_SECURITY_NETWORK */
 
 #ifdef CONFIG_SECURITY_INFINIBAND

-- 
2.27.0

Different from selinux_inet_conn_established(), it also gives the
secid to asoc->peer_secid in selinux_sctp_assoc_established(),
as one UDP-type socket may have more than one asocs.

Note that peer_secid in asoc will save the peer secid for this
asoc connection, and peer_sid in sksec will just keep the peer
secid for the latest connection. So the right use should be do
peeloff for UDP-type socket if there will be multiple asocs in
one socket, so that the peeloff socket has the right label for
its asoc.

Fixes: 72e89f50084c ("security: Add support for SCTP security hooks")
Reported-by: Prashanth Prahlad <redacted>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 security/selinux/hooks.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index f025fc00421b..793fdcbc68bd 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c

@@ -5525,6 +5525,21 @@ static void selinux_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk
 	selinux_netlbl_sctp_sk_clone(sk, newsk);
 }
 
+static void selinux_sctp_assoc_established(struct sctp_association *asoc,
+					   struct sk_buff *skb)
+{
+	struct sk_security_struct *sksec = asoc->base.sk->sk_security;
+	u16 family = asoc->base.sk->sk_family;
+
+	/* handle mapped IPv4 packets arriving via IPv6 sockets */
+	if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP))
+		family = PF_INET;
+
+	selinux_skb_peerlbl_sid(skb, family, &sksec->peer_sid);
+	asoc->secid = sksec->sid;
+	asoc->peer_secid = sksec->peer_sid;
+}
+
 static int selinux_inet_conn_request(const struct sock *sk, struct sk_buff *skb,
 				     struct request_sock *req)
 {

@@ -7290,6 +7305,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(sctp_assoc_request, selinux_sctp_assoc_request),
 	LSM_HOOK_INIT(sctp_sk_clone, selinux_sctp_sk_clone),
 	LSM_HOOK_INIT(sctp_bind_connect, selinux_sctp_bind_connect),
+	LSM_HOOK_INIT(sctp_assoc_established, selinux_sctp_assoc_established),
 	LSM_HOOK_INIT(inet_conn_request, selinux_inet_conn_request),
 	LSM_HOOK_INIT(inet_csk_clone, selinux_inet_csk_clone),
 	LSM_HOOK_INIT(inet_conn_established, selinux_inet_conn_established),

-- 
2.27.0

On Fri, 22 Oct 2021 02:36:09 -0400 Xin Long wrote:

This patch is to move secid and peer_secid from endpoint to association,
and pass asoc to sctp_assoc_request and sctp_sk_clone instead of ep. As
ep is the local endpoint and asoc represents a connection, and in SCTP
one sk/ep could have multiple asoc/connection, saving secid/peer_secid
for new asoc will overwrite the old asoc's.

Note that since asoc can be passed as NULL, security_sctp_assoc_request()
is moved to the place right after the new_asoc is created in
sctp_sf_do_5_1B_init() and sctp_sf_do_unexpected_init().

Fixes: 72e89f50084c ("security: Add support for SCTP security hooks")
Reported-by: Prashanth Prahlad <redacted>
Signed-off-by: Xin Long <lucien.xin@gmail.com>

missed one?

security/selinux/netlabel.c:274: warning: Function parameter or member
'asoc' not described in 'selinux_netlbl_sctp_assoc_request'
security/selinux/netlabel.c:274: warning: Excess function parameter 'ep' description in 'selinux_netlbl_sctp_assoc_request'

On Fri, Oct 22, 2021 at 11:36 PM Jakub Kicinski [off-list ref] wrote:

On Fri, 22 Oct 2021 02:36:09 -0400 Xin Long wrote:

quoted

This patch is to move secid and peer_secid from endpoint to association,
and pass asoc to sctp_assoc_request and sctp_sk_clone instead of ep. As
ep is the local endpoint and asoc represents a connection, and in SCTP
one sk/ep could have multiple asoc/connection, saving secid/peer_secid
for new asoc will overwrite the old asoc's.

Note that since asoc can be passed as NULL, security_sctp_assoc_request()
is moved to the place right after the new_asoc is created in
sctp_sf_do_5_1B_init() and sctp_sf_do_unexpected_init().

Fixes: 72e89f50084c ("security: Add support for SCTP security hooks")
Reported-by: Prashanth Prahlad <redacted>
Signed-off-by: Xin Long <lucien.xin@gmail.com>

missed one?

security/selinux/netlabel.c:274: warning: Function parameter or member
'asoc' not described in 'selinux_netlbl_sctp_assoc_request'
security/selinux/netlabel.c:274: warning: Excess function parameter 'ep' description in 'selinux_netlbl_sctp_assoc_request'

Yup, the function description also needs fixing:

@@ -260,11 +260,11 @@ int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,

 /**
  * selinux_netlbl_sctp_assoc_request - Label an incoming sctp association.
- * @ep: incoming association endpoint.
+ * @asoc: incoming association.
  * @skb: the packet.
  *
  * Description:
- * A new incoming connection is represented by @ep, ......
+ * A new incoming connection is represented by @asoc, ......
  * Returns zero on success, negative values on failure.
  *
  */

Thanks.

On Fri, 2021-10-22 at 02:36 -0400, Xin Long wrote:

There are a couple of problems in the currect security hooks in sctp:

1. The hooks incorrectly treat sctp_endpoint in SCTP as request_sock in
   TCP, while it's in fact no more than an extension of the sock, and
   represents the local host. It is created when sock is created, not
   when a conn request comes. sctp_association is actually the correct
   one to represent the connection, and created when a conn request
   arrives.

2. security_sctp_assoc_request() hook should also be called in
processing
   COOKIE ECHO, as that's the place where the real assoc is created and
   used in the future.

The problems above may cause accept sk, peeloff sk or client sk having
the incorrect security labels.

So this patchset is to change some hooks and pass asoc into them and
save
these secids into asoc, as well as add the missing sctp_assoc_request
hook into the COOKIE ECHO processing.

I've built this patchset on kernel 5.15-rc5 with no problems.
I tested this using the SELinux testsuite with Ondrej's "[PATCH
testsuite] tests/sctp: add client peeloff tests" [1] added. All SCTP
tests ran with no errors. Also ran the sctp-tests from [2] with no
errors.

[1]
https://lore.kernel.org/selinux/20211021144543.740762-1-omosnace@redhat.com/
[2] https://github.com/sctp/sctp-tests.git

Reviewed-by: Richard Haines <redacted>
Tested-by: Richard Haines <redacted>

Xin Long (4):
  security: pass asoc to sctp_assoc_request and sctp_sk_clone
  security: call security_sctp_assoc_request in sctp_sf_do_5_1D_ce
  security: add sctp_assoc_established hook
  security: implement sctp_assoc_established hook in selinux

 Documentation/security/SCTP.rst     | 65 +++++++++++++++--------------
 include/linux/lsm_hook_defs.h       |  6 ++-
 include/linux/lsm_hooks.h           | 13 ++++--
 include/linux/security.h            | 18 +++++---
 include/net/sctp/structs.h          | 20 ++++-----
 net/sctp/sm_statefuns.c             | 31 ++++++++------
 net/sctp/socket.c                   |  5 +--
 security/security.c                 | 15 +++++--
 security/selinux/hooks.c            | 36 +++++++++++-----
 security/selinux/include/netlabel.h |  4 +-
 security/selinux/netlabel.c         | 14 +++----
 11 files changed, 135 insertions(+), 92 deletions(-)

On Fri, 2021-10-22 at 02:36 -0400, Xin Long wrote:

I found another nit where ep needs changing:

diff -ur a/security/selinux/hooks.c b/security/selinux/hooks.c

--- a/security/selinux/hooks.c	2021-10-22 12:29:23.771796000 +0100
+++ b/security/selinux/hooks.c	2021-10-23 09:39:40.514988202 +0100

@@ -5409,7 +5409,7 @@
 	}
 
 	/* Compute the MLS component for the connection and store
-	 * the information in ep. This will be used by SCTP TCP type
+	 * the information in asoc. This will be used by SCTP TCP type
 	 * sockets and peeled off connections as they cause a new
 	 * socket to be generated. selinux_sctp_sk_clone() will then
 	 * plug this into the new socket.

Hi Xin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net/master]

url:    https://github.com/0day-ci/linux/commits/Xin-Long/security-fixups-for-the-security-hooks-in-sctp/20211022-143827
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 397430b50a363d8b7bdda00522123f82df6adc5e
config: hexagon-buildonly-randconfig-r006-20211024 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project a709787cd988aaca847995bd08cc9348c9c6c956)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/32fba59611e67404b515f7864aa67a3abd2f7978
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Xin-Long/security-fixups-for-the-security-hooks-in-sctp/20211022-143827
        git checkout 32fba59611e67404b515f7864aa67a3abd2f7978
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=hexagon 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>

All error/warnings (new ones prefixed by >>):

   In file included from fs/open.c:19:

quoted

include/linux/security.h:1651:2: error: void function 'security_sctp_assoc_established' should not return a value [-Wreturn-type]

           return 0;
           ^      ~
   1 error generated.
--
   In file included from fs/pipe.c:17:
   In file included from include/linux/pseudo_fs.h:4:
   In file included from include/linux/fs_context.h:14:

quoted

include/linux/security.h:1651:2: error: void function 'security_sctp_assoc_established' should not return a value [-Wreturn-type]

           return 0;
           ^      ~
   fs/pipe.c:755:15: warning: no previous prototype for function 'account_pipe_buffers' [-Wmissing-prototypes]
   unsigned long account_pipe_buffers(struct user_struct *user,
                 ^
   fs/pipe.c:755:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   unsigned long account_pipe_buffers(struct user_struct *user,
   ^
   static 
   fs/pipe.c:761:6: warning: no previous prototype for function 'too_many_pipe_buffers_soft' [-Wmissing-prototypes]
   bool too_many_pipe_buffers_soft(unsigned long user_bufs)
        ^
   fs/pipe.c:761:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   bool too_many_pipe_buffers_soft(unsigned long user_bufs)
   ^
   static 
   fs/pipe.c:768:6: warning: no previous prototype for function 'too_many_pipe_buffers_hard' [-Wmissing-prototypes]
   bool too_many_pipe_buffers_hard(unsigned long user_bufs)
        ^
   fs/pipe.c:768:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   bool too_many_pipe_buffers_hard(unsigned long user_bufs)
   ^
   static 
   fs/pipe.c:775:6: warning: no previous prototype for function 'pipe_is_unprivileged_user' [-Wmissing-prototypes]
   bool pipe_is_unprivileged_user(void)
        ^
   fs/pipe.c:775:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   bool pipe_is_unprivileged_user(void)
   ^
   static 
   fs/pipe.c:1245:5: warning: no previous prototype for function 'pipe_resize_ring' [-Wmissing-prototypes]
   int pipe_resize_ring(struct pipe_inode_info *pipe, unsigned int nr_slots)
       ^
   fs/pipe.c:1245:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int pipe_resize_ring(struct pipe_inode_info *pipe, unsigned int nr_slots)
   ^
   static 
   5 warnings and 1 error generated.
--
   In file included from fs/d_path.c:2:
   In file included from include/linux/syscalls.h:87:
   In file included from include/trace/syscall.h:7:
   In file included from include/linux/trace_events.h:10:
   In file included from include/linux/perf_event.h:59:

quoted

include/linux/security.h:1651:2: error: void function 'security_sctp_assoc_established' should not return a value [-Wreturn-type]

           return 0;
           ^      ~
   fs/d_path.c:320:7: warning: no previous prototype for function 'simple_dname' [-Wmissing-prototypes]
   char *simple_dname(struct dentry *dentry, char *buffer, int buflen)
         ^
   fs/d_path.c:320:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   char *simple_dname(struct dentry *dentry, char *buffer, int buflen)
   ^
   static 
   1 warning and 1 error generated.
--
   In file included from fs/statfs.c:2:
   In file included from include/linux/syscalls.h:87:
   In file included from include/trace/syscall.h:7:
   In file included from include/linux/trace_events.h:10:
   In file included from include/linux/perf_event.h:59:

quoted

include/linux/security.h:1651:2: error: void function 'security_sctp_assoc_established' should not return a value [-Wreturn-type]

           return 0;
           ^      ~

quoted

fs/statfs.c:131:3: warning: 'memcpy' will always overflow; destination buffer has size 64, but size argument is 88 [-Wfortify-source]

                   memcpy(&buf, st, sizeof(*st));
                   ^
   1 warning and 1 error generated.
--
   In file included from ipc/msg.c:33:

quoted

include/linux/security.h:1651:2: error: void function 'security_sctp_assoc_established' should not return a value [-Wreturn-type]

           return 0;
           ^      ~

quoted

ipc/msg.c:496:20: warning: implicit conversion from 'int' to 'unsigned short' changes value from 32768000 to 0 [-Wconstant-conversion]

           msginfo->msgseg = MSGSEG;
                           ~ ^~~~~~
   include/uapi/linux/msg.h:87:38: note: expanded from macro 'MSGSEG'
   #define MSGSEG (__MSGSEG <= 0xffff ? __MSGSEG : 0xffff)
                                        ^~~~~~~~
   include/uapi/linux/msg.h:86:36: note: expanded from macro '__MSGSEG'
   #define __MSGSEG ((MSGPOOL * 1024) / MSGSSZ) /* max no. of segments */
                     ~~~~~~~~~~~~~~~~~^~~~~~~~
   1 warning and 1 error generated.
--
   In file included from kernel/printk/printk.c:34:

quoted

include/linux/security.h:1651:2: error: void function 'security_sctp_assoc_established' should not return a value [-Wreturn-type]

           return 0;
           ^      ~
   kernel/printk/printk.c:175:5: warning: no previous prototype for function 'devkmsg_sysctl_set_loglvl' [-Wmissing-prototypes]
   int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write,
       ^
   kernel/printk/printk.c:175:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write,
   ^
   static 
   1 warning and 1 error generated.
--
   In file included from fs/afs/dir.c:16:
   In file included from fs/afs/internal.h:25:
   In file included from include/net/sock.h:46:
   In file included from include/linux/netdevice.h:45:
   In file included from include/uapi/linux/neighbour.h:6:
   In file included from include/linux/netlink.h:9:
   In file included from include/net/scm.h:8:

quoted

include/linux/security.h:1651:2: error: void function 'security_sctp_assoc_established' should not return a value [-Wreturn-type]

           return 0;
           ^      ~
   fs/afs/dir.c:164:11: warning: format specifies type 'unsigned short' but the argument has type 'int' [-Wformat]
                                  ntohs(dbuf->blocks[tmp].hdr.magic));
                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/printk.h:446:60: note: expanded from macro 'printk'
   #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
                                                       ~~~    ^~~~~~~~~~~
   include/linux/printk.h:418:19: note: expanded from macro 'printk_index_wrap'
                   _p_func(_fmt, ##__VA_ARGS__);                           \
                           ~~~~    ^~~~~~~~~~~
   include/linux/byteorder/generic.h:142:18: note: expanded from macro 'ntohs'
   #define ntohs(x) ___ntohs(x)
                    ^~~~~~~~~~~
   include/linux/byteorder/generic.h:137:21: note: expanded from macro '___ntohs'
   #define ___ntohs(x) __be16_to_cpu(x)
                       ^~~~~~~~~~~~~~~~
   include/uapi/linux/byteorder/little_endian.h:42:26: note: expanded from macro '__be16_to_cpu'
   #define __be16_to_cpu(x) __swab16((__force __u16)(__be16)(x))
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/uapi/linux/swab.h:105:2: note: expanded from macro '__swab16'
           (__builtin_constant_p((__u16)(x)) ?     \
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 warning and 1 error generated.
--
   In file included from drivers/char/mem.c:25:
   In file included from include/linux/shmem_fs.h:11:
   In file included from include/linux/fs_parser.h:11:
   In file included from include/linux/fs_context.h:14:

quoted

include/linux/security.h:1651:2: error: void function 'security_sctp_assoc_established' should not return a value [-Wreturn-type]

           return 0;
           ^      ~
   drivers/char/mem.c:95:13: warning: no previous prototype for function 'unxlate_dev_mem_ptr' [-Wmissing-prototypes]
   void __weak unxlate_dev_mem_ptr(phys_addr_t phys, void *addr)
               ^
   drivers/char/mem.c:94:29: note: expanded from macro 'unxlate_dev_mem_ptr'
   #define unxlate_dev_mem_ptr unxlate_dev_mem_ptr
                               ^
   drivers/char/mem.c:95:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void __weak unxlate_dev_mem_ptr(phys_addr_t phys, void *addr)
   ^
   static 
   1 warning and 1 error generated.
--
   In file included from drivers/char/random.c:335:
   In file included from include/linux/syscalls.h:87:
   In file included from include/trace/syscall.h:7:
   In file included from include/linux/trace_events.h:10:
   In file included from include/linux/perf_event.h:59:

quoted

include/linux/security.h:1651:2: error: void function 'security_sctp_assoc_established' should not return a value [-Wreturn-type]

           return 0;
           ^      ~

quoted

drivers/char/random.c:1257:41: warning: shift count >= width of type [-Wshift-count-overflow]

           c_high = (sizeof(cycles) > 4) ? cycles >> 32 : 0;
                                                  ^  ~~
   drivers/char/random.c:1258:35: warning: shift count >= width of type [-Wshift-count-overflow]
           j_high = (sizeof(now) > 4) ? now >> 32 : 0;
                                            ^  ~~
   drivers/char/random.c:2272:6: warning: no previous prototype for function 'add_hwgenerator_randomness' [-Wmissing-prototypes]
   void add_hwgenerator_randomness(const char *buffer, size_t count,
        ^
   drivers/char/random.c:2272:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void add_hwgenerator_randomness(const char *buffer, size_t count,
   ^
   static 
   3 warnings and 1 error generated.
--
   In file included from fs/cifs/ioctl.c:16:
   In file included from fs/cifs/cifspdu.h:12:
   In file included from include/net/sock.h:46:
   In file included from include/linux/netdevice.h:45:
   In file included from include/uapi/linux/neighbour.h:6:
   In file included from include/linux/netlink.h:9:
   In file included from include/net/scm.h:8:

quoted

include/linux/security.h:1651:2: error: void function 'security_sctp_assoc_established' should not return a value [-Wreturn-type]

           return 0;
           ^      ~
   fs/cifs/ioctl.c:324:10: warning: variable 'caps' set but not used [-Wunused-but-set-variable]
           __u64   caps;
                   ^
   1 warning and 1 error generated.
--
   In file included from fs/kernfs/file.c:19:
   In file included from fs/kernfs/kernfs-internal.h:20:
   In file included from include/linux/fs_context.h:14:

quoted

include/linux/security.h:1651:2: error: void function 'security_sctp_assoc_established' should not return a value [-Wreturn-type]

           return 0;
           ^      ~
   fs/kernfs/file.c:128:15: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
                   return NULL + !*ppos;
                          ~~~~ ^
   1 warning and 1 error generated.


vim +/security_sctp_assoc_established +1651 include/linux/security.h

  1647	
  1648	static inline void security_sctp_assoc_established(struct sctp_association *asoc,
  1649							   struct sk_buff *skb)
  1650	{

1651		return 0;

  1652	}
  1653	#endif	/* CONFIG_SECURITY_NETWORK */
  1654	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Hi Xin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net/master]

url:    https://github.com/0day-ci/linux/commits/Xin-Long/security-fixups-for-the-security-hooks-in-sctp/20211022-143827
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 397430b50a363d8b7bdda00522123f82df6adc5e
config: hexagon-randconfig-r041-20211025 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project a461fa64bb37cffd73f683c74f6b0780379fc2ca)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/32fba59611e67404b515f7864aa67a3abd2f7978
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Xin-Long/security-fixups-for-the-security-hooks-in-sctp/20211022-143827
        git checkout 32fba59611e67404b515f7864aa67a3abd2f7978
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=hexagon 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>

All warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/vgem/vgem_drv.c:36:
   In file included from include/linux/shmem_fs.h:11:
   In file included from include/linux/fs_parser.h:11:
   In file included from include/linux/fs_context.h:14:
   include/linux/security.h:1651:2: error: void function 'security_sctp_assoc_established' should not return a value [-Wreturn-type]
           return 0;
           ^      ~

quoted

drivers/gpu/drm/vgem/vgem_drv.c:460:10: warning: shift count >= width of type [-Wshift-count-overflow]

                                        DMA_BIT_MASK(64));
                                        ^~~~~~~~~~~~~~~~
   include/linux/dma-mapping.h:76:54: note: expanded from macro 'DMA_BIT_MASK'
   #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
                                                        ^ ~~~
   1 warning and 1 error generated.


vim +460 drivers/gpu/drm/vgem/vgem_drv.c

502e95c6678505 Zach Reizner  2015-03-04  444  
502e95c6678505 Zach Reizner  2015-03-04  445  static int __init vgem_init(void)
502e95c6678505 Zach Reizner  2015-03-04  446  {
502e95c6678505 Zach Reizner  2015-03-04  447  	int ret;
bcc0ef7f57e51e Daniel Vetter 2020-09-09  448  	struct platform_device *pdev;
502e95c6678505 Zach Reizner  2015-03-04  449  
bcc0ef7f57e51e Daniel Vetter 2020-09-09  450  	pdev = platform_device_register_simple("vgem", -1, NULL, 0);
bcc0ef7f57e51e Daniel Vetter 2020-09-09  451  	if (IS_ERR(pdev))
bcc0ef7f57e51e Daniel Vetter 2020-09-09  452  		return PTR_ERR(pdev);
e2aff44868ae60 Laura Abbott  2017-05-04  453  
bcc0ef7f57e51e Daniel Vetter 2020-09-09  454  	if (!devres_open_group(&pdev->dev, NULL, GFP_KERNEL)) {
bcc0ef7f57e51e Daniel Vetter 2020-09-09  455  		ret = -ENOMEM;
bcc0ef7f57e51e Daniel Vetter 2020-09-09  456  		goto out_unregister;
502e95c6678505 Zach Reizner  2015-03-04  457  	}
502e95c6678505 Zach Reizner  2015-03-04  458  
bcc0ef7f57e51e Daniel Vetter 2020-09-09  459  	dma_coerce_mask_and_coherent(&pdev->dev,
e2aff44868ae60 Laura Abbott  2017-05-04 @460  				     DMA_BIT_MASK(64));
bcc0ef7f57e51e Daniel Vetter 2020-09-09  461  
bcc0ef7f57e51e Daniel Vetter 2020-09-09  462  	vgem_device = devm_drm_dev_alloc(&pdev->dev, &vgem_driver,
bcc0ef7f57e51e Daniel Vetter 2020-09-09  463  					 struct vgem_device, drm);
bcc0ef7f57e51e Daniel Vetter 2020-09-09  464  	if (IS_ERR(vgem_device)) {
bcc0ef7f57e51e Daniel Vetter 2020-09-09  465  		ret = PTR_ERR(vgem_device);
bcc0ef7f57e51e Daniel Vetter 2020-09-09  466  		goto out_devres;
bcc0ef7f57e51e Daniel Vetter 2020-09-09  467  	}
bcc0ef7f57e51e Daniel Vetter 2020-09-09  468  	vgem_device->platform = pdev;
e2aff44868ae60 Laura Abbott  2017-05-04  469  
315f0242aa2b1e Chris Wilson  2017-05-08  470  	/* Final step: expose the device/driver to userspace */
315f0242aa2b1e Chris Wilson  2017-05-08  471  	ret = drm_dev_register(&vgem_device->drm, 0);
502e95c6678505 Zach Reizner  2015-03-04  472  	if (ret)
bcc0ef7f57e51e Daniel Vetter 2020-09-09  473  		goto out_devres;
502e95c6678505 Zach Reizner  2015-03-04  474  
502e95c6678505 Zach Reizner  2015-03-04  475  	return 0;
502e95c6678505 Zach Reizner  2015-03-04  476  
bcc0ef7f57e51e Daniel Vetter 2020-09-09  477  out_devres:
bcc0ef7f57e51e Daniel Vetter 2020-09-09  478  	devres_release_group(&pdev->dev, NULL);
d5c04dff24870e Deepak Sharma 2018-10-23  479  out_unregister:
bcc0ef7f57e51e Daniel Vetter 2020-09-09  480  	platform_device_unregister(pdev);
502e95c6678505 Zach Reizner  2015-03-04  481  	return ret;
502e95c6678505 Zach Reizner  2015-03-04  482  }
502e95c6678505 Zach Reizner  2015-03-04  483  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

On Fri, Oct 22, 2021 at 8:36 AM Xin Long [off-list ref] wrote:

The asoc created when receives the INIT chunk is a temporary one, it
will be delete after INIT_ACK chunk is replied. So for the real asoc

Nit: s/delete/deleted/

created in sctp_sf_do_5_1D_ce() when receives the COOKIE_ECHO chunk,

Nit: s/receives the COOKIE_ECHO chunk/the COOKIE_ECHO chunk is received/

Nit: s/an/a/

Nit: s/an/a/

--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.

On Fri, Oct 22, 2021 at 8:36 AM Xin Long [off-list ref] wrote:

security_sctp_assoc_established() is added to replace
security_inet_conn_established() called in
sctp_sf_do_5_1E_ca(), so that asoc can be accessed in security
subsystem and save the peer secid to asoc->peer_secid.

Fixes: 72e89f50084c ("security: Add support for SCTP security hooks")
Reported-by: Prashanth Prahlad <redacted>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 Documentation/security/SCTP.rst | 22 ++++++++++------------
 include/linux/lsm_hook_defs.h   |  2 ++
 include/linux/lsm_hooks.h       |  5 +++++
 include/linux/security.h        |  8 ++++++++
 net/sctp/sm_statefuns.c         |  2 +-
 security/security.c             |  7 +++++++
 6 files changed, 33 insertions(+), 13 deletions(-)

[...]

It has now been pointed out by the kernel robot as well, but you are
returning a value from a function with return type void here.

--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.

On Fri, Oct 22, 2021 at 8:36 AM Xin Long [off-list ref] wrote:

Different from selinux_inet_conn_established(), it also gives the
secid to asoc->peer_secid in selinux_sctp_assoc_established(),
as one UDP-type socket may have more than one asocs.

Note that peer_secid in asoc will save the peer secid for this
asoc connection, and peer_sid in sksec will just keep the peer
secid for the latest connection. So the right use should be do
peeloff for UDP-type socket if there will be multiple asocs in
one socket, so that the peeloff socket has the right label for
its asoc.

Hm... this sounds like something we should also try to fix (if
possible). In access control we can't trust userspace to do the right
thing - receiving from multiple peers on one SOCK_SEQPACKET socket
shouldn't cause checking against the wrong peer_sid. But that can be
addressed separately. (And maybe it's even already accounted for
somehow - I didn't yet look at the code closely.)

You could replace the above with
`selinux_inet_conn_established(asoc->base.sk, skb);` to reduce code
duplication.

--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.

On Mon, Oct 25, 2021 at 4:17 PM Ondrej Mosnacek [off-list ref] wrote:

On Fri, Oct 22, 2021 at 8:36 AM Xin Long [off-list ref] wrote:

quoted

Different from selinux_inet_conn_established(), it also gives the
secid to asoc->peer_secid in selinux_sctp_assoc_established(),
as one UDP-type socket may have more than one asocs.

Note that peer_secid in asoc will save the peer secid for this
asoc connection, and peer_sid in sksec will just keep the peer
secid for the latest connection. So the right use should be do
peeloff for UDP-type socket if there will be multiple asocs in
one socket, so that the peeloff socket has the right label for
its asoc.

Hm... this sounds like something we should also try to fix (if
possible). In access control we can't trust userspace to do the right
thing - receiving from multiple peers on one SOCK_SEQPACKET socket
shouldn't cause checking against the wrong peer_sid. But that can be
addressed separately. (And maybe it's even already accounted for
somehow - I didn't yet look at the code closely.)

quoted

Fixes: 72e89f50084c ("security: Add support for SCTP security hooks")
Reported-by: Prashanth Prahlad <redacted>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 security/selinux/hooks.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index f025fc00421b..793fdcbc68bd 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c

@@ -5525,6 +5525,21 @@ static void selinux_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk
        selinux_netlbl_sctp_sk_clone(sk, newsk);
 }

+static void selinux_sctp_assoc_established(struct sctp_association *asoc,
+                                          struct sk_buff *skb)
+{
+       struct sk_security_struct *sksec = asoc->base.sk->sk_security;
+       u16 family = asoc->base.sk->sk_family;
+
+       /* handle mapped IPv4 packets arriving via IPv6 sockets */
+       if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP))
+               family = PF_INET;
+
+       selinux_skb_peerlbl_sid(skb, family, &sksec->peer_sid);

You could replace the above with
`selinux_inet_conn_established(asoc->base.sk, skb);` to reduce code
duplication.

Hi Ondrej,

will do, thanks!

quoted

+       asoc->secid = sksec->sid;
+       asoc->peer_secid = sksec->peer_sid;
+}
+

Now I'm thinking: 'peer_sid' should be correct here.

BUT 'sid' is copied from its parent socket. Later when doing peel-off,
asoc->secid will be set back to the peel-off socket's sksec->sid.

Do you think this is okay? or should the peel-off socket have its own
sksec->sid, which might be different from the parent socket's?
(Note the socket's sid initially was set in selinux_socket_post_create())

quoted

 static int selinux_inet_conn_request(const struct sock *sk, struct sk_buff *skb,
                                     struct request_sock *req)
 {

@@ -7290,6 +7305,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
        LSM_HOOK_INIT(sctp_assoc_request, selinux_sctp_assoc_request),
        LSM_HOOK_INIT(sctp_sk_clone, selinux_sctp_sk_clone),
        LSM_HOOK_INIT(sctp_bind_connect, selinux_sctp_bind_connect),
+       LSM_HOOK_INIT(sctp_assoc_established, selinux_sctp_assoc_established),
        LSM_HOOK_INIT(inet_conn_request, selinux_inet_conn_request),
        LSM_HOOK_INIT(inet_csk_clone, selinux_inet_csk_clone),
        LSM_HOOK_INIT(inet_conn_established, selinux_inet_conn_established),
--

2.27.0

--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.

On Mon, Oct 25, 2021 at 12:51 PM Xin Long [off-list ref] wrote:

On Mon, Oct 25, 2021 at 4:17 PM Ondrej Mosnacek [off-list ref] wrote:

quoted

On Fri, Oct 22, 2021 at 8:36 AM Xin Long [off-list ref] wrote:

quoted

Different from selinux_inet_conn_established(), it also gives the
secid to asoc->peer_secid in selinux_sctp_assoc_established(),
as one UDP-type socket may have more than one asocs.

Note that peer_secid in asoc will save the peer secid for this
asoc connection, and peer_sid in sksec will just keep the peer
secid for the latest connection. So the right use should be do
peeloff for UDP-type socket if there will be multiple asocs in
one socket, so that the peeloff socket has the right label for
its asoc.

Hm... this sounds like something we should also try to fix (if
possible). In access control we can't trust userspace to do the right
thing - receiving from multiple peers on one SOCK_SEQPACKET socket
shouldn't cause checking against the wrong peer_sid. But that can be
addressed separately. (And maybe it's even already accounted for
somehow - I didn't yet look at the code closely.)

quoted

Fixes: 72e89f50084c ("security: Add support for SCTP security hooks")
Reported-by: Prashanth Prahlad <redacted>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 security/selinux/hooks.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index f025fc00421b..793fdcbc68bd 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c

@@ -5525,6 +5525,21 @@ static void selinux_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk
        selinux_netlbl_sctp_sk_clone(sk, newsk);
 }

+static void selinux_sctp_assoc_established(struct sctp_association *asoc,
+                                          struct sk_buff *skb)
+{
+       struct sk_security_struct *sksec = asoc->base.sk->sk_security;
+       u16 family = asoc->base.sk->sk_family;
+
+       /* handle mapped IPv4 packets arriving via IPv6 sockets */
+       if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP))
+               family = PF_INET;
+
+       selinux_skb_peerlbl_sid(skb, family, &sksec->peer_sid);

You could replace the above with
`selinux_inet_conn_established(asoc->base.sk, skb);` to reduce code
duplication.

Hi Ondrej,

will do, thanks!

quoted

quoted

+       asoc->secid = sksec->sid;
+       asoc->peer_secid = sksec->peer_sid;
+}
+

Now I'm thinking: 'peer_sid' should be correct here.

BUT 'sid' is copied from its parent socket. Later when doing peel-off,
asoc->secid will be set back to the peel-off socket's sksec->sid.

Hi,

I'm not sure I follow... When doing peel-off, security_sctp_sk_clone()
should be called, which sets the peel-off socket's sksec->sid to
asoc->secid, not the other way around. (Are we hitting the language
barrier here? :)

Do you think this is okay? or should the peel-off socket have its own
sksec->sid, which might be different from the parent socket's?
(Note the socket's sid initially was set in selinux_socket_post_create())

I *think* in case of a client socket it is expected for the
peeloff-style child socket to just inherit the same sksec->sid. But
frankly I haven't been with SELinux and kernel development long enough
to understand the intricacies of SELinux's network connection handling
very well... Hopefully Paul/Richard/Stephen can give a more confident
answer/review here.

--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.