Fixed memory leaks in zevent handling

Some nvlist_t could be leaked in error handling paths.
Also make sure cb argument to zfs_zevent_post() cannnot
be NULL.

Signed-off-by: Isaac Huang <he.huang@intel.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #2158

diff --git a/include/sys/fm/util.h b/include/sys/fm/util.h
index 18fe490732394168403cd718a2e9d7ae354c66df..6ee31764bfac651e5c661a488bfb20b2906c11fe 100644 (file)
--- a/include/sys/fm/util.h
+++ b/include/sys/fm/util.h
@@ -93,7 +93,7 @@ typedef struct zfs_zevent {
 extern void fm_init(void);
 extern void fm_fini(void);
 extern void fm_nvprint(nvlist_t *);
-extern void zfs_zevent_post(nvlist_t *, nvlist_t *, zevent_cb_t *);
+extern int zfs_zevent_post(nvlist_t *, nvlist_t *, zevent_cb_t *);
 extern void zfs_zevent_drain_all(int *);
 extern int zfs_zevent_fd_hold(int, minor_t *, zfs_zevent_t **);
 extern void zfs_zevent_fd_rele(int);

diff --git a/module/zfs/fm.c b/module/zfs/fm.c
index 246b3d2cf606d01742c5bd989cd3f29024fda3ea..d38cb067e12a330094c7ac809bf7b628248766e0 100644 (file)
--- a/module/zfs/fm.c
+++ b/module/zfs/fm.c
@@ -499,9 +499,13 @@ zfs_zevent_insert(zevent_t *ev)
 }
 
 /*
- * Post a zevent
+ * Post a zevent. The cb will be called when nvl and detector are no longer
+ * needed, i.e.:
+ * - An error happened and a zevent can't be posted. In this case, cb is called
+ *   before zfs_zevent_post() returns.
+ * - The event is being drained and freed.
  */
-void
+int
 zfs_zevent_post(nvlist_t *nvl, nvlist_t *detector, zevent_cb_t *cb)
 {
        int64_t tv_array[2];
@@ -509,25 +513,37 @@ zfs_zevent_post(nvlist_t *nvl, nvlist_t *detector, zevent_cb_t *cb)
        uint64_t eid;
        size_t nvl_size = 0;
        zevent_t *ev;
+       int error;
+
+       ASSERT(cb != NULL);
 
        gethrestime(&tv);
        tv_array[0] = tv.tv_sec;
        tv_array[1] = tv.tv_nsec;
-       if (nvlist_add_int64_array(nvl, FM_EREPORT_TIME, tv_array, 2)) {
+
+       error = nvlist_add_int64_array(nvl, FM_EREPORT_TIME, tv_array, 2);
+       if (error) {
                atomic_add_64(&erpt_kstat_data.erpt_set_failed.value.ui64, 1);
-               return;
+               goto out;
        }
 
        eid = atomic_inc_64_nv(&zevent_eid);
-       if (nvlist_add_uint64(nvl, FM_EREPORT_EID, eid)) {
+       error = nvlist_add_uint64(nvl, FM_EREPORT_EID, eid);
+       if (error) {
                atomic_add_64(&erpt_kstat_data.erpt_set_failed.value.ui64, 1);
-               return;
+               goto out;
+       }
+
+       error = nvlist_size(nvl, &nvl_size, NV_ENCODE_NATIVE);
+       if (error) {
+               atomic_add_64(&erpt_kstat_data.erpt_dropped.value.ui64, 1);
+               goto out;
        }
 
-       (void) nvlist_size(nvl, &nvl_size, NV_ENCODE_NATIVE);
        if (nvl_size > ERPT_DATA_SZ || nvl_size == 0) {
                atomic_add_64(&erpt_kstat_data.erpt_dropped.value.ui64, 1);
-               return;
+               error = EOVERFLOW;
+               goto out;
        }
 
        if (zfs_zevent_console)
@@ -536,7 +552,8 @@ zfs_zevent_post(nvlist_t *nvl, nvlist_t *detector, zevent_cb_t *cb)
        ev = zfs_zevent_alloc();
        if (ev == NULL) {
                atomic_add_64(&erpt_kstat_data.erpt_dropped.value.ui64, 1);
-               return;
+               error = ENOMEM;
+               goto out;
        }
 
        ev->ev_nvl = nvl;
@@ -548,6 +565,12 @@ zfs_zevent_post(nvlist_t *nvl, nvlist_t *detector, zevent_cb_t *cb)
        zfs_zevent_insert(ev);
        cv_broadcast(&zevent_cv);
        mutex_exit(&zevent_lock);
+
+out:
+       if (error)
+               cb(nvl, detector);
+
+       return (error);
 }
 
 static int

diff --git a/module/zfs/zfs_fm.c b/module/zfs/zfs_fm.c
index 05ee84c19e4d20e020b7b769601118bf099907d0..fb65ec6a65253dab7191204ae99ccf98408d67df 100644 (file)
--- a/module/zfs/zfs_fm.c
+++ b/module/zfs/zfs_fm.c
@@ -112,6 +112,11 @@ zfs_zevent_post_cb(nvlist_t *nvl, nvlist_t *detector)
                fm_nvlist_destroy(detector, FM_NVA_FREE);
 }
 
+static void
+zfs_zevent_post_cb_noop(nvlist_t *nvl, nvlist_t *detector)
+{
+}
+
 static void
 zfs_ereport_start(nvlist_t **ereport_out, nvlist_t **detector_out,
     const char *subclass, spa_t *spa, vdev_t *vd, zio_t *zio,
@@ -768,12 +773,7 @@ zfs_ereport_start_checksum(spa_t *spa, vdev_t *vd,
            FM_EREPORT_ZFS_CHECKSUM, spa, vd, zio, offset, length);
 
        if (report->zcr_ereport == NULL) {
-               report->zcr_free(report->zcr_cbdata, report->zcr_cbinfo);
-               if (report->zcr_ckinfo != NULL) {
-                       kmem_free(report->zcr_ckinfo,
-                           sizeof (*report->zcr_ckinfo));
-               }
-               kmem_free(report, sizeof (*report));
+               zfs_ereport_free_checksum(report);
                return;
        }
 #endif
@@ -789,13 +789,15 @@ zfs_ereport_finish_checksum(zio_cksum_report_t *report,
     const void *good_data, const void *bad_data, boolean_t drop_if_identical)
 {
 #ifdef _KERNEL
-       zfs_ecksum_info_t *info = NULL;
+       zfs_ecksum_info_t *info;
+
        info = annotate_ecksum(report->zcr_ereport, report->zcr_ckinfo,
            good_data, bad_data, report->zcr_length, drop_if_identical);
-
        if (info != NULL)
                zfs_zevent_post(report->zcr_ereport,
                    report->zcr_detector, zfs_zevent_post_cb);
+       else
+               zfs_zevent_post_cb(report->zcr_ereport, report->zcr_detector);
 
        report->zcr_ereport = report->zcr_detector = NULL;
        if (info != NULL)
@@ -826,7 +828,8 @@ void
 zfs_ereport_send_interim_checksum(zio_cksum_report_t *report)
 {
 #ifdef _KERNEL
-       zfs_zevent_post(report->zcr_ereport, report->zcr_detector, NULL);
+       zfs_zevent_post(report->zcr_ereport, report->zcr_detector,
+           zfs_zevent_post_cb_noop);
 #endif
 }