rev. 1.94 of netinet/in.c broke CARP

[Gleb Smirnoff]

  Bruce,

On Thu, Jan 25, 2007 at 05:38:43PM +0000, Bruce M. Simpson wrote:
B> Gleb Smirnoff wrote:
B> >  I've just discovered, that revision 1.94 of in.c has broke CARP. This
B> >change adds a code to in_ifdetach() that goes through the global list
B> >of all multicast instances and deletes all the instances, that are
B> >belonging to a particular interface. This is intended to avoid leaking
B> >multicast instances.
B> >  
B> The irony of this of course is that I was working on two separate fixes; 
B> one to prevent the MROUTING code panicking when an interface was 
B> suddenly removed, and the other to prevent the netinet ifp detach path 
B> from panicking in the same circumstances.
B> 
B> These are resource leaks which have been in BSD for years and years. I 
B> neglected to test them *together* however; carp(4) was being used in the 
B> test cases for both bugs.

Yes, I knew that we were leaking memory allocated for multicast purposes
on interface detach. I was trying to fix this with Oleg and Yar, but failed.

B> >  Before this change, most of the subsystems, that allocated multicast
B> >membership instances had freed is theirselves. I don't know about others,
B> >but at least CARP is broken now. It attempts to free a memory, that
B> >already has been freed.
B> >  
B> I would suggest that the correct fix, for now, would be for carp(4) to 
B> now *not* perform its own cleanup for the IPv4 groups it joins on member 
B> interfaces.

Unfortunately, this won't be a correct fix. In a scenario when the parent
interface stays on its place, but you are creating, attaching and
destroying a CARP interface, the multicast membership would not be
left and memory won't be freed. So, after the following sequence

ifconfig fxp0 10.0.0.1/24
ifconfig carp0 create
ifconfig carp0 vhid 1 10.0.0.2/24
ifconfig carp0 destroy

, we would still have a multicast membership on fxp0.

B> The symptom here is that carp(4) needs to join a multicast group on its 
B> member interface. When the interface goes away, the group membership is 
B> now destroyed, at the netinet global level, by the netinet detach path 
B> first.
B> 
B> However, carp(4) is keeping its own imo_membership vector of the 
B> addresses it joined on its member interfaces (rather than using the one 
B> which netinet assigns to it in its attach path), and later tries to free 
B> these memberships.
B> 
B> netinet6 does not have the same problem because in6 memberships are 
B> reference counted.
B> 
B> The root problem is that we should be using consistent semantics for 
B> both the IPv4 and IPv6 paths, and the kernel APIs where soft-ifnets 
B> (such as carp(4)) and routing code (such as MROUTING) need to manipulate 
B> multicast group memberships.

Looks like we need refcounting in IPv4, too.

-- 
Totus tuus, Glebius.
GLEBIUS-RIPN GLEB-RIPE