Fix LOR between the syncache and inpcb locks when MAC is present in the

kernel.  This LOR snuck in with some of the recent syncache changes.  To
fix this, the inpcb handling was changed:

- Hang a MAC label off the syncache object
- When the syncache entry is initially created, we pickup the PCB lock
  is held because we extract information from it while initializing the
  syncache entry.  While we do this, copy the MAC label associated with
  the PCB and use it for the syncache entry.
- When the packet is transmitted, copy the label from the syncache entry
  to the mbuf so it can be processed by security policies which analyze
  mbuf labels.

This change required that the MAC framework be extended to support the
label copy operations from the PCB to the syncache entry, and then from
the syncache entry to the mbuf.

These functions really should be referencing the syncache structure instead
of the label.  However, due to some of the complexities associated with
exposing this syncache structure we operate directly on it's label pointer.
This should be OK since we aren't making any access control decisions within
this code directly, we are merely allocating and copying label storage so
we can properly initialize mbuf labels for any packets the syncache code
might create.

This also has a nice side effect of caching.  Prior to this change, the
PCB would be looked up/locked for each packet transmitted.  Now the label
is cached at the time the syncache entry is initialized.

Submitted by:	andre [1]
Discussed with:	rwatson

[1] andre submitted the tcp_syncache.c changes

5 files changed, 122 insertions, 43 deletions

