Subject: [review please] tcp syn cache cleanup code for sc->sc_so
To: None <tech-net@netbsd.org>
From: None <itojun@iijlab.net>
List: tech-net
Date: 08/20/1999 14:03:59
Hello, could someone review the following patch.
By addition of IPsec code, tcp syn cache has additional member,
sc_so, which points back to socket structure of listening socket.
This is required because, when responding to the connection attempt
at syn_cache_respond(), we need to check the security policy
of relevant listening socket ("need encryption on reply" for example).
There can be little chance of race condition, when:
- you run web server (listening socket on port 80)
- you received SYN on port 80, created syn cache
- web server is killed (by signal, for example) and listening socket
goes away
- sc_so on syn cache still has dangling reference at sc_so, and
may cause some trouble
First of all I'm guilty for the bug, I'm sorry about this.
One obvious fix to this is to totally forget about syn cache, which is
not acceptable.
Another fix would be to forget about syn cache (make copy of
socket structure right after SYN) when ipsec policy is associated
to the socket - this may not be desirable and addes too many confusion
in the code.
The following patch does third option - tries to cleanup reference
to listening socket from syn cache entries when listening socket
goes away. This may not show the best behavior (if listening socket
goes away, ipsec policy will become inaccessible), but this should
protect kenrel from panic'ing.
I would like to know if this fix looks feasible enough.
The diff is based on KAME repository so there may be some difference
in line numbers. Basically tcp_close() calls syn_cache_cleanup()
which checks every entries in syn cache for to-be-dangling pointer.
Relationship betwen syn cache and per-socket ipsec policy is,
I would say, very tricky...
itojun
Index: tcp_input.c
===================================================================
RCS file: /cvsroot/kame/kame/netbsd/sys/netinet/tcp_input.c,v
retrieving revision 1.5
diff -c -r1.5 tcp_input.c
*** tcp_input.c 1999/08/19 12:11:59 1.5
--- tcp_input.c 1999/08/20 04:53:01
***************
*** 2545,2550 ****
--- 2545,2574 ----
}
/*
+ * Remove reference from syn cache to socket structure,
+ * before socket strucuture goes away.
+ */
+ void
+ syn_cache_cleanup(so)
+ struct socket *so;
+ {
+ struct syn_cache *sc;
+ int i, s;
+
+ s = splsoftnet();
+
+ for (i = 0; i < TCP_MAXRXTSHIFT; i++) {
+ for (sc = TAILQ_FIRST(&tcp_syn_cache_timeq[i]);
+ sc != NULL;
+ sc = TAILQ_NEXT(sc, sc_timeq)) {
+ if (sc->sc_so == so)
+ sc->sc_so = NULL;
+ }
+ }
+ splx(s);
+ }
+
+ /*
* Find an entry in the syn cache.
*/
struct syn_cache *
Index: tcp_subr.c
===================================================================
RCS file: /cvsroot/kame/kame/netbsd/sys/netinet/tcp_subr.c,v
retrieving revision 1.5
diff -c -r1.5 tcp_subr.c
*** tcp_subr.c 1999/08/11 06:04:32 1.5
--- tcp_subr.c 1999/08/20 04:53:01
***************
*** 905,916 ****
--- 905,918 ----
pool_put(&tcpcb_pool, tp);
if (inp) {
inp->inp_ppcb = 0;
+ syn_cache_cleanup(so);
soisdisconnected(so);
in_pcbdetach(inp);
}
#ifdef INET6
else if (in6p) {
in6p->in6p_ppcb = 0;
+ syn_cache_cleanup(so);
soisdisconnected(so);
in6_pcbdetach(in6p);
}
Index: tcp_var.h
===================================================================
RCS file: /cvsroot/kame/kame/netbsd/sys/netinet/tcp_var.h,v
retrieving revision 1.4
diff -c -r1.4 tcp_var.h
*** tcp_var.h 1999/08/12 13:58:16 1.4
--- tcp_var.h 1999/08/20 04:53:01
***************
*** 686,691 ****
--- 686,692 ----
struct tcphdr *));
int syn_cache_respond __P((struct syn_cache *, struct mbuf *));
void syn_cache_timer __P((void));
+ void syn_cache_cleanup __P((struct socket *));
int tcp_newreno __P((struct tcpcb *, struct tcphdr *));
#endif