Leaks of inpcb

[rrs]

Summary

The recent move of inpref up in the tcp_subr.c tcp init code brings in the potential for
leaking inp's.

There are at least two places that may return NULL and another that has been missing (i.e. when
the stack switch is occurring and the new stack rejects the switch (for example rack not liking a connection without sack).
Lets fix this by making sure we clean up the reference and we pay attention to the init code like we should.

Test Plan

Validate that we don't leak inpcb's like I see now in NF with the recent changes.

[rscheff]

What happens if tcpcb and inpcb are released without making sure the write lock is released? (This seems a follow up on D25583 , correct?) IIRC, if tcp_newtcpcb() returns NULL, the associated inpcb would be freed in the calling function - but would that mean that the write lock held in some data structure would still linger (and leak memory)?

Would a similar construct not be needed around line 474 - when a non-default stack fails to initialize properly? Or is this not necessary, as the default stack should take over, and if that fails, there is a panic anyway?

Apart from my curiosity, these changes look good to me.

[rrs]

Richard:

First of all the tricky thing to remember about inp locks is that

1. They are recursive

and

2. They do not ever get freed (this via some trickery with UMA)

So the reason I did not test the in_pcbrele_wlocked() return is that if the caller
is holding a lock (it should be) then it is going to unlock it no matter what
you do (return NULL or not). This means that the caller must also have
a reference on the inp (which means it should never return 1 from
the decrement).

Looking at where it gets called from is tcp_usr_attach() which does
in_pcballoc() then calls this create function

the alloc does a reference cnt setup to 1 with INP_WLOCK set.

This means that when you increment the ref you are at 2.
Any return (with a NULL) must decrement the reference, but
you should always get a 0 return from that function since the
pcb is not dead and it has a ref on it.

R