Remove a potential panic condition introduced by reduced TCP wait

state.  Those changed attempted to work around the changed invariant
that inp->in_socket was sometimes now NULL, but the logic wasn't
quite right, meaning that inp->in_socket would be dereferenced by
cr_canseesocket() if security.bsd.see_other_uids, jail, or MAC
were in use.  Attempt to clarify and correct the logic.

Note: the work-around originally introduced with the reduced TCP
wait state handling to use cr_cansee() instead of cr_canseesocket()
in this case isn't really right, although it "Does the right thing"
for most of the cases in the base system.  We'll need to address
this at some point in the future.

Pointed out by:	dcs
Obtained from:	TrustedBSD Project
Sponsored by:	DARPA, Network Associates Laboratories

2 files changed, 30 insertions, 10 deletions

diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c
index e96647f..58395f5 100644
--- a/sys/netinet/tcp_subr.c
+++ b/sys/netinet/tcp_subr.c
@@ -926,11 +926,21 @@ tcp_pcblist(SYSCTL_HANDLER_ARGS)
 	for (inp = LIST_FIRST(tcbinfo.listhead), i = 0; inp && i < n;
 	     inp = LIST_NEXT(inp, inp_list)) {
 		INP_LOCK(inp);
-		if (inp->inp_gencnt <= gencnt &&
-		    (((inp->inp_vflag & INP_TIMEWAIT) && 
-		    cr_cansee(req->td->td_ucred, intotw(inp)->tw_cred) == 0) ||
-		    cr_canseesocket(req->td->td_ucred, inp->inp_socket) == 0))
-			inp_list[i++] = inp;
+		if (inp->inp_gencnt <= gencnt) {
+			/*
+			 * XXX: This use of cr_cansee(), introduced with
+			 * TCP state changes, is not quite right, but for
+			 * now, better than nothing.
+			 */
+			if (inp->inp_vflag & INP_TIMEWAIT)
+				error = cr_cansee(req->td->td_ucred,
+				    intotw(inp)->tw_cred);
+			else
+				error = cr_canseesocket(req->td->td_ucred,
+				    inp->inp_socket);
+			if (error == 0)
+				inp_list[i++] = inp;
+		}
 		INP_UNLOCK(inp);
 	}
 	INP_INFO_RUNLOCK(&tcbinfo);
diff --git a/sys/netinet/tcp_timewait.c b/sys/netinet/tcp_timewait.c
index e96647f..58395f5 100644
--- a/sys/netinet/tcp_timewait.c
+++ b/sys/netinet/tcp_timewait.c
@@ -926,11 +926,21 @@ tcp_pcblist(SYSCTL_HANDLER_ARGS)
 	for (inp = LIST_FIRST(tcbinfo.listhead), i = 0; inp && i < n;
 	     inp = LIST_NEXT(inp, inp_list)) {
 		INP_LOCK(inp);
-		if (inp->inp_gencnt <= gencnt &&
-		    (((inp->inp_vflag & INP_TIMEWAIT) && 
-		    cr_cansee(req->td->td_ucred, intotw(inp)->tw_cred) == 0) ||
-		    cr_canseesocket(req->td->td_ucred, inp->inp_socket) == 0))
-			inp_list[i++] = inp;
+		if (inp->inp_gencnt <= gencnt) {
+			/*
+			 * XXX: This use of cr_cansee(), introduced with
+			 * TCP state changes, is not quite right, but for
+			 * now, better than nothing.
+			 */
+			if (inp->inp_vflag & INP_TIMEWAIT)
+				error = cr_cansee(req->td->td_ucred,
+				    intotw(inp)->tw_cred);
+			else
+				error = cr_canseesocket(req->td->td_ucred,
+				    inp->inp_socket);
+			if (error == 0)
+				inp_list[i++] = inp;
+		}
 		INP_UNLOCK(inp);
 	}
 	INP_INFO_RUNLOCK(&tcbinfo);