From 4d451bae8a31d09d81a502867eb6a4b6f8001291 Mon Sep 17 00:00:00 2001 From: Rob Norris Date: Tue, 11 Nov 2025 11:16:30 +1100 Subject: [PATCH] libspl: hide global data objects Currently libspl is a static archive that is linked into multiple shared objects, which then re-export its symbols. We intend to fix this soon. For the moment though, most programs shipped with OpenZFS depend on two or more of these shared objects, and see the same symbols twice. For functions this is not a problem, as they do not have any mutable state and so the linker can simply select the first one and use that for all. For global data objects however, each shared object will have direct (non-relocatable) references to its own instance of the symbol, such that changes on one will not necessarily be seen by the other. While this shouldn't be a problem in practice as these reexported interfaces are not supposed to be used, they are technically undefined behaviour in C (C17 6.9.2) and are reported by ASAN as a violation of C++'s "One Definition Rule". To fix this, we hide these globals inside their compilation units, and add access functions and macros as appropriate to preserve the existing API (though not ABI). Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf Signed-off-by: Rob Norris Closes #17861 --- cmd/zstream/zstream_redup.c | 4 ++-- cmd/ztest.c | 19 ++++------------- lib/libspl/include/sys/misc.h | 3 --- lib/libspl/include/sys/systm.h | 4 +++- lib/libspl/include/sys/taskq.h | 7 +++++-- lib/libspl/include/sys/thread.h | 6 ++---- lib/libspl/libspl.c | 13 +++++++++--- lib/libspl/random.c | 15 +++++++++---- lib/libspl/taskq.c | 37 ++++++++++++++++++--------------- lib/libspl/thread.c | 2 +- 10 files changed, 58 insertions(+), 52 deletions(-) diff --git a/cmd/zstream/zstream_redup.c b/cmd/zstream/zstream_redup.c index 0e18c5249..c1cb6d4d3 100644 --- a/cmd/zstream/zstream_redup.c +++ b/cmd/zstream/zstream_redup.c @@ -191,9 +191,9 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose) #ifdef _ILP32 uint64_t max_rde_size = SMALLEST_POSSIBLE_MAX_RDT_MB << 20; #else - uint64_t physmem = sysconf(_SC_PHYS_PAGES) * sysconf(_SC_PAGESIZE); + uint64_t physbytes = sysconf(_SC_PHYS_PAGES) * sysconf(_SC_PAGESIZE); uint64_t max_rde_size = - MAX((physmem * MAX_RDT_PHYSMEM_PERCENT) / 100, + MAX((physbytes * MAX_RDT_PHYSMEM_PERCENT) / 100, SMALLEST_POSSIBLE_MAX_RDT_MB << 20); #endif diff --git a/cmd/ztest.c b/cmd/ztest.c index 54b4fdb4c..d94451906 100644 --- a/cmd/ztest.c +++ b/cmd/ztest.c @@ -140,9 +140,9 @@ #include #include #include +#include static int ztest_fd_data = -1; -static int ztest_fd_rand = -1; typedef struct ztest_shared_hdr { uint64_t zh_hdr_size; @@ -903,13 +903,10 @@ ztest_random(uint64_t range) { uint64_t r; - ASSERT3S(ztest_fd_rand, >=, 0); - if (range == 0) return (0); - if (read(ztest_fd_rand, &r, sizeof (r)) != sizeof (r)) - fatal(B_TRUE, "short read from /dev/urandom"); + random_get_pseudo_bytes((uint8_t *)&r, sizeof (r)); return (r % range); } @@ -8146,10 +8143,8 @@ ztest_raidz_expand_run(ztest_shared_t *zs, spa_t *spa) /* Setup a 1 MiB buffer of random data */ uint64_t bufsize = 1024 * 1024; void *buffer = umem_alloc(bufsize, UMEM_NOFAIL); + random_get_pseudo_bytes((uint8_t *)&buffer, bufsize); - if (read(ztest_fd_rand, buffer, bufsize) != bufsize) { - fatal(B_TRUE, "short read from /dev/urandom"); - } /* * Put some data in the pool and then attach a vdev to initiate * reflow. @@ -8955,13 +8950,7 @@ main(int argc, char **argv) exit(EXIT_FAILURE); } - /* - * Force random_get_bytes() to use /dev/urandom in order to prevent - * ztest from needlessly depleting the system entropy pool. - */ - random_path = "/dev/urandom"; - ztest_fd_rand = open(random_path, O_RDONLY | O_CLOEXEC); - ASSERT3S(ztest_fd_rand, >=, 0); + libspl_init(); if (!fd_data_str) { process_options(argc, argv); diff --git a/lib/libspl/include/sys/misc.h b/lib/libspl/include/sys/misc.h index 8f2f5f933..171bbc1de 100644 --- a/lib/libspl/include/sys/misc.h +++ b/lib/libspl/include/sys/misc.h @@ -31,9 +31,6 @@ #include -extern const char *random_path; -extern const char *urandom_path; - /* * Hostname information */ diff --git a/lib/libspl/include/sys/systm.h b/lib/libspl/include/sys/systm.h index 94fcfdd2a..f984125c3 100644 --- a/lib/libspl/include/sys/systm.h +++ b/lib/libspl/include/sys/systm.h @@ -29,6 +29,8 @@ #ifndef _LIBSPL_SYS_SYSTM_H #define _LIBSPL_SYS_SYSTM_H -extern uint64_t physmem; +uint64_t libspl_physmem(void); + +#define physmem libspl_physmem() #endif diff --git a/lib/libspl/include/sys/taskq.h b/lib/libspl/include/sys/taskq.h index 63238734b..fbe3f388c 100644 --- a/lib/libspl/include/sys/taskq.h +++ b/lib/libspl/include/sys/taskq.h @@ -86,8 +86,11 @@ typedef struct taskq { #define TASKQID_INVALID ((taskqid_t)0) -extern taskq_t *system_taskq; -extern taskq_t *system_delay_taskq; +extern taskq_t *_system_taskq(void); +extern taskq_t *_system_delay_taskq(void); + +#define system_taskq _system_taskq() +#define system_delay_taskq _system_delay_taskq() extern taskq_t *taskq_create(const char *, int, pri_t, int, int, uint_t); extern taskq_t *taskq_create_synced(const char *, int, pri_t, int, int, uint_t, diff --git a/lib/libspl/include/sys/thread.h b/lib/libspl/include/sys/thread.h index a5108a03d..6390c5bfd 100644 --- a/lib/libspl/include/sys/thread.h +++ b/lib/libspl/include/sys/thread.h @@ -58,11 +58,9 @@ typedef pthread_t kthread_t; #define current_is_reclaim_thread() (0) /* in libzpool, p0 exists only to have its address taken */ -typedef struct proc { - uintptr_t this_is_never_used_dont_dereference_it; -} proc_t; +typedef void (proc_t)(void); +extern void p0(void); -extern struct proc p0; #define curproc (&p0) #define PS_NONE -1 diff --git a/lib/libspl/libspl.c b/lib/libspl/libspl.c index 63e948bc2..208b3e428 100644 --- a/lib/libspl/libspl.c +++ b/lib/libspl/libspl.c @@ -31,11 +31,18 @@ #include #include #include +#include #include #include "libspl_impl.h" -uint64_t physmem; -struct utsname hw_utsname; +static uint64_t hw_physmem = 0; +static struct utsname hw_utsname = {}; + +uint64_t +libspl_physmem(void) +{ + return (hw_physmem); +} utsname_t * utsname(void) @@ -46,7 +53,7 @@ utsname(void) void libspl_init(void) { - physmem = sysconf(_SC_PHYS_PAGES); + hw_physmem = sysconf(_SC_PHYS_PAGES); VERIFY0(uname(&hw_utsname)); diff --git a/lib/libspl/random.c b/lib/libspl/random.c index 039273790..30281504f 100644 --- a/lib/libspl/random.c +++ b/lib/libspl/random.c @@ -32,15 +32,22 @@ #include #include "libspl_impl.h" -const char *random_path = "/dev/random"; -const char *urandom_path = "/dev/urandom"; +#define RANDOM_PATH "/dev/random" +#define URANDOM_PATH "/dev/urandom" + static int random_fd = -1, urandom_fd = -1; void random_init(void) { - VERIFY((random_fd = open(random_path, O_RDONLY | O_CLOEXEC)) != -1); - VERIFY((urandom_fd = open(urandom_path, O_RDONLY | O_CLOEXEC)) != -1); + /* Handle multiple calls. */ + if (random_fd != -1) { + ASSERT3U(urandom_fd, !=, -1); + return; + } + + VERIFY((random_fd = open(RANDOM_PATH, O_RDONLY | O_CLOEXEC)) != -1); + VERIFY((urandom_fd = open(URANDOM_PATH, O_RDONLY | O_CLOEXEC)) != -1); } void diff --git a/lib/libspl/taskq.c b/lib/libspl/taskq.c index 075d18893..043f70225 100644 --- a/lib/libspl/taskq.c +++ b/lib/libspl/taskq.c @@ -36,9 +36,20 @@ #include #include -int taskq_now; -taskq_t *system_taskq; -taskq_t *system_delay_taskq; +static taskq_t *__system_taskq = NULL; +static taskq_t *__system_delay_taskq = NULL; + +taskq_t +*_system_taskq(void) +{ + return (__system_taskq); +} + +taskq_t +*_system_delay_taskq(void) +{ + return (__system_delay_taskq); +} static pthread_key_t taskq_tsd; @@ -111,11 +122,6 @@ taskq_dispatch(taskq_t *tq, task_func_t func, void *arg, uint_t tqflags) { taskq_ent_t *t; - if (taskq_now) { - func(arg); - return (1); - } - mutex_enter(&tq->tq_lock); ASSERT(tq->tq_flags & TASKQ_ACTIVE); if ((t = task_alloc(tq, tqflags)) == NULL) { @@ -378,9 +384,6 @@ taskq_member(taskq_t *tq, kthread_t *t) { int i; - if (taskq_now) - return (1); - for (i = 0; i < tq->tq_nthreads; i++) if (tq->tq_threadlist[i] == t) return (1); @@ -405,18 +408,18 @@ void system_taskq_init(void) { VERIFY0(pthread_key_create(&taskq_tsd, NULL)); - system_taskq = taskq_create("system_taskq", 64, maxclsyspri, 4, 512, + __system_taskq = taskq_create("system_taskq", 64, maxclsyspri, 4, 512, TASKQ_DYNAMIC | TASKQ_PREPOPULATE); - system_delay_taskq = taskq_create("delay_taskq", 4, maxclsyspri, 4, + __system_delay_taskq = taskq_create("delay_taskq", 4, maxclsyspri, 4, 512, TASKQ_DYNAMIC | TASKQ_PREPOPULATE); } void system_taskq_fini(void) { - taskq_destroy(system_taskq); - system_taskq = NULL; /* defensive */ - taskq_destroy(system_delay_taskq); - system_delay_taskq = NULL; + taskq_destroy(__system_taskq); + __system_taskq = NULL; /* defensive */ + taskq_destroy(__system_delay_taskq); + __system_delay_taskq = NULL; VERIFY0(pthread_key_delete(taskq_tsd)); } diff --git a/lib/libspl/thread.c b/lib/libspl/thread.c index 17c36c754..f00e0a01a 100644 --- a/lib/libspl/thread.c +++ b/lib/libspl/thread.c @@ -32,7 +32,7 @@ #include /* this only exists to have its address taken */ -struct proc p0; +void p0(void) {} /* * =========================================================================