From 0426c16804e572f088dd293029ab39fa75e1e203 Mon Sep 17 00:00:00 2001 From: Isaac Huang Date: Mon, 3 Mar 2014 20:00:11 -0700 Subject: [PATCH] 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 Signed-off-by: Brian Behlendorf Closes #2158 --- include/sys/fm/util.h | 2 +- module/zfs/fm.c | 41 ++++++++++++++++++++++++++++++++--------- module/zfs/zfs_fm.c | 21 ++++++++++++--------- 3 files changed, 45 insertions(+), 19 deletions(-) diff --git a/include/sys/fm/util.h b/include/sys/fm/util.h index 18fe49073..6ee31764b 100644 --- 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 246b3d2cf..d38cb067e 100644 --- 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 05ee84c19..fb65ec6a6 100644 --- 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 }