Socket MAC labels so_label and so_peerlabel are now protected by

SOCK_LOCK(so):

- Hold socket lock over calls to MAC entry points reading or
  manipulating socket labels.

- Assert socket lock in MAC entry point implementations.

- When externalizing the socket label, first make a thread-local
  copy while holding the socket lock, then release the socket lock
  to externalize to userspace.

16 files changed, 104 insertions, 11 deletions

diff --git a/sys/compat/svr4/svr4_stream.c b/sys/compat/svr4/svr4_stream.c
index 6bb4189..db7eabc 100644
--- a/sys/compat/svr4/svr4_stream.c
+++ b/sys/compat/svr4/svr4_stream.c
@@ -171,7 +171,9 @@ svr4_sendit(td, s, mp, flags)
 		return (error);
 
 #ifdef MAC
+	SOCK_LOCK(so);
 	error = mac_check_socket_send(td->td_ucred, so);
+	SOCK_UNLOCK(so);
 	if (error)
 		goto done1;
 #endif
@@ -275,7 +277,9 @@ svr4_recvit(td, s, mp, namelenp)
 		return (error);
 
 #ifdef MAC
+	SOCK_LOCK(so);
 	error = mac_check_socket_receive(td->td_ucred, so);
+	SOCK_UNLOCK(so);
 	if (error)
 		goto done1;
 #endif
diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c
index ab2ae0a..a964592 100644
--- a/sys/kern/kern_prot.c
+++ b/sys/kern/kern_prot.c
@@ -1685,7 +1685,9 @@ cr_canseesocket(struct ucred *cred, struct socket *so)
 	if (error)
 		return (ENOENT);
 #ifdef MAC
+	SOCK_LOCK(so);
 	error = mac_check_socket_visible(cred, so);
+	SOCK_UNLOCK(so);
 	if (error)
 		return (error);
 #endif
diff --git a/sys/kern/sys_socket.c b/sys/kern/sys_socket.c
index 5331574..5f14608 100644
--- a/sys/kern/sys_socket.c
+++ b/sys/kern/sys_socket.c
@@ -77,7 +77,9 @@ soo_read(fp, uio, active_cred, flags, td)
 
 	NET_LOCK_GIANT();
 #ifdef MAC
+	SOCK_LOCK(so);
 	error = mac_check_socket_receive(active_cred, so);
