Author: avg
Date: Sun Oct  1 16:34:16 2017
New Revision: 324166
URL: https://svnweb.freebsd.org/changeset/base/324166

Log:
  MFV r323531: 8521 nvlist memory leak in get_clones_stat() and spa_load_best()
  
  illumos/illumos-gate@7d3000f774e20097a1ee45cbd06d0e38065ddd5a
  
https://github.com/illumos/illumos-gate/commit/7d3000f774e20097a1ee45cbd06d0e38065ddd5a
  
  https://www.illumos.org/issues/8521
    Yuri reported this to the mailing list:
    doing a `reboot -d` on current illumos-gate HEAD gives the following "::
    findleaks -dv" output:
    findleaks: maximum buffers => 301061
    findleaks: actual buffers => 297587
    findleaks:
    findleaks: potential pointers => 29289774
    findleaks: dismissals => 26242305 (89.5%)
    findleaks: misses => 331153 ( 1.1%)
    findleaks: dups => 2419681 ( 8.2%)
    findleaks: follows => 296635 ( 1.0%)
    findleaks:
    findleaks: peak memory usage => 7353 kB
    findleaks: elapsed CPU time => 1.5 seconds
    findleaks: elapsed wall time => 2.0 seconds
    findleaks:
    CACHE LEAKED BUFCTL CALLER
    ffffff03d222b008 120 ffffff03ef7ceb78 nv_alloc_sys+0x1f
    ffffff03d222a448 123 ffffff03f4150cc8 nv_alloc_sys+0x1f
    ffffff03d222b448 5 ffffff03f28bd598 nv_alloc_sys+0x1f
    ffffff03d222b888 87 ffffff03f28c10f0 nv_alloc_sys+0x1f
    ffffff03d222c008 21 ffffff03f4139310 nv_alloc_sys+0x1f
    ffffff03d222b888 43 ffffff040ef3f3e8 nv_alloc_sys+0x1f
    ffffff03d222c008 120 ffffff03f4591e58 nv_alloc_sys+0x1f
    ffffff03d222b008 121 ffffff03f352c068 nv_alloc_sys+0x1f
    ffffff03d222a448 112 ffffff03f414e5f8 nv_alloc_sys+0x1f
    ffffff03d222b008 119 ffffff03ee92fdc0 nv_alloc_sys+0x1f
    ffffff03d222b888 46 ffffff03f28c1378 nv_alloc_sys+0x1f
    ffffff03d222b448 4 ffffff03f28c7708 nv_alloc_sys+0x1f
    ffffff03d222c008 20 ffffff03f2a6e7e8 nv_alloc_sys+0x1f
  
  Reviewed by: Steve Gonczi <steve.gon...@delphix.com>
  Reviewed by: George Wilson <george.wil...@delphix.com>
  Reviewed by: Yuri Pankov <yur...@gmx.com>
  Reviewed by: Matt Ahrens <mahr...@delphix.com>
  Approved by: Dan McDonald <dan...@joyent.com>
  Author: Pavel Zakharov <pavel.zakha...@delphix.com>
  
  MFC after:    5 weeks
  X-MFC after:  r324163

Modified:
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/spa.c
  head/sys/kern/kern_linker.c
  head/sys/kern/kern_sysctl.c
  head/sys/sys/sysctl.h
Directory Properties:
  head/sys/cddl/contrib/opensolaris/   (props changed)

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c   Sun Oct 
 1 16:29:20 2017        (r324165)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c   Sun Oct 
 1 16:34:16 2017        (r324166)
@@ -1808,10 +1808,10 @@ get_clones_stat(dsl_dataset_t *ds, nvlist_t *nv)
                fnvlist_add_nvlist(propval, ZPROP_VALUE, val);
                fnvlist_add_nvlist(nv, zfs_prop_to_name(ZFS_PROP_CLONES),
                    propval);
-       } else {
-               nvlist_free(val);
-               nvlist_free(propval);
        }
