Subject: Re: Possible memory leak for multicast all-hosts address?
To: None <itojun@iijlab.net>
From: enami tsugutomo <enami@but-b.or.jp>
List: tech-net
Date: 10/08/2000 04:49:54
> 	the patch should do the right thing.  please test.

This patch doesn't DTRT.  Just calling in_delmulti() for the multicast
address records listed in ia_multiaddrs is insufficient, since they
are reference counted and is referenced from ip_moptions.

A reference via ip_moptions is already killed by in_pcbpurgeif().

enami.

Index: in.c
===================================================================
RCS file: /cvsroot/syssrc/sys/netinet/in.c,v
retrieving revision 1.64
diff -c -r1.64 in.c
*** in.c	2000/10/08 02:05:47	1.64
--- in.c	2000/10/08 09:08:36
***************
*** 145,161 ****
  int hostzeroisbroadcast = HOSTZEROBROADCAST;
  
  /*
!  * This structure is used to keep track of in6_multi chains which belong to
!  * deleted interface addresses.
   */
! static LIST_HEAD(, multi_kludge) in_mk; /* XXX BSS initialization */
  
- struct multi_kludge {
- 	LIST_ENTRY(multi_kludge) mk_entry;
- 	struct ifnet *mk_ifp;
- 	LIST_HEAD(, in_multi) mk_head;
- };
- 
  /*
   * Return 1 if an internet address is for a ``local'' host
   * (one to which we have a connection).  If subnetsarelocal
--- 145,156 ----
  int hostzeroisbroadcast = HOSTZEROBROADCAST;
  
  /*
!  * This list is used to keep track of in_multi chains which belong to
!  * deleted interface addresses.  We use in_ifaddr so that a chain head
!  * won't be deallocated until all multicast address record are deleted.
   */
! static TAILQ_HEAD(, in_ifaddr) in_mk = TAILQ_HEAD_INITIALIZER(in_mk);
  
  /*
   * Return 1 if an internet address is for a ``local'' host
   * (one to which we have a connection).  If subnetsarelocal
***************
*** 569,587 ****
  	TAILQ_REMOVE(&ifp->if_addrlist, &ia->ia_ifa, ifa_list);
  	IFAFREE(&ia->ia_ifa);
  	TAILQ_REMOVE(&in_ifaddr, ia, ia_list);
! 	if (LIST_FIRST(&ia->ia_multiaddrs) != NULL) {
! 		/*
! 		 * XXX thorpej@netbsd.org -- if the interface is going
! 		 * XXX away, don't save the multicast entries, delete them!
! 		 */
! 		if (ia->ia_ifa.ifa_ifp->if_output == if_nulloutput) {
! 			struct in_multi *inm;
! 
! 			while ((inm = LIST_FIRST(&ia->ia_multiaddrs)) != NULL)
! 				in_delmulti(inm);
! 		} else
! 			in_savemkludge(ia);
! 	}
  	IFAFREE(&ia->ia_ifa);
  	in_setmaxmtu();
  }
--- 564,578 ----
  	TAILQ_REMOVE(&ifp->if_addrlist, &ia->ia_ifa, ifa_list);
  	IFAFREE(&ia->ia_ifa);
  	TAILQ_REMOVE(&in_ifaddr, ia, ia_list);
! 	if (ia->ia_allhosts != NULL)
! 		in_delmulti(ia->ia_allhosts);
! 	if (LIST_FIRST(&ia->ia_multiaddrs) != NULL &&
! 	    /*
! 	     * If the interface is going away, don't bother to save
! 	     * the multicast entries.
! 	     */
! 	    ifp->if_output != if_nulloutput)
! 		in_savemkludge(ia);
  	IFAFREE(&ia->ia_ifa);
  	in_setmaxmtu();
  }
***************
*** 898,908 ****
  	 * If the interface supports multicast, join the "all hosts"
  	 * multicast group on that interface.
  	 */
! 	if (ifp->if_flags & IFF_MULTICAST) {
  		struct in_addr addr;
  
  		addr.s_addr = INADDR_ALLHOSTS_GROUP;
! 		in_addmulti(&addr, ifp);
  	}
  	return (error);
  bad:
--- 889,899 ----
  	 * If the interface supports multicast, join the "all hosts"
  	 * multicast group on that interface.
  	 */
! 	if ((ifp->if_flags & IFF_MULTICAST) != 0 && ia->ia_allhosts == NULL) {
  		struct in_addr addr;
  
  		addr.s_addr = INADDR_ALLHOSTS_GROUP;
! 		ia->ia_allhosts = in_addmulti(&addr, ifp);
  	}
  	return (error);
  bad:
***************
*** 954,960 ****
   * Multicast address kludge:
   * If there were any multicast addresses attached to this interface address,
   * either move them to another address on this interface, or save them until
!  * such time as this interface is reconfigured for IPv6.
   */
  void
  in_savemkludge(oia)
--- 945,951 ----
   * Multicast address kludge:
   * If there were any multicast addresses attached to this interface address,
   * either move them to another address on this interface, or save them until
!  * such time as this interface is reconfigured for IPv4.
   */
  void
  in_savemkludge(oia)