+	SOCK_UNLOCK(so);
 	if (error) {
 		NET_UNLOCK_GIANT();
 		return (error);
@@ -102,7 +104,9 @@ soo_write(fp, uio, active_cred, flags, td)
 
 	NET_LOCK_GIANT();
 #ifdef MAC
+	SOCK_LOCK(so);
 	error = mac_check_socket_send(active_cred, so);
+	SOCK_UNLOCK(so);
 	if (error) {
 		NET_UNLOCK_GIANT();
 		return (error);
diff --git a/sys/kern/uipc_sockbuf.c b/sys/kern/uipc_sockbuf.c
index 0d75abe..7dbc19d 100644
--- a/sys/kern/uipc_sockbuf.c
+++ b/sys/kern/uipc_sockbuf.c
@@ -209,7 +209,9 @@ sonewconn(head, connstatus)
 	so->so_timeo = head->so_timeo;
 	so->so_cred = crhold(head->so_cred);
 #ifdef MAC
+	SOCK_LOCK(head);
 	mac_create_socket_from_socket(head, so);
+	SOCK_UNLOCK(head);
 #endif
 	if (soreserve(so, head->so_snd.sb_hiwat, head->so_rcv.sb_hiwat) ||
 	    (*so->so_proto->pr_usrreqs->pru_attach)(so, 0, NULL)) {
diff --git a/sys/kern/uipc_socket2.c b/sys/kern/uipc_socket2.c
index 0d75abe..7dbc19d 100644
--- a/sys/kern/uipc_socket2.c
+++ b/sys/kern/uipc_socket2.c
@@ -209,7 +209,9 @@ sonewconn(head, connstatus)
 	so->so_timeo = head->so_timeo;
 	so->so_cred = crhold(head->so_cred);
 #ifdef MAC
+	SOCK_LOCK(head);
 	mac_create_socket_from_socket(head, so);
+	SOCK_UNLOCK(head);
 #endif
 	if (soreserve(so, head->so_snd.sb_hiwat, head->so_rcv.sb_hiwat) ||
 	    (*so->so_proto->pr_usrreqs->pru_attach)(so, 0, NULL)) {
diff --git a/sys/kern/uipc_syscalls.c b/sys/kern/uipc_syscalls.c
index 53d4962..18a5e24 100644
--- a/sys/kern/uipc_syscalls.c
+++ b/sys/kern/uipc_syscalls.c
@@ -190,7 +190,9 @@ kern_bind(td, fd, sa)
 	if ((error = fgetsock(td, fd, &so, NULL)) != 0)
 		goto done2;
 #ifdef MAC
+	SOCK_LOCK(so);
 	error = mac_check_socket_bind(td->td_ucred, so, sa);
+	SOCK_UNLOCK(so);
 	if (error)
 		goto done1;
 #endif
@@ -223,7 +225,9 @@ listen(td, uap)
 	NET_LOCK_GIANT();
 	if ((error = fgetsock(td, uap->s, &so, NULL)) == 0) {
 #ifdef MAC
+		SOCK_LOCK(so);
 		error = mac_check_socket_listen(td->td_ucred, so);
+		SOCK_UNLOCK(so);
 		if (error)
 			goto done;
 #endif
@@ -482,7 +486,9 @@ kern_connect(td, fd, sa)
 		goto done1;
 	}
 #ifdef MAC
+	SOCK_LOCK(so);
 	error = mac_check_socket_connect(td->td_ucred, so, sa);
+	SOCK_UNLOCK(so);
 	if (error)
 		goto bad;
 #endif
@@ -701,7 +707,9 @@ kern_sendit(td, s, mp, flags, control)
 		goto bad2;
 
 #ifdef MAC
+	SOCK_LOCK(so);
 	error = mac_check_socket_send(td->td_ucred, so);
+	SOCK_UNLOCK(so);
 	if (error)
 		goto bad;
 #endif
@@ -944,7 +952,9 @@ recvit(td, s, mp, namelenp)
 	}
 
 #ifdef MAC
+	SOCK_LOCK(so);
 	error = mac_check_socket_receive(td->td_ucred, so);
+	SOCK_UNLOCK(so);
 	if (error) {
 		fputsock(so);
 		NET_UNLOCK_GIANT();
@@ -1750,7 +1760,9 @@ do_sendfile(struct thread *td, struct sendfile_args *uap, int compat)
 	}
 
 #ifdef MAC
+	SOCK_LOCK(so);
 	error = mac_check_socket_send(td->td_ucred, so);
+	SOCK_UNLOCK(so);
 	if (error)
 		goto done;
 #endif
diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c
index 6660d7b..aa435f2 100644
--- a/sys/kern/uipc_usrreq.c
+++ b/sys/kern/uipc_usrreq.c
@@ -846,8 +846,10 @@ unp_connect(so, nam, td)
 		    sizeof(unp->unp_peercred));
 		unp->unp_flags |= UNP_HAVEPC;
 #ifdef MAC
+		SOCK_LOCK(so);
 		mac_set_socket_peer_from_socket(so, so3);
 		mac_set_socket_peer_from_socket(so3, so);
+		SOCK_UNLOCK(so);
 #endif
 
 		so2 = so3;
diff --git a/sys/netatalk/ddp_input.c b/sys/netatalk/ddp_input.c
index 44c0f6f..679f943 100644
--- a/sys/netatalk/ddp_input.c
+++ b/sys/netatalk/ddp_input.c
@@ -366,10 +366,13 @@ ddp_input(m, ifp, elh, phase)
     }
 
 #ifdef MAC
+    SOCK_LOCK(ddp->ddp_socket);
     if (mac_check_socket_deliver(ddp->ddp_socket, m) != 0) {
+	SOCK_UNLOCK(ddp->ddp_socket);
 	m_freem(m);
 	return;
     }
+    SOCK_UNLOCK(ddp->ddp_socket);
 #endif
 
     /* 
diff --git a/sys/netatalk/ddp_output.c b/sys/netatalk/ddp_output.c
index 232fa2b..7270d54 100644
--- a/sys/netatalk/ddp_output.c
+++ b/sys/netatalk/ddp_output.c
@@ -52,7 +52,9 @@ ddp_output(struct mbuf *m, struct socket *so)
     struct ddpcb *ddp = sotoddpcb(so);
 
 #ifdef MAC
+    SOCK_LOCK(so);
     mac_create_mbuf_from_socket(so, m);
+    SOCK_UNLOCK(so);
 #endif
 
     M_PREPEND(m, sizeof(struct ddpehdr), M_TRYWAIT);
diff --git a/sys/netinet/in_pcb.c b/sys/netinet/in_pcb.c
index ad461fd..adad6de 100644
--- a/sys/netinet/in_pcb.c
+++ b/sys/netinet/in_pcb.c
@@ -176,7 +176,9 @@ in_pcballoc(so, pcbinfo, type)
 	error = mac_init_inpcb(inp, M_NOWAIT);
 	if (error != 0)
 		goto out;
+	SOCK_LOCK(so);
 	mac_create_inpcb_from_socket(so, inp);
+	SOCK_UNLOCK(so);
 #endif
 #if defined(IPSEC) || defined(FAST_IPSEC)
 #ifdef FAST_IPSEC
@@ -1175,10 +1177,11 @@ in_pcbsosetlabel(so)
 #ifdef MAC
 	struct inpcb *inp;
 
-	/* XXX: Will assert socket lock when we have them. */
 	inp = (struct inpcb *)so->so_pcb;
 	INP_LOCK(inp);
+	SOCK_LOCK(so);
 	mac_inpcb_sosetlabel(so, inp);
+	SOCK_UNLOCK(so);
 	INP_UNLOCK(inp);
 #endif
 }