+
+       nvlist_free(val);
+       nvlist_free(propval);
 }
 
 /*

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/spa.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/spa.c   Sun Oct  1 
16:29:20 2017        (r324165)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/spa.c   Sun Oct  1 
16:34:16 2017        (r324166)
@@ -3096,6 +3096,8 @@ spa_load_best(spa_t *spa, spa_load_state_t state, int 
 
        if (config && (rewind_error || state != SPA_LOAD_RECOVER))
                spa_config_set(spa, config);
+       else
+               nvlist_free(config);
 
        if (state == SPA_LOAD_RECOVER) {
                ASSERT3P(loadinfo, ==, NULL);

Modified: head/sys/kern/kern_linker.c
==============================================================================
--- head/sys/kern/kern_linker.c Sun Oct  1 16:29:20 2017        (r324165)
+++ head/sys/kern/kern_linker.c Sun Oct  1 16:34:16 2017        (r324166)
@@ -288,7 +288,7 @@ linker_file_sysuninit(linker_file_t lf)
 }
 
 static void
-linker_file_register_sysctls(linker_file_t lf)
+linker_file_register_sysctls(linker_file_t lf, bool enable)
 {
        struct sysctl_oid **start, **stop, **oidp;
 
@@ -303,8 +303,34 @@ linker_file_register_sysctls(linker_file_t lf)
 
        sx_xunlock(&kld_sx);
        sysctl_wlock();
+       for (oidp = start; oidp < stop; oidp++) {
+               if (enable)
+                       sysctl_register_oid(*oidp);
+               else
+                       sysctl_register_disabled_oid(*oidp);
+       }
+       sysctl_wunlock();
+       sx_xlock(&kld_sx);
+}
+
+static void
+linker_file_enable_sysctls(linker_file_t lf)
+{
+       struct sysctl_oid **start, **stop, **oidp;
+
+       KLD_DPF(FILE,
+           ("linker_file_enable_sysctls: enable SYSCTLs for %s\n",
+           lf->filename));
+
+       sx_assert(&kld_sx, SA_XLOCKED);
+
+       if (linker_file_lookup_set(lf, "sysctl_set", &start, &stop, NULL) != 0)
+               return;
+
+       sx_xunlock(&kld_sx);
+       sysctl_wlock();
        for (oidp = start; oidp < stop; oidp++)
-               sysctl_register_oid(*oidp);
+               sysctl_enable_oid(*oidp);
        sysctl_wunlock();
        sx_xlock(&kld_sx);
 }
@@ -430,8 +456,9 @@ linker_load_file(const char *filename, linker_file_t *
                                return (error);
                        }
                        modules = !TAILQ_EMPTY(&lf->modules);
-                       linker_file_register_sysctls(lf);
+                       linker_file_register_sysctls(lf, false);
                        linker_file_sysinit(lf);
+                       linker_file_enable_sysctls(lf);
                        lf->flags |= LINKER_FILE_LINKED;
 
                        /*
@@ -692,8 +719,8 @@ linker_file_unload(linker_file_t file, int flags)
         */
        if (file->flags & LINKER_FILE_LINKED) {
                file->flags &= ~LINKER_FILE_LINKED;
-               linker_file_sysuninit(file);
                linker_file_unregister_sysctls(file);
+               linker_file_sysuninit(file);
        }
        TAILQ_REMOVE(&linker_files, file, link);
 
@@ -1642,7 +1669,7 @@ restart:
                if (linker_file_lookup_set(lf, "sysinit_set", &si_start,
                    &si_stop, NULL) == 0)
                        sysinit_add(si_start, si_stop);
-               linker_file_register_sysctls(lf);
+               linker_file_register_sysctls(lf, true);
                lf->flags |= LINKER_FILE_LINKED;
                continue;
 fail:

Modified: head/sys/kern/kern_sysctl.c
==============================================================================
--- head/sys/kern/kern_sysctl.c Sun Oct  1 16:29:20 2017        (r324165)
+++ head/sys/kern/kern_sysctl.c Sun Oct  1 16:34:16 2017        (r324166)
@@ -408,8 +408,8 @@ SYSCTL_PROC(_sysctl, 0, reuse_test, CTLTYPE_STRING|CTL
        0, 0, sysctl_reuse_test, "-", "");
 #endif
 
-void
-sysctl_register_oid(struct sysctl_oid *oidp)
+static void
+sysctl_register_oid_impl(struct sysctl_oid *oidp, bool enable)
 {
        struct sysctl_oid_list *parent = oidp->oid_parent;
        struct sysctl_oid *p;
@@ -491,6 +491,17 @@ retry:
        }
        /* update the OID number, if any */
        oidp->oid_number = oid_number;
