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 <behlendorf1@llnl.gov>
Reviewed-by: John Gallagher <john.gallagher@delphix.com>
Signed-off-by: sara hartse <sara.hartse@delphix.com>
Closes openzfs#8467 
Closes openzfs#8476

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);

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)

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);

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),

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),

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),

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);
 

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);

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)

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,

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]);

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,

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);

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);
 

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,

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));
 

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)

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"