From cacd79e2c9f9c3c7a98f7b9e884b0eb5b1d02f1f Mon Sep 17 00:00:00 2001 From: Robert Watson Date: Thu, 10 Apr 2003 20:33:10 +0000 Subject: [PATCH] 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 --- sys/netinet/tcp_subr.c | 20 +++++++++++++++----- sys/netinet/tcp_timewait.c | 20 +++++++++++++++----- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c index e96647fa7f62..58395f5ef82e 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 e96647fa7f62..58395f5ef82e 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);