Skip to content

Commit

Permalink
libuutil: deobfuscate internal pointers
Browse files Browse the repository at this point in the history 
uu_avl and uu_list stored internal next/prev pointers and parent
pointers (unused) obfuscated (byte swapped) to hide them from a long
forgotten leak checker (No one at the 2022 OpenZFS developers meeting
could recall the history.)  This would break on CHERI systems and adds
no obvious value.  Rename the members, use proper types rather than
uintptr_t, and eliminate the related macros.

Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Brooks Davis <brooks.davis@sri.com>
Closes openzfs#14126
  • Loading branch information
@brooksdavis @andrewc12
brooksdavis authored and andrewc12 committed Nov 10, 2022
1 parent c8fa84c commit e955f10
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 57 deletions.
31 changes: 6 additions & 25 deletions include/libuutil_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,21 +46,6 @@ __attribute__((format(printf, 1, 2), __noreturn__))
void uu_panic(const char *format, ...);


/*
* For debugging purposes, libuutil keeps around linked lists of all uu_lists
* and uu_avls, along with pointers to their parents. These can cause false
* negatives when looking for memory leaks, so we encode the pointers by
* storing them with swapped endianness; this is not perfect, but it's about
* the best we can do without wasting a lot of space.
*/
#ifdef _LP64
#define UU_PTR_ENCODE(ptr) BSWAP_64((uintptr_t)(void *)(ptr))
#else
#define UU_PTR_ENCODE(ptr) BSWAP_32((uintptr_t)(void *)(ptr))
#endif

#define UU_PTR_DECODE(ptr) ((void *)UU_PTR_ENCODE(ptr))

/*
* uu_list structures
*/
Expand All @@ -80,11 +65,11 @@ struct uu_list_walk {
};

