o correct locking problem: the inpcb must be held across tcp_respond

o add assertions in tcp_respond to validate inpcb locking assumptions
o use local variable instead of chasing pointers in tcp_respond

Supported by:	FreeBSD Foundation
This commit is contained in:
Sam Leffler 2003-11-08 22:59:22 +00:00
parent 2a0746208b
commit c29afad673
4 changed files with 46 additions and 36 deletions

View File

@ -2348,9 +2348,6 @@ dropwithreset:
&tcp_savetcp, 0);
#endif
if (tp)
INP_UNLOCK(inp);
if (thflags & TH_ACK)
/* mtod() below is safe as long as hdr dropping is delayed */
tcp_respond(tp, mtod(m, void *), th, m, (tcp_seq)0, th->th_ack,
@ -2362,6 +2359,9 @@ dropwithreset:
tcp_respond(tp, mtod(m, void *), th, m, th->th_seq+tlen,
(tcp_seq)0, TH_RST|TH_ACK);
}
if (tp)
INP_UNLOCK(inp);
if (headlocked)
INP_INFO_WUNLOCK(&tcbinfo);
return;

View File

@ -2348,9 +2348,6 @@ dropwithreset:
&tcp_savetcp, 0);
#endif
if (tp)
INP_UNLOCK(inp);
if (thflags & TH_ACK)
/* mtod() below is safe as long as hdr dropping is delayed */
tcp_respond(tp, mtod(m, void *), th, m, (tcp_seq)0, th->th_ack,
@ -2362,6 +2359,9 @@ dropwithreset:
tcp_respond(tp, mtod(m, void *), th, m, th->th_seq+tlen,
(tcp_seq)0, TH_RST|TH_ACK);
}
if (tp)
INP_UNLOCK(inp);
if (headlocked)
INP_INFO_WUNLOCK(&tcbinfo);
return;

View File

