Can tcp_close() be called in INP_TIMEWAIT case

[John Baldwin]

On Thursday, September 24, 2015 02:13:24 PM Julien Charbon wrote:
>  So the issue is:
> 
>  - tcp_close() is called for some reasons with an inp in INP_TIMEWAIT
> state and sets the INP_DROPPED flag,
>  - tcp_detach() is called when the last reference on socket is dropped
> 
>  then now in_pcbfree() can be called twice instead of once:
> 
>  1. First in tcp_detach():
> 
> static void
> tcp_detach(struct socket *so, struct inpcb *inp)
> {
>         struct tcpcb *tp;
>         tp = intotcpcb(inp);
> 
>         if (inp->inp_flags & INP_TIMEWAIT) {
>                 if (inp->inp_flags & INP_DROPPED) {
>                         in_pcbdetach(inp);
>                         in_pcbfree(inp); <--
>                 }
> 
>  2. Second when tcptw expires here:
> 
> void
> tcp_twclose(struct tcptw *tw, int reuse)
> {
>         struct socket *so;
>         struct inpcb *inp;
> 
>         inp = tw->tw_inpcb;
> 
>         tcp_tw_2msl_stop(tw, reuse);
>         inp->inp_ppcb = NULL;
>         in_pcbdrop(inp);
> 
>         so = inp->inp_socket;
>         if (so != NULL) {
>                 ...
>         } else {
>                 in_pcbfree(inp); <--
>         }
> 
>  This behavior is backed by Palle kernel panic backstraces and coredumps.
> 
>  o Solutions:
> 
>  Long:  Forbid to call tcp_close() when inp is in INP_TIMEWAIT state,
> the TCP stack rule being:
> 
>  - if !INP_TIMEWAIT: Call tcp_close()
>  - if INP_TIMEWAIT: Call tcp_twclose() (or call nothing, the tcptw will
> expire/be recycled anyway)
> 
>  Short:
>   if INP_TIMEWAIT & INP_DROPPED:
>     Do not call in_pcbfree(inp) in tcp_detach() unless tcptw is already
> discarded.
> 
>  The long solution seems cleaner, backed by tcp_detach() old comments
> and most of current tcp_close() calls but I won't take that longer path
> without -net approval first.

I prefer the longer solution if it keeps tcp_detach() simpler by avoiding
an extra condition.  Please just document it via assertions in tcp_close()
(or is this the assertion that fired and triggered the reported panic?  If so,
then you obviously don't need to add it. :-P)

-- 
John Baldwin