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 <[email protected]> Reviewed-by: John Gallagher <[email protected]> Signed-off-by: sara hartse <[email protected]> Closes openzfs#8467 Closes openzfs#8476
zfsonfreebsd · Apr 5, 2019 · a887d65 · a887d65
1 parent af65079
commit a887d65
Show file tree

Hide file tree

Showing 18 changed files with 33 additions and 17 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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"