diff --git a/sys/netinet/tcp_syncache.c b/sys/netinet/tcp_syncache.c
index e8ca230..b1b209d 100644
--- a/sys/netinet/tcp_syncache.c
+++ b/sys/netinet/tcp_syncache.c
@@ -139,6 +139,9 @@ struct syncache {
 #define SCF_UNREACH	0x10			/* icmp unreachable received */
 #define SCF_SIGNATURE	0x20			/* send MD5 digests */
 #define SCF_SACK	0x80			/* send SACK option */
+#ifdef MAC
+	struct label	*sc_label;		/* MAC label reference */
+#endif
 };
 
 struct syncache_head {
@@ -256,6 +259,9 @@ syncache_free(struct syncache *sc)
 {
 	if (sc->sc_ipopts)
 		(void) m_free(sc->sc_ipopts);
+#ifdef MAC
+	mac_destroy_syncache(&sc->sc_label);
+#endif
 
 	uma_zfree(tcp_syncache.zone, sc);
 }
@@ -850,6 +856,9 @@ syncache_add(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th,
 #ifdef INET6
 	int autoflowlabel = 0;
 #endif
+#ifdef MAC
+	struct label *maclabel;
+#endif
 	struct syncache scs;
 
 	INP_INFO_WLOCK_ASSERT(&tcbinfo);
@@ -876,6 +885,15 @@ syncache_add(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th,
 	so = NULL;
 	tp = NULL;
 
+#ifdef MAC
+	if (mac_init_syncache(&maclabel) != 0) {
+		*lsop = NULL;
+		INP_UNLOCK(inp);
+		INP_INFO_WUNLOCK(&tcbinfo);
+		return (1);
+	} else
+		mac_init_syncache_from_inpcb(maclabel, inp);
+#endif
 	INP_UNLOCK(inp);
 	INP_INFO_WUNLOCK(&tcbinfo);
 
@@ -912,6 +930,16 @@ syncache_add(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th,
 		 */
 		if (sc->sc_flags & SCF_TIMESTAMP)
 			sc->sc_tsreflect = to->to_tsval;
+#ifdef MAC
+		/*
+		 * Since we have already unconditionally allocated label
+		 * storage, free it up.  The syncache entry will already
+		 * have an initialized label we can use.
+		 */
+		mac_destroy_syncache(&maclabel);
+		KASSERT(sc->sc_label != NULL,
+		    ("%s: label not initialized", __func__));
+#endif
 		if (syncache_respond(sc, m) == 0) {
 			SYNCACHE_TIMEOUT(sc, sch, 1);
 			tcpstat.tcps_sndacks++;
@@ -948,6 +976,9 @@ syncache_add(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th,
 	/*
 	 * Fill in the syncache values.
 	 */
+#ifdef MAC
+	sc->sc_label = maclabel;
+#endif
 	sc->sc_ipopts = ipopts;
 	bcopy(inc, &sc->sc_inc, sizeof(struct in_conninfo));
 #ifdef INET6
@@ -1033,10 +1064,19 @@ syncache_add(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th,
 			syncache_free(sc);
 		else if (sc != &scs)
 			syncache_insert(sc, sch);   /* locks and unlocks sch */
+#ifdef MAC
+		else
+			mac_destroy_syncache(&sc->sc_label);
+#endif
 		tcpstat.tcps_sndacks++;
 		tcpstat.tcps_sndtotal++;
 	} else {
-		syncache_free(sc);
+		if (sc != &scs)
+			syncache_free(sc);
+#ifdef MAC
+		else
+			mac_destroy_syncache(&sc->sc_label);
+#endif
 		tcpstat.tcps_sc_dropped++;
 	}
 
@@ -1056,9 +1096,6 @@ syncache_respond(struct syncache *sc, struct mbuf *m)
 #ifdef INET6
 	struct ip6_hdr *ip6 = NULL;
 #endif
-#ifdef MAC
-	struct inpcb *inp = NULL;
-#endif
 
 	hlen =
 #ifdef INET6
@@ -1100,50 +1137,14 @@ syncache_respond(struct syncache *sc, struct mbuf *m)
 	m = m_gethdr(M_DONTWAIT, MT_DATA);
 	if (m == NULL)
 		return (ENOBUFS);
+#ifdef MAC
+	mac_create_mbuf_from_syncache(sc->sc_label, m);
+#endif
 	m->m_data += max_linkhdr;
 	m->m_len = tlen;
 	m->m_pkthdr.len = tlen;
 	m->m_pkthdr.rcvif = NULL;
 
-#ifdef MAC
-	/*
-	 * For MAC look up the inpcb to get access to the label information.
-	 * We don't store the inpcb pointer in struct syncache to make locking
-	 * less complicated and to save locking operations.  However for MAC
-	 * this gives a slight overhead as we have to do a full pcblookup here.
-	 */
-	INP_INFO_RLOCK(&tcbinfo);
-	if (inp == NULL) {
-#ifdef INET6 /* && MAC */
-		if (sc->sc_inc.inc_isipv6)
-			inp = in6_pcblookup_hash(&tcbinfo,
-				&sc->sc_inc.inc6_faddr, sc->sc_inc.inc_fport,
-				&sc->sc_inc.inc6_laddr, sc->sc_inc.inc_lport,
-				1, NULL);
-		else
-#endif /* INET6 */
-			inp = in_pcblookup_hash(&tcbinfo,
-				sc->sc_inc.inc_faddr, sc->sc_inc.inc_fport,
-				sc->sc_inc.inc_laddr, sc->sc_inc.inc_lport,
-				1, NULL);
-		if (inp == NULL) {
-			m_freem(m);
-			INP_INFO_RUNLOCK(&tcbinfo);
-			return (ESHUTDOWN);
-		}
-	}
-	INP_LOCK(inp);
-	if (!inp->inp_socket->so_options & SO_ACCEPTCONN) {
-		m_freem(m);
-		INP_UNLOCK(inp);
-		INP_INFO_RUNLOCK(&tcbinfo);
-		return (ESHUTDOWN);
-	}
-	mac_create_mbuf_from_inpcb(inp, m);
-	INP_UNLOCK(inp);
-	INP_INFO_RUNLOCK(&tcbinfo);
-#endif /* MAC */
-
 #ifdef INET6
 	if (sc->sc_inc.inc_isipv6) {
 		ip6 = mtod(m, struct ip6_hdr *);
diff --git a/sys/security/mac/mac_framework.h b/sys/security/mac/mac_framework.h
index a895cfe..c8c41c2 100644
--- a/sys/security/mac/mac_framework.h
+++ b/sys/security/mac/mac_framework.h
@@ -210,6 +210,10 @@ void	mac_update_ipq(struct mbuf *fragment, struct ipq *ipq);
 void	mac_inpcb_sosetlabel(struct socket *so, struct inpcb *inp);
 
 void	mac_create_mbuf_from_firewall(struct mbuf *m);
+void	mac_destroy_syncache(struct label **label);
+int	mac_init_syncache(struct label **label);
+void	mac_init_syncache_from_inpcb(struct label *label, struct inpcb *inp);
+void	mac_create_mbuf_from_syncache(struct label *sc_label, struct mbuf *m);
 /*
  * Labeling event operations: processes.
  */
diff --git a/sys/security/mac/mac_inet.c b/sys/security/mac/mac_inet.c
index 0d35e48..7896332 100644
--- a/sys/security/mac/mac_inet.c
+++ b/sys/security/mac/mac_inet.c
@@ -288,3 +288,57 @@ mac_create_mbuf_from_firewall(struct mbuf *m)
 	label = mac_mbuf_to_label(m);
 	MAC_PERFORM(create_mbuf_from_firewall, m, label);
 }
+
+/*
+ * These functions really should be referencing the syncache structure instead
+ * of the label.  However, due to some of the complexities associated with
+ * exposing this syncache structure we operate directly on it's label pointer.
+ * This should be OK since we aren't making any access control decisions within
+ * this code directly, we are merely allocating and copying label storage so
+ * we can properly initialize mbuf labels for any packets the syncache code
+ * might create.
+ */
+void
+mac_destroy_syncache(struct label **label)
+{
+
+	MAC_PERFORM(destroy_syncache_label, *label);
+	mac_labelzone_free(*label);
+	*label = NULL;
+}
+
+int
+mac_init_syncache(struct label **label)
+{
+	int error;
+
+	*label = mac_labelzone_alloc(M_NOWAIT);
+	if (*label == NULL)
+		return (ENOMEM);
+	/*
+	 * Since we are holding the inpcb locks the policy can not allocate
+	 * policy specific label storage using M_WAITOK.  So we need to do a
+	 * MAC_CHECK instead of the typical MAC_PERFORM so we can propagate
+	 * allocation failures back to the syncache code.
+	 */
+	MAC_CHECK(init_syncache_label, *label, M_NOWAIT);
+	return (error);
+}
+
+void
+mac_init_syncache_from_inpcb(struct label *label, struct inpcb *inp)
+{
+
+	INP_LOCK_ASSERT(inp);
+	MAC_PERFORM(init_syncache_from_inpcb, label, inp);
+}
+
+void
+mac_create_mbuf_from_syncache(struct label *sc_label, struct mbuf *m)
+{
+	struct label *mbuf_label;
+
+	M_ASSERTPKTHDR(m);
+	mbuf_label = mac_mbuf_to_label(m);
+	MAC_PERFORM(create_mbuf_from_syncache, sc_label, m, mbuf_label);
+}
diff --git a/sys/security/mac/mac_policy.h b/sys/security/mac/mac_policy.h
index f7c5670..e75a1e3 100644
--- a/sys/security/mac/mac_policy.h
+++ b/sys/security/mac/mac_policy.h
@@ -331,6 +331,12 @@ typedef void	(*mpo_inpcb_sosetlabel_t)(struct socket *so,
 
 typedef	void	(*mpo_create_mbuf_from_firewall_t)(struct mbuf *m,
 		    struct label *label);
+typedef void	(*mpo_destroy_syncache_label_t)(struct label *label);
+typedef int	(*mpo_init_syncache_label_t)(struct label *label, int flag);
+typedef void	(*mpo_init_syncache_from_inpcb_t)(struct label *label,
+		    struct inpcb *inp);
+typedef void	(*mpo_create_mbuf_from_syncache_t)(struct label *sc_label,
+		    struct mbuf *m, struct label *mbuf_label);
 /*
  * Labeling event operations: processes.
  */
@@ -888,6 +894,10 @@ struct mac_policy_ops {
 	mpo_check_vnode_write_t			mpo_check_vnode_write;
 	mpo_associate_nfsd_label_t		mpo_associate_nfsd_label;
 	mpo_create_mbuf_from_firewall_t		mpo_create_mbuf_from_firewall;
+	mpo_init_syncache_label_t		mpo_init_syncache_label;
+	mpo_destroy_syncache_label_t		mpo_destroy_syncache_label;
+	mpo_init_syncache_from_inpcb_t		mpo_init_syncache_from_inpcb;
+	mpo_create_mbuf_from_syncache_t		mpo_create_mbuf_from_syncache;
 	mpo_priv_check_t			mpo_priv_check;
 	mpo_priv_grant_t			mpo_priv_grant;
 };
diff --git a/sys/sys/mac_policy.h b/sys/sys/mac_policy.h
index f7c5670..e75a1e3 100644
--- a/sys/sys/mac_policy.h
+++ b/sys/sys/mac_policy.h
@@ -331,6 +331,12 @@ typedef void	(*mpo_inpcb_sosetlabel_t)(struct socket *so,
 
 typedef	void	(*mpo_create_mbuf_from_firewall_t)(struct mbuf *m,
 		    struct label *label);
+typedef void	(*mpo_destroy_syncache_label_t)(struct label *label);
+typedef int	(*mpo_init_syncache_label_t)(struct label *label, int flag);
+typedef void	(*mpo_init_syncache_from_inpcb_t)(struct label *label,
+		    struct inpcb *inp);
+typedef void	(*mpo_create_mbuf_from_syncache_t)(struct label *sc_label,
+		    struct mbuf *m, struct label *mbuf_label);
 /*
  * Labeling event operations: processes.
  */
@@ -888,6 +894,10 @@ struct mac_policy_ops {
 	mpo_check_vnode_write_t			mpo_check_vnode_write;
 	mpo_associate_nfsd_label_t		mpo_associate_nfsd_label;
 	mpo_create_mbuf_from_firewall_t		mpo_create_mbuf_from_firewall;
+	mpo_init_syncache_label_t		mpo_init_syncache_label;
+	mpo_destroy_syncache_label_t		mpo_destroy_syncache_label;
+	mpo_init_syncache_from_inpcb_t		mpo_init_syncache_from_inpcb;
+	mpo_create_mbuf_from_syncache_t		mpo_create_mbuf_from_syncache;
 	mpo_priv_check_t			mpo_priv_check;
 	mpo_priv_grant_t			mpo_priv_grant;
 };