***************
*** 973,997 ****
  			LIST_INSERT_HEAD(&ia->ia_multiaddrs, inm, inm_list);
  		}
  	} else {	/* last address on this if deleted, save */
! 		struct multi_kludge *mk;
! 
! 		mk = malloc(sizeof(*mk), M_IPMADDR, M_WAITOK);
! 
! 		LIST_INIT(&mk->mk_head);
! 		mk->mk_ifp = oia->ia_ifp;
! 
! 		for (inm = oia->ia_multiaddrs.lh_first; inm; inm = next){
! 			next = inm->inm_list.le_next;
! 			IFAFREE(&inm->inm_ia->ia_ifa); /* release reference */
! 			inm->inm_ia = NULL;
! 			LIST_INSERT_HEAD(&mk->mk_head, inm, inm_list);
! 		}
! 
! 		if (mk->mk_head.lh_first != NULL) {
! 			LIST_INSERT_HEAD(&in_mk, mk, mk_entry);
! 		} else {
! 			FREE(mk, M_IPMADDR);
! 		}
  	}
  }
  
--- 964,971 ----
  			LIST_INSERT_HEAD(&ia->ia_multiaddrs, inm, inm_list);
  		}
  	} else {	/* last address on this if deleted, save */
! 		TAILQ_INSERT_TAIL(&in_mk, oia, ia_list);
! 		IFAREF(&oia->ia_ifa);
  	}
  }
  
***************
*** 1005,1025 ****
  	struct in_ifaddr *ia;
  	struct ifnet *ifp;
  {
! 	struct multi_kludge *mk;
  
! 	for (mk = in_mk.lh_first; mk; mk = mk->mk_entry.le_next) {
! 		if (mk->mk_ifp == ifp) {
  			struct in_multi *inm, *next;
  
! 			for (inm = mk->mk_head.lh_first; inm; inm = next){
! 				next = inm->inm_list.le_next;
! 				inm->inm_ia = ia;
  				IFAREF(&ia->ia_ifa);
  				LIST_INSERT_HEAD(&ia->ia_multiaddrs,
! 						 inm, inm_list);
  			}
! 			LIST_REMOVE(mk, mk_entry);
! 			free(mk, M_IPMADDR);
  			break;
  		}
  	}
--- 979,1002 ----
  	struct in_ifaddr *ia;
  	struct ifnet *ifp;
  {
! 	struct in_ifaddr *oia;
  
! 	for (oia = TAILQ_FIRST(&in_mk); oia != NULL;
! 	    oia = TAILQ_NEXT(oia, ia_list)) {
! 		if (oia->ia_ifp == ifp) {
  			struct in_multi *inm, *next;
  
! 			for (inm = LIST_FIRST(&oia->ia_multiaddrs);
! 			    inm != NULL; inm = next) {
! 				next = LIST_NEXT(inm, inm_list);
! 				IFAFREE(&inm->inm_ia->ia_ifa);
  				IFAREF(&ia->ia_ifa);
+ 				inm->inm_ia = ia;
  				LIST_INSERT_HEAD(&ia->ia_multiaddrs,
! 				    inm, inm_list);
  			}
! 	    		TAILQ_REMOVE(&in_mk, oia, ia_list);
! 			IFAFREE(&oia->ia_ifa);
  			break;
  		}
  	}
***************
*** 1029,1046 ****
  in_purgemkludge(ifp)
  	struct ifnet *ifp;
  {
! 	struct multi_kludge *mk;
! 	struct in_multi *inm;
  
! 	for (mk = in_mk.lh_first; mk; mk = mk->mk_entry.le_next) {
! 		if (mk->mk_ifp != ifp)
  			continue;
  
! 		/* leave from all multicast groups joined */
! 		while ((inm = LIST_FIRST(&mk->mk_head)) != NULL)
! 			in_delmulti(inm);
! 		LIST_REMOVE(mk, mk_entry);
! 		free(mk, M_IPMADDR);
  		break;
  	}
  }
--- 1006,1025 ----
  in_purgemkludge(ifp)
  	struct ifnet *ifp;
  {
! 	struct in_ifaddr *oia;
  
! 	for (oia = TAILQ_FIRST(&in_mk); oia != NULL;
! 	    oia = TAILQ_NEXT(oia, ia_list)) {
! 		if (oia->ia_ifp != ifp)
  			continue;
+ 
+ 		/*
+ 		 * Leaving from all multicast groups joined through
+ 		 * this interface is done via in_pcbpurgeif().
+ 		 */
  
! 	    	TAILQ_REMOVE(&in_mk, oia, ia_list);
! 		IFAFREE(&oia->ia_ifa);
  		break;
  	}
  }
Index: in_var.h
===================================================================
RCS file: /cvsroot/syssrc/sys/netinet/in_var.h,v
retrieving revision 1.40
diff -c -r1.40 in_var.h
*** in_var.h	2000/10/08 02:05:48	1.40
--- in_var.h	2000/10/08 09:08:36
***************
*** 100,105 ****
--- 100,107 ----
  #define	ia_broadaddr	ia_dstaddr
  	struct	sockaddr_in ia_sockmask; /* reserve space for general netmask */
  	LIST_HEAD(, in_multi) ia_multiaddrs; /* list of multicast addresses */
+ 	struct	in_multi *ia_allhosts;	/* multicast address record for
+ 					   the allhosts multicast group */
  };
  
  struct	in_aliasreq {