From: Sara Hartse Date: Fri, 5 Apr 2019 01:57:06 +0000 (-0700) Subject: Restrict kstats and print real pointers X-Git-Tag: zfs-0.8.0-rc4~34 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a887d653b32aaba3fe04c7b25ff0091b9ea9c64e;p=zfs Restrict kstats and print real pointers There are several places where we use zfs_dbgmsg and %p to print pointers. In the Linux kernel, these values obfuscated to prevent information leaks which means the pointers aren't very useful for debugging crash dumps. We decided to restrict the permissions of dbgmsg (and some other kstats while we were at it) and print pointers with %px in zfs_dbgmsg as well as spl_dumpstack Reviewed-by: Brian Behlendorf Reviewed-by: John Gallagher Signed-off-by: sara hartse Closes #8467 Closes #8476 --- diff --git a/cmd/ztest/ztest.c b/cmd/ztest/ztest.c index a1ab56bd2..9c2cf9501 100644 --- a/cmd/ztest/ztest.c +++ b/cmd/ztest/ztest.c @@ -6468,7 +6468,7 @@ ztest_initialize(ztest_ds_t *zd, uint64_t id) char *path = strdup(rand_vd->vdev_path); boolean_t active = rand_vd->vdev_initialize_thread != NULL; - zfs_dbgmsg("vd %p, guid %llu", rand_vd, guid); + zfs_dbgmsg("vd %px, guid %llu", rand_vd, guid); spa_config_exit(spa, SCL_VDEV, FTAG); uint64_t cmd = ztest_random(POOL_INITIALIZE_FUNCS); diff --git a/include/spl/sys/debug.h b/include/spl/sys/debug.h index d9336c7d1..b17d77d28 100644 --- a/include/spl/sys/debug.h +++ b/include/spl/sys/debug.h @@ -102,7 +102,7 @@ void spl_dumpstack(void); if (!(_verify3_left OP _verify3_right)) \ spl_panic(__FILE__, __FUNCTION__, __LINE__, \ "VERIFY3(" #LEFT " " #OP " " #RIGHT ") " \ - "failed (%p " #OP " %p)\n", \ + "failed (%px" #OP " %px)\n", \ (void *) (_verify3_left), \ (void *) (_verify3_right)); \ } while (0) diff --git a/include/spl/sys/kstat.h b/include/spl/sys/kstat.h index 53274d8f5..3ce474248 100644 --- a/include/spl/sys/kstat.h +++ b/include/spl/sys/kstat.h @@ -196,7 +196,7 @@ extern kstat_t *__kstat_create(const char *ks_module, int ks_instance, extern void kstat_proc_entry_init(kstat_proc_entry_t *kpep, const char *module, const char *name); extern void kstat_proc_entry_delete(kstat_proc_entry_t *kpep); -extern void kstat_proc_entry_install(kstat_proc_entry_t *kpep, +extern void kstat_proc_entry_install(kstat_proc_entry_t *kpep, mode_t mode, const struct file_operations *file_ops, void *data); extern void __kstat_install(kstat_t *ksp); diff --git a/include/spl/sys/procfs_list.h b/include/spl/sys/procfs_list.h index cbcb4bcff..eb1519c0a 100644 --- a/include/spl/sys/procfs_list.h +++ b/include/spl/sys/procfs_list.h @@ -58,6 +58,7 @@ typedef struct procfs_list_node { void procfs_list_install(const char *module, const char *name, + mode_t mode, procfs_list_t *procfs_list, int (*show)(struct seq_file *f, void *p), int (*show_header)(struct seq_file *f), diff --git a/include/sys/zfs_context.h b/include/sys/zfs_context.h index 87ddde30a..e3fa2e61b 100644 --- a/include/sys/zfs_context.h +++ b/include/sys/zfs_context.h @@ -375,6 +375,7 @@ typedef struct procfs_list_node { void procfs_list_install(const char *module, const char *name, + mode_t mode, procfs_list_t *procfs_list, int (*show)(struct seq_file *f, void *p), int (*show_header)(struct seq_file *f), diff --git a/lib/libzpool/kernel.c b/lib/libzpool/kernel.c index 926f4f4f4..f22ad56b5 100644 --- a/lib/libzpool/kernel.c +++ b/lib/libzpool/kernel.c @@ -437,6 +437,7 @@ seq_printf(struct seq_file *m, const char *fmt, ...) void procfs_list_install(const char *module, const char *name, + mode_t mode, procfs_list_t *procfs_list, int (*show)(struct seq_file *f, void *p), int (*show_header)(struct seq_file *f), diff --git a/module/spl/spl-kstat.c b/module/spl/spl-kstat.c index 7207a35e0..feff31e6c 100644 --- a/module/spl/spl-kstat.c +++ b/module/spl/spl-kstat.c @@ -659,7 +659,7 @@ kstat_detect_collision(kstat_proc_entry_t *kpep) * kstat. */ void -kstat_proc_entry_install(kstat_proc_entry_t *kpep, +kstat_proc_entry_install(kstat_proc_entry_t *kpep, mode_t mode, const struct file_operations *file_ops, void *data) { kstat_module_t *module; @@ -693,7 +693,7 @@ kstat_proc_entry_install(kstat_proc_entry_t *kpep, list_add_tail(&kpep->kpe_list, &module->ksm_kstat_list); kpep->kpe_owner = module; - kpep->kpe_proc = proc_create_data(kpep->kpe_name, 0644, + kpep->kpe_proc = proc_create_data(kpep->kpe_name, mode, module->ksm_proc, file_ops, data); if (kpep->kpe_proc == NULL) { list_del_init(&kpep->kpe_list); @@ -710,7 +710,15 @@ void __kstat_install(kstat_t *ksp) { ASSERT(ksp); - kstat_proc_entry_install(&ksp->ks_proc, &proc_kstat_operations, ksp); + mode_t mode; + /* Specify permission modes for different kstats */ + if (strncmp(ksp->ks_proc.kpe_name, "dbufs", KSTAT_STRLEN) == 0) { + mode = 0600; + } else { + mode = 0644; + } + kstat_proc_entry_install( + &ksp->ks_proc, mode, &proc_kstat_operations, ksp); } EXPORT_SYMBOL(__kstat_install); diff --git a/module/spl/spl-procfs-list.c b/module/spl/spl-procfs-list.c index 4902e0a56..f6a00da5c 100644 --- a/module/spl/spl-procfs-list.c +++ b/module/spl/spl-procfs-list.c @@ -201,6 +201,7 @@ static struct file_operations procfs_list_operations = { void procfs_list_install(const char *module, const char *name, + mode_t mode, procfs_list_t *procfs_list, int (*show)(struct seq_file *f, void *p), int (*show_header)(struct seq_file *f), @@ -218,7 +219,7 @@ procfs_list_install(const char *module, procfs_list->pl_node_offset = procfs_list_node_off; kstat_proc_entry_init(&procfs_list->pl_kstat_entry, module, name); - kstat_proc_entry_install(&procfs_list->pl_kstat_entry, + kstat_proc_entry_install(&procfs_list->pl_kstat_entry, mode, &procfs_list_operations, procfs_list); } EXPORT_SYMBOL(procfs_list_install); diff --git a/module/zfs/arc.c b/module/zfs/arc.c index 50d0125df..c72487894 100644 --- a/module/zfs/arc.c +++ b/module/zfs/arc.c @@ -2266,7 +2266,7 @@ arc_buf_fill(arc_buf_t *buf, spa_t *spa, const zbookmark_phys_t *zb, */ if (error != 0) { zfs_dbgmsg( - "hdr %p, compress %d, psize %d, lsize %d", + "hdr %px, compress %d, psize %d, lsize %d", hdr, arc_hdr_get_compress(hdr), HDR_GET_PSIZE(hdr), HDR_GET_LSIZE(hdr)); if (hash_lock != NULL) diff --git a/module/zfs/metaslab.c b/module/zfs/metaslab.c index 06d8383f0..ec89810b4 100644 --- a/module/zfs/metaslab.c +++ b/module/zfs/metaslab.c @@ -2643,7 +2643,7 @@ metaslab_condense(metaslab_t *msp, uint64_t txg, dmu_tx_t *tx) ASSERT(msp->ms_loaded); - zfs_dbgmsg("condensing: txg %llu, msp[%llu] %p, vdev id %llu, " + zfs_dbgmsg("condensing: txg %llu, msp[%llu] %px, vdev id %llu, " "spa %s, smp size %llu, segments %lu, forcing condense=%s", txg, msp->ms_id, msp, msp->ms_group->mg_vd->vdev_id, msp->ms_group->mg_vd->vdev_spa->spa_name, diff --git a/module/zfs/range_tree.c b/module/zfs/range_tree.c index 1a31c1129..391533b3f 100644 --- a/module/zfs/range_tree.c +++ b/module/zfs/range_tree.c @@ -118,7 +118,7 @@ range_tree_stat_verify(range_tree_t *rt) for (i = 0; i < RANGE_TREE_HISTOGRAM_SIZE; i++) { if (hist[i] != rt->rt_histogram[i]) { - zfs_dbgmsg("i=%d, hist=%p, hist=%llu, rt_hist=%llu", + zfs_dbgmsg("i=%d, hist=%px, hist=%llu, rt_hist=%llu", i, hist, hist[i], rt->rt_histogram[i]); } VERIFY3U(hist[i], ==, rt->rt_histogram[i]); diff --git a/module/zfs/spa_stats.c b/module/zfs/spa_stats.c index 3b51250c6..6895428f4 100644 --- a/module/zfs/spa_stats.c +++ b/module/zfs/spa_stats.c @@ -131,6 +131,7 @@ spa_read_history_init(spa_t *spa) shl->procfs_list.pl_private = shl; procfs_list_install(module, "reads", + 0600, &shl->procfs_list, spa_read_history_show, spa_read_history_show_header, @@ -301,6 +302,7 @@ spa_txg_history_init(spa_t *spa) shl->procfs_list.pl_private = shl; procfs_list_install(module, "txgs", + 0644, &shl->procfs_list, spa_txg_history_show, spa_txg_history_show_header, @@ -706,6 +708,7 @@ spa_mmp_history_init(spa_t *spa) shl->procfs_list.pl_private = shl; procfs_list_install(module, "multihost", + 0644, &shl->procfs_list, spa_mmp_history_show, spa_mmp_history_show_header, diff --git a/module/zfs/space_map.c b/module/zfs/space_map.c index 5cf3feaae..d9cd8767e 100644 --- a/module/zfs/space_map.c +++ b/module/zfs/space_map.c @@ -848,7 +848,7 @@ space_map_truncate(space_map_t *sm, int blocksize, dmu_tx_t *tx) doi.doi_bonus_size != sizeof (space_map_phys_t)) || doi.doi_data_block_size != blocksize || doi.doi_metadata_block_size != 1 << space_map_ibs) { - zfs_dbgmsg("txg %llu, spa %s, sm %p, reallocating " + zfs_dbgmsg("txg %llu, spa %s, sm %px, reallocating " "object[%llu]: old bonus %u, old blocksz %u", dmu_tx_get_txg(tx), spa_name(spa), sm, sm->sm_object, doi.doi_bonus_size, doi.doi_data_block_size); diff --git a/module/zfs/vdev_removal.c b/module/zfs/vdev_removal.c index 99d67b7be..f2d18d925 100644 --- a/module/zfs/vdev_removal.c +++ b/module/zfs/vdev_removal.c @@ -340,7 +340,7 @@ vdev_remove_initiate_sync(void *arg, dmu_tx_t *tx) */ vdev_config_dirty(vd); - zfs_dbgmsg("starting removal thread for vdev %llu (%p) in txg %llu " + zfs_dbgmsg("starting removal thread for vdev %llu (%px) in txg %llu " "im_obj=%llu", vd->vdev_id, vd, dmu_tx_get_txg(tx), vic->vic_mapping_object); diff --git a/module/zfs/zfs_debug.c b/module/zfs/zfs_debug.c index 62c59e020..538533d27 100644 --- a/module/zfs/zfs_debug.c +++ b/module/zfs/zfs_debug.c @@ -94,6 +94,7 @@ zfs_dbgmsg_init(void) { procfs_list_install("zfs", "dbgmsg", + 0600, &zfs_dbgmsgs, zfs_dbgmsg_show, zfs_dbgmsg_show_header, diff --git a/module/zfs/zil.c b/module/zfs/zil.c index e5de8b5e4..a70fe1a62 100644 --- a/module/zfs/zil.c +++ b/module/zfs/zil.c @@ -3232,7 +3232,7 @@ zil_close(zilog_t *zilog) txg_wait_synced(zilog->zl_dmu_pool, txg); if (zilog_is_dirty(zilog)) - zfs_dbgmsg("zil (%p) is dirty, txg %llu", zilog, txg); + zfs_dbgmsg("zil (%px) is dirty, txg %llu", zilog, txg); if (txg < spa_freeze_txg(zilog->zl_spa)) VERIFY(!zilog_is_dirty(zilog)); diff --git a/module/zfs/zio.c b/module/zfs/zio.c index 1915de417..016ac07ea 100644 --- a/module/zfs/zio.c +++ b/module/zfs/zio.c @@ -1893,7 +1893,7 @@ zio_deadman_impl(zio_t *pio, int ziodepth) uint64_t delta = gethrtime() - pio->io_timestamp; uint64_t failmode = spa_get_deadman_failmode(pio->io_spa); - zfs_dbgmsg("slow zio[%d]: zio=%p timestamp=%llu " + zfs_dbgmsg("slow zio[%d]: zio=%px timestamp=%llu " "delta=%llu queued=%llu io=%llu " "path=%s last=%llu " "type=%d priority=%d flags=0x%x " @@ -3444,7 +3444,7 @@ zio_dva_allocate(zio_t *zio) } if (error != 0) { - zfs_dbgmsg("%s: metaslab allocation failure: zio %p, " + zfs_dbgmsg("%s: metaslab allocation failure: zio %px, " "size %llu, error %d", spa_name(spa), zio, zio->io_size, error); if (error == ENOSPC && zio->io_size > SPA_MINBLOCKSIZE) diff --git a/tests/zfs-tests/tests/functional/cli_user/misc/dbufstat_001_pos.ksh b/tests/zfs-tests/tests/functional/cli_user/misc/dbufstat_001_pos.ksh index 95f0598c6..0e187015f 100755 --- a/tests/zfs-tests/tests/functional/cli_user/misc/dbufstat_001_pos.ksh +++ b/tests/zfs-tests/tests/functional/cli_user/misc/dbufstat_001_pos.ksh @@ -33,11 +33,11 @@ log_assert "dbufstat generates output and doesn't return an error code" typeset -i i=0 while [[ $i -lt ${#args[*]} ]]; do - log_must eval "dbufstat ${args[i]} > /dev/null" + log_must eval "sudo dbufstat ${args[i]} > /dev/null" ((i = i + 1)) done # A simple test of dbufstat filter functionality -log_must eval "dbufstat -F object=10,dbc=1,pool=$TESTPOOL > /dev/null" +log_must eval "sudo dbufstat -F object=10,dbc=1,pool=$TESTPOOL > /dev/null" log_pass "dbufstat generates output and doesn't return an error code"