@ -378,6 +378,7 @@ tcp_respond(tp, ipgen, th, m, ack, seq, flags)
int isipv6;
#endif /* INET6 */
int ipflags = 0;
struct inpcb *inp;
KASSERT(tp != NULL || m != NULL, ("tcp_respond: tp and m both NULL"));
@ -388,18 +389,23 @@ tcp_respond(tp, ipgen, th, m, ack, seq, flags)
ip = ipgen;
if (tp) {
inp = tp->t_inpcb;
KASSERT(inp != NULL, ("tcp control block w/o inpcb"));
INP_INFO_WLOCK_ASSERT(&tcbinfo);
INP_LOCK_ASSERT(inp);
if (!(flags & TH_RST)) {
win = sbspace(&tp->t_inpcb->inp_socket->so_rcv);
win = sbspace(&inp->inp_socket->so_rcv);
if (win > (long)TCP_MAXWIN << tp->rcv_scale)
win = (long)TCP_MAXWIN << tp->rcv_scale;
}
#ifdef INET6
if (isipv6)
ro6 = &tp->t_inpcb->in6p_route;
ro6 = &inp->in6p_route;
else
#endif /* INET6 */
ro = &tp->t_inpcb->inp_route;
ro = &inp->inp_route;
} else {
inp = NULL;
#ifdef INET6
if (isipv6) {
ro6 = &sro6;
@ -480,12 +486,12 @@ tcp_respond(tp, ipgen, th, m, ack, seq, flags)
m->m_pkthdr.len = tlen;
m->m_pkthdr.rcvif = (struct ifnet *) 0;
#ifdef MAC
if (tp != NULL && tp->t_inpcb != NULL) {
if (inp != NULL) {
/*
* Packet is associated with a socket, so allow the
* label of the response to reflect the socket label.
*/
mac_create_mbuf_from_socket(tp->t_inpcb->inp_socket, m);
mac_create_mbuf_from_socket(inp->inp_socket, m);
} else {
/*
* Packet is not associated with a socket, so possibly
@ -510,7 +516,7 @@ tcp_respond(tp, ipgen, th, m, ack, seq, flags)
nth->th_sum = in6_cksum(m, IPPROTO_TCP,
sizeof(struct ip6_hdr),
tlen - sizeof(struct ip6_hdr));
ip6->ip6_hlim = in6_selecthlim(tp ? tp->t_inpcb : NULL,
ip6->ip6_hlim = in6_selecthlim(inp,
ro6 && ro6->ro_rt ?
ro6->ro_rt->rt_ifp :
NULL);
@ -523,26 +529,25 @@ tcp_respond(tp, ipgen, th, m, ack, seq, flags)
m->m_pkthdr.csum_data = offsetof(struct tcphdr, th_sum);
}
#ifdef TCPDEBUG
if (tp == NULL || (tp->t_inpcb->inp_socket->so_options & SO_DEBUG))
if (tp == NULL || (inp->inp_socket->so_options & SO_DEBUG))
tcp_trace(TA_OUTPUT, 0, tp, mtod(m, void *), th, 0);
#endif
#ifdef INET6
if (isipv6) {
(void)ip6_output(m, NULL, ro6, ipflags, NULL, NULL,
tp ? tp->t_inpcb : NULL);
(void) ip6_output(m, NULL, ro6, ipflags, NULL, NULL, inp);
if (ro6 == &sro6 && ro6->ro_rt) {
RTFREE(ro6->ro_rt);
ro6->ro_rt = NULL;
}
} else
#endif /* INET6 */
{
(void) ip_output(m, NULL, ro, ipflags, NULL, tp ? tp->t_inpcb : NULL);
if (ro == &sro && ro->ro_rt) {
RTFREE(ro->ro_rt);
ro->ro_rt = NULL;
{
(void) ip_output(m, NULL, ro, ipflags, NULL, inp);
if (ro == &sro && ro->ro_rt) {
RTFREE(ro->ro_rt);
ro->ro_rt = NULL;
}
}
}
}
/*

View File

@ -378,6 +378,7 @@ tcp_respond(tp, ipgen, th, m, ack, seq, flags)
int isipv6;
#endif /* INET6 */
int ipflags = 0;
struct inpcb *inp;
KASSERT(tp != NULL || m != NULL, ("tcp_respond: tp and m both NULL"));
@ -388,18 +389,23 @@ tcp_respond(tp, ipgen, th, m, ack, seq, flags)
ip = ipgen;
if (tp) {
inp = tp->t_inpcb;
KASSERT(inp != NULL, ("tcp control block w/o inpcb"));
INP_INFO_WLOCK_ASSERT(&tcbinfo);
INP_LOCK_ASSERT(inp);
if (!(flags & TH_RST)) {
win = sbspace(&tp->t_inpcb->inp_socket->so_rcv);
win = sbspace(&inp->inp_socket->so_rcv);
if (win > (long)TCP_MAXWIN << tp->rcv_scale)
win = (long)TCP_MAXWIN << tp->rcv_scale;
}
#ifdef INET6
if (isipv6)
ro6 = &tp->t_inpcb->in6p_route;
ro6 = &inp->in6p_route;
else
#endif /* INET6 */
ro = &tp->t_inpcb->inp_route;
ro = &inp->inp_route;
} else {
inp = NULL;
#ifdef INET6
if (isipv6) {
ro6 = &sro6;
@ -480,12 +486,12 @@ tcp_respond(tp, ipgen, th, m, ack, seq, flags)
m->m_pkthdr.len = tlen;
m->m_pkthdr.rcvif = (struct ifnet *) 0;
#ifdef MAC
if (tp != NULL && tp->t_inpcb != NULL) {
if (inp != NULL) {
/*
* Packet is associated with a socket, so allow the
* label of the response to reflect the socket label.
*/
mac_create_mbuf_from_socket(tp->t_inpcb->inp_socket, m);
mac_create_mbuf_from_socket(inp->inp_socket, m);
} else {
/*
* Packet is not associated with a socket, so possibly
@ -510,7 +516,7 @@ tcp_respond(tp, ipgen, th, m, ack, seq, flags)
nth->th_sum = in6_cksum(m, IPPROTO_TCP,
sizeof(struct ip6_hdr),
tlen - sizeof(struct ip6_hdr));
ip6->ip6_hlim = in6_selecthlim(tp ? tp->t_inpcb : NULL,
ip6->ip6_hlim = in6_selecthlim(inp,
ro6 && ro6->ro_rt ?
ro6->ro_rt->rt_ifp :
NULL);
@ -523,26 +529,25 @@ tcp_respond(tp, ipgen, th, m, ack, seq, flags)
m->m_pkthdr.csum_data = offsetof(struct tcphdr, th_sum);
}
#ifdef TCPDEBUG
if (tp == NULL || (tp->t_inpcb->inp_socket->so_options & SO_DEBUG))
if (tp == NULL || (inp->inp_socket->so_options & SO_DEBUG))
tcp_trace(TA_OUTPUT, 0, tp, mtod(m, void *), th, 0);
#endif
#ifdef INET6
if (isipv6) {
(void)ip6_output(m, NULL, ro6, ipflags, NULL, NULL,
tp ? tp->t_inpcb : NULL);
(void) ip6_output(m, NULL, ro6, ipflags, NULL, NULL, inp);
if (ro6 == &sro6 && ro6->ro_rt) {
RTFREE(ro6->ro_rt);
ro6->ro_rt = NULL;
}
} else
#endif /* INET6 */
{
(void) ip_output(m, NULL, ro, ipflags, NULL, tp ? tp->t_inpcb : NULL);
if (ro == &sro && ro->ro_rt) {
RTFREE(ro->ro_rt);
ro->ro_rt = NULL;
{
(void) ip_output(m, NULL, ro, ipflags, NULL, inp);
if (ro == &sro && ro->ro_rt) {
RTFREE(ro->ro_rt);
ro->ro_rt = NULL;
}
}
}
}
/*