+
+       /*
+        * Mark the leaf as dormant if it's not to be immediately enabled.
+        * We do not disable nodes as they can be shared between modules
+        * and it is always safe to access a node.
+        */
+       if (!enable && (oidp->oid_kind & CTLTYPE) != CTLTYPE_NODE) {
+               KASSERT((oidp->oid_kind & CTLFLAG_DORMANT) == 0,
+                   ("internal flag is set in oid_kind"));
+               oidp->oid_kind |= CTLFLAG_DORMANT;
+       }
        if (q != NULL)
                SLIST_INSERT_AFTER(q, oidp, oid_link);
        else
@@ -510,6 +521,35 @@ retry:
 }
 
 void
+sysctl_register_oid(struct sysctl_oid *oidp)
+{
+
+       sysctl_register_oid_impl(oidp, true);
+}
+
+void
+sysctl_register_disabled_oid(struct sysctl_oid *oidp)
+{
+
+       sysctl_register_oid_impl(oidp, false);
+}
+
+void
+sysctl_enable_oid(struct sysctl_oid *oidp)
+{
+
+       SYSCTL_ASSERT_WLOCKED();
+       if ((oidp->oid_kind & CTLTYPE) == CTLTYPE_NODE) {
+               KASSERT((oidp->oid_kind & CTLFLAG_DORMANT) == 0,
+                   ("sysctl node is marked as dormant"));
+               return;
+       }
+       KASSERT((oidp->oid_kind & CTLFLAG_DORMANT) != 0,
+           ("enabling already enabled sysctl oid"));
+       oidp->oid_kind &= ~CTLFLAG_DORMANT;
+}
+
+void
 sysctl_unregister_oid(struct sysctl_oid *oidp)
 {
        struct sysctl_oid *p;
@@ -1057,7 +1097,7 @@ sysctl_sysctl_next_ls(struct sysctl_oid_list *lsp, int
                *next = oidp->oid_number;
                *oidpp = oidp;
 
-               if (oidp->oid_kind & CTLFLAG_SKIP)
+               if ((oidp->oid_kind & (CTLFLAG_SKIP | CTLFLAG_DORMANT)) != 0)
                        continue;
 
                if (!namelen) {
@@ -1878,6 +1918,8 @@ sysctl_find_oid(int *name, u_int namelen, struct sysct
                        }
                        lsp = SYSCTL_CHILDREN(oid);
                } else if (indx == namelen) {
+                       if ((oid->oid_kind & CTLFLAG_DORMANT) != 0)
+                               return (ENOENT);
                        *noid = oid;
                        if (nindx != NULL)
                                *nindx = indx;

Modified: head/sys/sys/sysctl.h
==============================================================================
--- head/sys/sys/sysctl.h       Sun Oct  1 16:29:20 2017        (r324165)
+++ head/sys/sys/sysctl.h       Sun Oct  1 16:34:16 2017        (r324166)
@@ -83,6 +83,7 @@ struct ctlname {
 #define        CTLFLAG_RD      0x80000000      /* Allow reads of variable */
 #define        CTLFLAG_WR      0x40000000      /* Allow writes to the variable 
*/
 #define        CTLFLAG_RW      (CTLFLAG_RD|CTLFLAG_WR)
+#define        CTLFLAG_DORMANT 0x20000000      /* This sysctl is not active 
yet */
 #define        CTLFLAG_ANYBODY 0x10000000      /* All users can set this var */
 #define        CTLFLAG_SECURE  0x08000000      /* Permit set only if 
securelevel<=0 */
 #define        CTLFLAG_PRISON  0x04000000      /* Prisoned roots can fiddle */
@@ -219,6 +220,8 @@ int sysctl_dpcpu_quad(SYSCTL_HANDLER_ARGS);
  * These functions are used to add/remove an oid from the mib.
  */
 void sysctl_register_oid(struct sysctl_oid *oidp);
+void sysctl_register_disabled_oid(struct sysctl_oid *oidp);
+void sysctl_enable_oid(struct sysctl_oid *oidp);
 void sysctl_unregister_oid(struct sysctl_oid *oidp);
 
 /* Declare a static oid to allow child oids to be added to it. */
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to