struct uu_list {
uintptr_t ul_next_enc;
uintptr_t ul_prev_enc;
uu_list_t *ul_next;
uu_list_t *ul_prev;

uu_list_pool_t *ul_pool;
uintptr_t ul_parent_enc; /* encoded parent pointer */
void *ul_parent;
size_t ul_offset;
size_t ul_numnodes;
uint8_t ul_debug;
Expand All @@ -95,8 +80,6 @@ struct uu_list {
uu_list_walk_t ul_null_walk; /* for robust walkers */
};

#define UU_LIST_PTR(ptr) ((uu_list_t *)UU_PTR_DECODE(ptr))

#define UU_LIST_POOL_MAXNAME 64

struct uu_list_pool {
Expand Down Expand Up @@ -129,20 +112,18 @@ struct uu_avl_walk {
};

struct uu_avl {
uintptr_t ua_next_enc;
uintptr_t ua_prev_enc;
uu_avl_t *ua_next;
uu_avl_t *ua_prev;

uu_avl_pool_t *ua_pool;
uintptr_t ua_parent_enc;
void *ua_parent;
uint8_t ua_debug;
uint8_t ua_index; /* mark for uu_avl_index_ts */

struct avl_tree ua_tree;
uu_avl_walk_t ua_null_walk;
};

#define UU_AVL_PTR(x) ((uu_avl_t *)UU_PTR_DECODE(x))

#define UU_AVL_POOL_MAXNAME 64

struct uu_avl_pool {
Expand Down
30 changes: 14 additions & 16 deletions lib/libuutil/uu_avl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ uu_avl_pool_create(const char *name, size_t objsize, size_t nodeoffset,

(void) pthread_mutex_init(&pp->uap_lock, NULL);

pp->uap_null_avl.ua_next_enc = UU_PTR_ENCODE(&pp->uap_null_avl);
pp->uap_null_avl.ua_prev_enc = UU_PTR_ENCODE(&pp->uap_null_avl);
pp->uap_null_avl.ua_next = &pp->uap_null_avl;
pp->uap_null_avl.ua_prev = &pp->uap_null_avl;

(void) pthread_mutex_lock(&uu_apool_list_lock);
pp->uap_next = next = &uu_null_apool;
Expand All @@ -114,10 +114,8 @@ void
uu_avl_pool_destroy(uu_avl_pool_t *pp)
{
if (pp->uap_debug) {
if (pp->uap_null_avl.ua_next_enc !=
UU_PTR_ENCODE(&pp->uap_null_avl) ||
pp->uap_null_avl.ua_prev_enc !=
UU_PTR_ENCODE(&pp->uap_null_avl)) {
if (pp->uap_null_avl.ua_next != &pp->uap_null_avl ||
pp->uap_null_avl.ua_prev != &pp->uap_null_avl) {
uu_panic("uu_avl_pool_destroy: Pool \"%.*s\" (%p) has "
"outstanding avls, or is corrupt.\n",
(int)sizeof (pp->uap_name), pp->uap_name,
Expand Down Expand Up @@ -224,7 +222,7 @@ uu_avl_create(uu_avl_pool_t *pp, void *parent, uint32_t flags)
}

ap->ua_pool = pp;
ap->ua_parent_enc = UU_PTR_ENCODE(parent);
ap->ua_parent = parent;
ap->ua_debug = pp->uap_debug || (flags & UU_AVL_DEBUG);
ap->ua_index = (pp->uap_last_index = INDEX_NEXT(pp->uap_last_index));

Expand All @@ -236,11 +234,11 @@ uu_avl_create(uu_avl_pool_t *pp, void *parent, uint32_t flags)

(void) pthread_mutex_lock(&pp->uap_lock);
next = &pp->uap_null_avl;
prev = UU_PTR_DECODE(next->ua_prev_enc);
ap->ua_next_enc = UU_PTR_ENCODE(next);
ap->ua_prev_enc = UU_PTR_ENCODE(prev);
next->ua_prev_enc = UU_PTR_ENCODE(ap);
prev->ua_next_enc = UU_PTR_ENCODE(ap);
prev = next->ua_prev;
ap->ua_next = next;
ap->ua_prev = prev;
next->ua_prev = ap;
prev->ua_next = ap;
(void) pthread_mutex_unlock(&pp->uap_lock);

return (ap);
Expand All @@ -263,11 +261,11 @@ uu_avl_destroy(uu_avl_t *ap)
}
}
(void) pthread_mutex_lock(&pp->uap_lock);
UU_AVL_PTR(ap->ua_next_enc)->ua_prev_enc = ap->ua_prev_enc;
UU_AVL_PTR(ap->ua_prev_enc)->ua_next_enc = ap->ua_next_enc;
ap->ua_next->ua_prev = ap->ua_prev;
ap->ua_prev->ua_next = ap->ua_next;
(void) pthread_mutex_unlock(&pp->uap_lock);
ap->ua_prev_enc = UU_PTR_ENCODE(NULL);
ap->ua_next_enc = UU_PTR_ENCODE(NULL);
ap->ua_prev = NULL;
ap->ua_next = NULL;

ap->ua_pool = NULL;
avl_destroy(&ap->ua_tree);
Expand Down
30 changes: 14 additions & 16 deletions lib/libuutil/uu_list.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ uu_list_pool_create(const char *name, size_t objsize,

(void) pthread_mutex_init(&pp->ulp_lock, NULL);

pp->ulp_null_list.ul_next_enc = UU_PTR_ENCODE(&pp->ulp_null_list);
pp->ulp_null_list.ul_prev_enc = UU_PTR_ENCODE(&pp->ulp_null_list);
pp->ulp_null_list.ul_next = &pp->ulp_null_list;
pp->ulp_null_list.ul_prev = &pp->ulp_null_list;

(void) pthread_mutex_lock(&uu_lpool_list_lock);
pp->ulp_next = next = &uu_null_lpool;
Expand All @@ -110,10 +110,8 @@ void
uu_list_pool_destroy(uu_list_pool_t *pp)
{
if (pp->ulp_debug) {
if (pp->ulp_null_list.ul_next_enc !=
UU_PTR_ENCODE(&pp->ulp_null_list) ||
pp->ulp_null_list.ul_prev_enc !=
UU_PTR_ENCODE(&pp->ulp_null_list)) {
if (pp->ulp_null_list.ul_next != &pp->ulp_null_list ||
pp->ulp_null_list.ul_prev != &pp->ulp_null_list) {
uu_panic("uu_list_pool_destroy: Pool \"%.*s\" (%p) has "
"outstanding lists, or is corrupt.\n",
(int)sizeof (pp->ulp_name), pp->ulp_name,
Expand Down Expand Up @@ -202,7 +200,7 @@ uu_list_create(uu_list_pool_t *pp, void *parent, uint32_t flags)
}

lp->ul_pool = pp;
lp->ul_parent_enc = UU_PTR_ENCODE(parent);
lp->ul_parent = parent;
lp->ul_offset = pp->ulp_nodeoffset;
lp->ul_debug = pp->ulp_debug || (flags & UU_LIST_DEBUG);
lp->ul_sorted = (flags & UU_LIST_SORTED);
Expand All @@ -217,11 +215,11 @@ uu_list_create(uu_list_pool_t *pp, void *parent, uint32_t flags)

(void) pthread_mutex_lock(&pp->ulp_lock);
next = &pp->ulp_null_list;
prev = UU_PTR_DECODE(next->ul_prev_enc);
lp->ul_next_enc = UU_PTR_ENCODE(next);
lp->ul_prev_enc = UU_PTR_ENCODE(prev);
next->ul_prev_enc = UU_PTR_ENCODE(lp);
prev->ul_next_enc = UU_PTR_ENCODE(lp);
prev = next->ul_prev;
lp->ul_next = next;
lp->ul_prev = prev;
next->ul_prev = lp;
prev->ul_next = lp;
(void) pthread_mutex_unlock(&pp->ulp_lock);

return (lp);
Expand Down Expand Up @@ -250,11 +248,11 @@ uu_list_destroy(uu_list_t *lp)
}

(void) pthread_mutex_lock(&pp->ulp_lock);
UU_LIST_PTR(lp->ul_next_enc)->ul_prev_enc = lp->ul_prev_enc;
UU_LIST_PTR(lp->ul_prev_enc)->ul_next_enc = lp->ul_next_enc;
lp->ul_next->ul_prev = lp->ul_prev;
lp->ul_prev->ul_next = lp->ul_next;
(void) pthread_mutex_unlock(&pp->ulp_lock);
lp->ul_prev_enc = UU_PTR_ENCODE(NULL);
lp->ul_next_enc = UU_PTR_ENCODE(NULL);
lp->ul_prev = NULL;
lp->ul_next = NULL;
lp->ul_pool = NULL;
uu_free(lp);
}
Expand Down

0 comments on commit e955f10

Please sign in to comment.