From 6917b2b1d9aa04da49623cb452fe1e1390096fb9 Mon Sep 17 00:00:00 2001 From: rwatson Date: Tue, 23 Nov 2004 23:41:20 +0000 Subject: In tcp_reass(), assert the inpcb lock on the passed tcpcb, since the contents of the tcpcb are read and modified in volume. In tcp_input(), replace th comparison with 0 with a comparison with NULL. At the 'findpcb', 'dropafterack', and 'dropwithreset' labels in tcp_input(), assert 'headlocked'. Try to improve consistency between various assertions regarding headlocked to be more informative. MFC after: 2 weeks --- sys/netinet/tcp_input.c | 31 +++++++++++++++++++------------ sys/netinet/tcp_reass.c | 31 +++++++++++++++++++------------ 2 files changed, 38 insertions(+), 24 deletions(-) (limited to 'sys/netinet') diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c index 0dc3e89..0639698 100644 --- a/sys/netinet/tcp_input.c +++ b/sys/netinet/tcp_input.c @@ -229,6 +229,8 @@ tcp_reass(tp, th, tlenp, m) struct socket *so = tp->t_inpcb->inp_socket; int flags; + INP_LOCK_ASSERT(tp->t_inpcb); + /* * XXX: tcp_reass() is rather inefficient with its data structures * and should be rewritten (see NetBSD for optimizations). While @@ -236,10 +238,10 @@ tcp_reass(tp, th, tlenp, m) */ /* - * Call with th==0 after become established to + * Call with th==NULL after become established to * force pre-ESTABLISHED data up to user socket. */ - if (th == 0) + if (th == NULL) goto present; /* @@ -613,6 +615,7 @@ tcp_input(m, off0) INP_INFO_WLOCK(&tcbinfo); headlocked = 1; findpcb: + KASSERT(headlocked, ("tcp_input: findpcb: head not locked")); #ifdef IPFIREWALL_FORWARD /* Grab info from PACKET_TAG_IPFORWARD tag prepended to the chain. */ fwd_tag = m_tag_find(m, PACKET_TAG_IPFORWARD, NULL); @@ -999,7 +1002,7 @@ findpcb: goto drop; } after_listen: - KASSERT(headlocked, ("tcp_input(): after_listen head is not locked")); + KASSERT(headlocked, ("tcp_input: after_listen: head not locked")); INP_LOCK_ASSERT(inp); /* XXX temp debugging */ @@ -1403,8 +1406,8 @@ after_listen: } trimthenstep6: - KASSERT(headlocked, - ("tcp_input(): trimthenstep6 head is not locked")); + KASSERT(headlocked, ("tcp_input: trimthenstep6: head not " + "locked")); INP_LOCK_ASSERT(inp); /* @@ -1974,8 +1977,8 @@ trimthenstep6: } process_ACK: - KASSERT(headlocked, - ("tcp_input(): process_ACK head is not locked")); + KASSERT(headlocked, ("tcp_input: process_ACK: head not " + "locked")); INP_LOCK_ASSERT(inp); acked = th->th_ack - tp->snd_una; @@ -2123,7 +2126,8 @@ process_ACK: */ case TCPS_CLOSING: if (ourfinisacked) { - KASSERT(headlocked, ("headlocked")); + KASSERT(headlocked, ("tcp_input: process_ACK: " + "head not locked")); tcp_twstart(tp); INP_INFO_WUNLOCK(&tcbinfo); m_freem(m); @@ -2158,7 +2162,7 @@ process_ACK: } step6: - KASSERT(headlocked, ("tcp_input(): step6 head is not locked")); + KASSERT(headlocked, ("tcp_input: step6: head not locked")); INP_LOCK_ASSERT(inp); /* @@ -2243,7 +2247,7 @@ step6: tp->rcv_up = tp->rcv_nxt; } dodata: /* XXX */ - KASSERT(headlocked, ("tcp_input(): dodata head is not locked")); + KASSERT(headlocked, ("tcp_input: dodata: head not locked")); INP_LOCK_ASSERT(inp); /* @@ -2351,7 +2355,8 @@ dodata: /* XXX */ * standard timers. */ case TCPS_FIN_WAIT_2: - KASSERT(headlocked == 1, ("headlocked should be 1")); + KASSERT(headlocked == 1, ("tcp_input: dodata: " + "TCP_FIN_WAIT_2: head not locked")); tcp_twstart(tp); INP_INFO_WUNLOCK(&tcbinfo); return; @@ -2379,7 +2384,7 @@ dodata: /* XXX */ (void) tcp_output(tp); check_delack: - KASSERT(headlocked == 1, ("headlocked should be 1")); + KASSERT(headlocked == 1, ("tcp_input: check_delack: head not locked")); INP_LOCK_ASSERT(inp); if (tp->t_flags & TF_DELACK) { tp->t_flags &= ~TF_DELACK; @@ -2391,6 +2396,7 @@ check_delack: return; dropafterack: + KASSERT(headlocked, ("tcp_input: dropafterack: head not locked")); /* * Generate an ACK dropping incoming segment if it occupies * sequence space, where the ACK reflects our state. @@ -2426,6 +2432,7 @@ dropafterack: return; dropwithreset: + KASSERT(headlocked, ("tcp_input: dropwithreset: head not locked")); /* * Generate a RST, dropping incoming segment. * Make ACK acceptable to originator of segment. diff --git a/sys/netinet/tcp_reass.c b/sys/netinet/tcp_reass.c index 0dc3e89..0639698 100644 --- a/sys/netinet/tcp_reass.c +++ b/sys/netinet/tcp_reass.c @@ -229,6 +229,8 @@ tcp_reass(tp, th, tlenp, m) struct socket *so = tp->t_inpcb->inp_socket; int flags; + INP_LOCK_ASSERT(tp->t_inpcb); + /* * XXX: tcp_reass() is rather inefficient with its data structures * and should be rewritten (see NetBSD for optimizations). While @@ -236,10 +238,10 @@ tcp_reass(tp, th, tlenp, m) */ /* - * Call with th==0 after become established to + * Call with th==NULL after become established to * force pre-ESTABLISHED data up to user socket. */ - if (th == 0) + if (th == NULL) goto present; /* @@ -613,6 +615,7 @@ tcp_input(m, off0) INP_INFO_WLOCK(&tcbinfo); headlocked = 1; findpcb: + KASSERT(headlocked, ("tcp_input: findpcb: head not locked")); #ifdef IPFIREWALL_FORWARD /* Grab info from PACKET_TAG_IPFORWARD tag prepended to the chain. */ fwd_tag = m_tag_find(m, PACKET_TAG_IPFORWARD, NULL); @@ -999,7 +1002,7 @@ findpcb: goto drop; } after_listen: - KASSERT(headlocked, ("tcp_input(): after_listen head is not locked")); + KASSERT(headlocked, ("tcp_input: after_listen: head not locked")); INP_LOCK_ASSERT(inp); /* XXX temp debugging */ @@ -1403,8 +1406,8 @@ after_listen: } trimthenstep6: - KASSERT(headlocked, - ("tcp_input(): trimthenstep6 head is not locked")); + KASSERT(headlocked, ("tcp_input: trimthenstep6: head not " + "locked")); INP_LOCK_ASSERT(inp); /* @@ -1974,8 +1977,8 @@ trimthenstep6: } process_ACK: - KASSERT(headlocked, - ("tcp_input(): process_ACK head is not locked")); + KASSERT(headlocked, ("tcp_input: process_ACK: head not " + "locked")); INP_LOCK_ASSERT(inp); acked = th->th_ack - tp->snd_una; @@ -2123,7 +2126,8 @@ process_ACK: */ case TCPS_CLOSING: if (ourfinisacked) { - KASSERT(headlocked, ("headlocked")); + KASSERT(headlocked, ("tcp_input: process_ACK: " + "head not locked")); tcp_twstart(tp); INP_INFO_WUNLOCK(&tcbinfo); m_freem(m); @@ -2158,7 +2162,7 @@ process_ACK: } step6: - KASSERT(headlocked, ("tcp_input(): step6 head is not locked")); + KASSERT(headlocked, ("tcp_input: step6: head not locked")); INP_LOCK_ASSERT(inp); /* @@ -2243,7 +2247,7 @@ step6: tp->rcv_up = tp->rcv_nxt; } dodata: /* XXX */ - KASSERT(headlocked, ("tcp_input(): dodata head is not locked")); + KASSERT(headlocked, ("tcp_input: dodata: head not locked")); INP_LOCK_ASSERT(inp); /* @@ -2351,7 +2355,8 @@ dodata: /* XXX */ * standard timers. */ case TCPS_FIN_WAIT_2: - KASSERT(headlocked == 1, ("headlocked should be 1")); + KASSERT(headlocked == 1, ("tcp_input: dodata: " + "TCP_FIN_WAIT_2: head not locked")); tcp_twstart(tp); INP_INFO_WUNLOCK(&tcbinfo); return; @@ -2379,7 +2384,7 @@ dodata: /* XXX */ (void) tcp_output(tp); check_delack: - KASSERT(headlocked == 1, ("headlocked should be 1")); + KASSERT(headlocked == 1, ("tcp_input: check_delack: head not locked")); INP_LOCK_ASSERT(inp); if (tp->t_flags & TF_DELACK) { tp->t_flags &= ~TF_DELACK; @@ -2391,6 +2396,7 @@ check_delack: return; dropafterack: + KASSERT(headlocked, ("tcp_input: dropafterack: head not locked")); /* * Generate an ACK dropping incoming segment if it occupies * sequence space, where the ACK reflects our state. @@ -2426,6 +2432,7 @@ dropafterack: return; dropwithreset: + KASSERT(headlocked, ("tcp_input: dropwithreset: head not locked")); /* * Generate a RST, dropping incoming segment. * Make ACK acceptable to originator of segment. -- cgit v1.1