diff --git a/sys/netinet/ip_divert.c b/sys/netinet/ip_divert.c
index ccf0d96..f745fea 100644
--- a/sys/netinet/ip_divert.c
+++ b/sys/netinet/ip_divert.c
@@ -263,7 +263,9 @@ div_output(struct socket *so, struct mbuf *m,
 	KASSERT(m->m_pkthdr.rcvif == NULL, ("rcvif not null"));
 
 #ifdef MAC
+	SOCK_LOCK(so);
 	mac_create_mbuf_from_socket(so, m);
+	SOCK_UNLOCK(so);
 #endif
 
 	if (control)
diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c
index 6178bfb..04b4d2c 100644
--- a/sys/netinet/tcp_input.c
+++ b/sys/netinet/tcp_input.c
@@ -1352,7 +1352,9 @@ after_listen:
 			tcpstat.tcps_connects++;
 			soisconnected(so);
 #ifdef MAC
+			SOCK_LOCK(so);
 			mac_set_socket_peer_from_mbuf(m, so);
+			SOCK_UNLOCK(so);
 #endif
 			/* Do window scaling on this connection? */
 			if ((tp->t_flags & (TF_RCVD_SCALE|TF_REQ_SCALE)) ==
diff --git a/sys/netinet/tcp_reass.c b/sys/netinet/tcp_reass.c
index 6178bfb..04b4d2c 100644
--- a/sys/netinet/tcp_reass.c
+++ b/sys/netinet/tcp_reass.c
@@ -1352,7 +1352,9 @@ after_listen:
 			tcpstat.tcps_connects++;
 			soisconnected(so);
 #ifdef MAC
+			SOCK_LOCK(so);
 			mac_set_socket_peer_from_mbuf(m, so);
+			SOCK_UNLOCK(so);
 #endif
 			/* Do window scaling on this connection? */
 			if ((tp->t_flags & (TF_RCVD_SCALE|TF_REQ_SCALE)) ==
diff --git a/sys/netinet/tcp_syncache.c b/sys/netinet/tcp_syncache.c
index 1fb7e99..640a9f0 100644
--- a/sys/netinet/tcp_syncache.c
+++ b/sys/netinet/tcp_syncache.c
@@ -559,7 +559,9 @@ syncache_socket(sc, lso, m)
 		goto abort2;
 	}
 #ifdef MAC
+	SOCK_LOCK(so);
 	mac_set_socket_peer_from_mbuf(m, so);
+	SOCK_UNLOCK(so);
 #endif
 
 	inp = sotoinpcb(so);
diff --git a/sys/security/mac/mac_socket.c b/sys/security/mac/mac_socket.c
index 1ddef70..7b48f79 100644
--- a/sys/security/mac/mac_socket.c
+++ b/sys/security/mac/mac_socket.c
@@ -218,6 +218,7 @@ mac_create_socket_from_socket(struct socket *oldsocket,
     struct socket *newsocket)
 {
 
+	SOCK_LOCK_ASSERT(oldsocket);
 	MAC_PERFORM(create_socket_from_socket, oldsocket, oldsocket->so_label,
 	    newsocket, newsocket->so_label);
 }
@@ -227,6 +228,7 @@ mac_relabel_socket(struct ucred *cred, struct socket *socket,
     struct label *newlabel)
 {
 
+	SOCK_LOCK_ASSERT(socket);
 	MAC_PERFORM(relabel_socket, cred, socket, socket->so_label, newlabel);
 }
 
@@ -235,6 +237,8 @@ mac_set_socket_peer_from_mbuf(struct mbuf *mbuf, struct socket *socket)
 {
 	struct label *label;
 
+	SOCK_LOCK_ASSERT(socket);
+
 	label = mac_mbuf_to_label(mbuf);
 
 	MAC_PERFORM(set_socket_peer_from_mbuf, mbuf, label, socket,
@@ -246,6 +250,12 @@ mac_set_socket_peer_from_socket(struct socket *oldsocket,
     struct socket *newsocket)
 {
 
+	/*
+	 * XXXRW: only hold the socket lock on one at a time, as one
+	 * socket is the original, and one is the new.  However, it's
+	 * called in both directions, so we can't assert the lock
+	 * here currently.
+	 */
 	MAC_PERFORM(set_socket_peer_from_socket, oldsocket,
 	    oldsocket->so_label, newsocket, newsocket->so_peerlabel);
 }
@@ -257,6 +267,7 @@ mac_create_mbuf_from_socket(struct socket *socket, struct mbuf *mbuf)
 
 	label = mac_mbuf_to_label(mbuf);
 
+	SOCK_LOCK_ASSERT(socket);
 	MAC_PERFORM(create_mbuf_from_socket, socket, socket->so_label, mbuf,
 	    label);
 }
@@ -267,6 +278,8 @@ mac_check_socket_bind(struct ucred *ucred, struct socket *socket,
 {
 	int error;
 
+	SOCK_LOCK_ASSERT(socket);
+
 	if (!mac_enforce_socket)
 		return (0);
 
@@ -282,6 +295,8 @@ mac_check_socket_connect(struct ucred *cred, struct socket *socket,
 {
 	int error;
 
+	SOCK_LOCK_ASSERT(socket);
+
 	if (!mac_enforce_socket)
 		return (0);
 
@@ -297,6 +312,8 @@ mac_check_socket_deliver(struct socket *socket, struct mbuf *mbuf)
 	struct label *label;
 	int error;
 
+	SOCK_LOCK_ASSERT(socket);
+
 	if (!mac_enforce_socket)
 		return (0);
 
@@ -313,6 +330,8 @@ mac_check_socket_listen(struct ucred *cred, struct socket *socket)
 {
 	int error;
 
+	SOCK_LOCK_ASSERT(socket);
+
 	if (!mac_enforce_socket)
 		return (0);
 
@@ -325,6 +344,8 @@ mac_check_socket_receive(struct ucred *cred, struct socket *so)
 {
 	int error;
 
+	SOCK_LOCK_ASSERT(so);
+
 	if (!mac_enforce_socket)
 		return (0);
 
@@ -339,6 +360,8 @@ mac_check_socket_relabel(struct ucred *cred, struct socket *socket,
 {
 	int error;
 
+	SOCK_LOCK_ASSERT(socket);
+
 	MAC_CHECK(check_socket_relabel, cred, socket, socket->so_label,
 	    newlabel);
 
@@ -350,6 +373,8 @@ mac_check_socket_send(struct ucred *cred, struct socket *so)
 {
 	int error;
 
+	SOCK_LOCK_ASSERT(so);
+
 	if (!mac_enforce_socket)
 		return (0);
 
@@ -363,6 +388,8 @@ mac_check_socket_visible(struct ucred *cred, struct socket *socket)
 {
 	int error;
 
+	SOCK_LOCK_ASSERT(socket);
+
 	if (!mac_enforce_socket)
 		return (0);
 
@@ -377,12 +404,24 @@ mac_socket_label_set(struct ucred *cred, struct socket *so,
 {
 	int error;
 
+	/*
+	 * We acquire the socket lock when we perform the test and set,
+	 * but have to release it as the pcb code needs to acquire the
+	 * pcb lock, which will precede the socket lock in the lock
+	 * order.  However, this is fine, as any race will simply
+	 * result in the inpcb being refreshed twice, but still
+	 * consistently, as the inpcb code will acquire the socket lock
+	 * before refreshing, holding both locks.
+	 */
+	SOCK_LOCK(so);
 	error = mac_check_socket_relabel(cred, so, label);
-	if (error)
+	if (error) {
+		SOCK_UNLOCK(so);
 		return (error);
+	}
 
 	mac_relabel_socket(cred, so, label);
-
+	SOCK_UNLOCK(so);
 	/*
 	 * If the protocol has expressed interest in socket layer changes,
 	 * such as if it needs to propagate changes to a cached pcb
@@ -419,9 +458,7 @@ mac_setsockopt_label(struct ucred *cred, struct socket *so, struct mac *mac)
 	if (error)
 		goto out;
 
-	/* XXX: Socket lock here. */
 	error = mac_socket_label_set(cred, so, intlabel);
-	/* XXX: Socket unlock here. */
 out:
 	mac_socket_label_free(intlabel);
 	return (error);
@@ -431,6 +468,7 @@ int
 mac_getsockopt_label(struct ucred *cred, struct socket *so, struct mac *mac)
 {
 	char *buffer, *elements;
+	struct label *intlabel;
 	int error;
 
 	error = mac_check_structmac_consistent(mac);
@@ -445,8 +483,13 @@ mac_getsockopt_label(struct ucred *cred, struct socket *so, struct mac *mac)
 	}
 
 	buffer = malloc(mac->m_buflen, M_MACTEMP, M_WAITOK | M_ZERO);
-	error = mac_externalize_socket_label(so->so_label, elements,
-	    buffer, mac->m_buflen);
+	intlabel = mac_socket_label_alloc(M_WAITOK);
+	SOCK_LOCK(so);
+	mac_copy_socket_label(so->so_label, intlabel);
+	SOCK_UNLOCK(so);
+	error = mac_externalize_socket_label(intlabel, elements, buffer,
+	    mac->m_buflen);
+	mac_socket_label_free(intlabel);
 	if (error == 0)
 		error = copyout(buffer, mac->m_string, strlen(buffer)+1);
 
@@ -461,6 +504,7 @@ mac_getsockopt_peerlabel(struct ucred *cred, struct socket *so,
     struct mac *mac)
 {
 	char *elements, *buffer;
+	struct label *intlabel;
 	int error;
 
 	error = mac_check_structmac_consistent(mac);
@@ -475,8 +519,13 @@ mac_getsockopt_peerlabel(struct ucred *cred, struct socket *so,
 	}
 
 	buffer = malloc(mac->m_buflen, M_MACTEMP, M_WAITOK | M_ZERO);
-	error = mac_externalize_socket_peer_label(so->so_peerlabel,
-	    elements, buffer, mac->m_buflen);
+	intlabel = mac_socket_label_alloc(M_WAITOK);
+	SOCK_LOCK(so);
+	mac_copy_socket_label(so->so_peerlabel, intlabel);
+	SOCK_UNLOCK(so);
+	error = mac_externalize_socket_peer_label(intlabel, elements, buffer,
+	    mac->m_buflen);
+	mac_socket_label_free(intlabel);
 	if (error == 0)
 		error = copyout(buffer, mac->m_string, strlen(buffer)+1);
 
diff --git a/sys/sys/socketvar.h b/sys/sys/socketvar.h
index e33be97..84a1747 100644
--- a/sys/sys/socketvar.h
+++ b/sys/sys/socketvar.h
@@ -124,8 +124,8 @@ struct socket {
 	void	(*so_upcall)(struct socket *, void *, int);
 	void	*so_upcallarg;
 	struct	ucred *so_cred;		/* user credentials */
-	struct	label *so_label;	/* MAC label for socket */
-	struct	label *so_peerlabel;	/* cached MAC label for socket peer */
+	struct	label *so_label;	/* (b) MAC label for socket */
+	struct	label *so_peerlabel;	/* (b) cached MAC label for peer */
 	/* NB: generation count must not be first; easiest to make it last. */
 	so_gen_t so_gencnt;		/* generation count */
 	void	*so_emuldata;		/* private data for emulators */