On Thu, 14 Feb 2002 itojun@iijlab.net wrote:

> >> Looking over the code, I kinda doubt that key_freesp() is being called
> >> twice on the same entry outside of the cache.  I'm pretty sure that it's
> >> the cache that's doing it.  Also, the pointers to the entry appear to be
> >> copied all over, not just in the cache code.  I don't know that it would
> >> be possible to set all of the pointer to NULL, let alone know where they
> >> all are.
> >
> >Then we might need to start doing refcounting.
>
> 	we do reference-count struct inpcbpolicy.  also, I couldn't find the
> 	problem described in earlier email (pcbsp->cache[x] freed twice).
> 	key_freesp() is called in ipsec_invalpcbcache(), however, after calling
> 	key_freesp() pcbsp->cache[x] is set to NULL.

Hmmm... I see what the concern was. in_pcbdisconnect() will call
ipsec_pcbdisconn(), and might call in_pcbdetach(). The latter will call
ipsec4_delete_pcbpolicy(). Both ipsec routines will call key_freesp().

But looking at key_freesp(), it is doing reference counting. Correctly.
The routine decriments the refcount, and only actually free's if the
refcount is zero.

The only things I can think to do are add invalid refcount checks. So the
places which inc refcounts check for overflow, and key_freesp() checks for
underflow.

Take care,

Bill