Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys Fix memory leak of llentry#la_opaque



details:   https://anonhg.NetBSD.org/src/rev/9439226359f1
branches:  trunk
changeset: 812475:9439226359f1
user:      ozaki-r <ozaki-r%NetBSD.org@localhost>
date:      Thu Dec 17 02:38:33 2015 +0000

description:
Fix memory leak of llentry#la_opaque

llentry#la_opaque which is for token ring is allocated in arp.c
and freed in arp.c when freeing llentry. However, llentry can be
freed from other places, e.g., lltable_free. In such cases,
la_opaque is never freed.

To fix that, add a new callback (lle_ll_free) to llentry and
register a destruction function of la_opque to it. On freeing a
llentry, we can surely free la_opque via the callback.

diffstat:

 sys/net/if_llatbl.h  |   9 +++++--
 sys/netinet/if_arp.c |  57 +++++++++++++++++++++++++++++----------------------
 2 files changed, 38 insertions(+), 28 deletions(-)

diffs (134 lines):

diff -r b075d389025c -r 9439226359f1 sys/net/if_llatbl.h
--- a/sys/net/if_llatbl.h       Wed Dec 16 23:14:42 2015 +0000
+++ b/sys/net/if_llatbl.h       Thu Dec 17 02:38:33 2015 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_llatbl.h,v 1.6 2015/11/25 06:21:26 ozaki-r Exp $    */
+/*     $NetBSD: if_llatbl.h,v 1.7 2015/12/17 02:38:33 ozaki-r Exp $    */
 /*
  * Copyright (c) 2004 Luigi Rizzo, Alessandro Cerri. All rights reserved.
  * Copyright (c) 2004-2008 Qing Li. All rights reserved.
@@ -79,6 +79,7 @@
        struct lltable           *lle_tbl;
        struct llentries         *lle_head;
        void                    (*lle_free)(struct llentry *);
+       void                    (*lle_ll_free)(struct llentry *);
        struct mbuf              *la_hold;
        int                      la_numheld;  /* # of packets currently held */
        time_t                   la_expire;
@@ -202,9 +203,11 @@
 } while (0)
 
 #define        LLE_FREE_LOCKED(lle) do {                               \
-       if ((lle)->lle_refcnt == 1)                             \
+       if ((lle)->lle_refcnt == 1) {                           \
+               if ((lle)->lle_ll_free != NULL)                 \
+                       (lle)->lle_ll_free(lle);                \
                (lle)->lle_free(lle);                           \
-       else {                                                  \
+       } else {                                                \
                LLE_REMREF(lle);                                \
                LLE_WUNLOCK(lle);                               \
        }                                                       \
diff -r b075d389025c -r 9439226359f1 sys/netinet/if_arp.c
--- a/sys/netinet/if_arp.c      Wed Dec 16 23:14:42 2015 +0000
+++ b/sys/netinet/if_arp.c      Thu Dec 17 02:38:33 2015 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_arp.c,v 1.197 2015/12/16 05:44:59 ozaki-r Exp $     */
+/*     $NetBSD: if_arp.c,v 1.198 2015/12/17 02:38:33 ozaki-r Exp $     */
 
 /*-
  * Copyright (c) 1998, 2000, 2008 The NetBSD Foundation, Inc.
@@ -68,7 +68,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_arp.c,v 1.197 2015/12/16 05:44:59 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_arp.c,v 1.198 2015/12/17 02:38:33 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_ddb.h"
@@ -175,6 +175,11 @@
 static void arp_dad_stop(struct ifaddr *);
 static void arp_dad_duplicated(struct ifaddr *);
 
+static void arp_init_llentry(struct ifnet *, struct llentry *);
+#if NTOKEN > 0
+static void arp_free_llentry_tokenring(struct llentry *);
+#endif
+
 struct ifqueue arpintrq = {
        .ifq_head = NULL,
        .ifq_tail = NULL,
@@ -424,6 +429,30 @@
        return gate;
 }
 
+static void
+arp_init_llentry(struct ifnet *ifp, struct llentry *lle)
+{
+
+       switch (ifp->if_type) {
+#if NTOKEN > 0
+       case IFT_ISO88025:
+               lle->la_opaque = kmem_intr_alloc(sizeof(struct token_rif),
+                   KM_NOSLEEP);
+               lle->lle_ll_free = arp_free_llentry_tokenring;
+               break;
+#endif
+       }
+}
+
+#if NTOKEN > 0
+static void
+arp_free_llentry_tokenring(struct llentry *lle)
+{
+
+       kmem_intr_free(lle->la_opaque, sizeof(struct token_rif));
+}
+#endif
+
 /*
  * Parallel to llc_rtrequest.
  */
@@ -646,20 +675,11 @@
                }
                rt->rt_llinfo = la;
                LLE_ADDREF(la);
-               switch (ifp->if_type) {
-#if NTOKEN > 0
-               case IFT_ISO88025:
-                       la->la_opaque = kmem_alloc(sizeof(struct token_rif),
-                           KM_SLEEP);
-                       break;
-#endif /* NTOKEN > 0 */
-               default:
-                       break;
-               }
                la->la_rt = rt;
                rt->rt_refcnt++;
                rt->rt_flags |= RTF_LLINFO;
                arp_inuse++, arp_allocated++;
+               arp_init_llentry(ifp, la);
 
                LLE_WUNLOCK(la);
                la = NULL;
@@ -682,19 +702,6 @@
                IF_AFDATA_WLOCK(ifp);
                LLE_WLOCK(la);
 
-               if (la->la_opaque != NULL) {
-                       switch (ifp->if_type) {
-#if NTOKEN > 0
-                       case IFT_ISO88025:
-                               kmem_free(la->la_opaque,
-                                   sizeof(struct token_rif));
-                               break;
-#endif /* NTOKEN > 0 */
-                       default:
-                               break;
-                       }
-               }
-
                if (la->la_rt != NULL) {
                        /*
                         * Don't rtfree (may actually free objects) here.



Home | Main Index | Thread Index | Old Index