c8h4/postgresql
1
0
Fork 0
Code Issues Pull requests Actions Packages Projects Releases Wiki Activity

Prevent access of uninitialized memory in radix tree nodes

Browse source 
RT_NODE_16_SEARCH_EQ() performs comparisions using vector registers
on x64-64 and aarch64. We apply a mask to the resulting bitfield
to eliminate irrelevant bits that may be set. This ensures correct
behavior, but Valgrind complains of the partially-uninitialised
values. So far the warnings have only occurred on aarch64, which
explains why this hasn't been seen earlier.

To fix this warning, initialize the whole fixed-sized part of the nodes
upon allocation, rather than just do the minimum initialization to
function correctly. The initialization for node48 is a bit different
in that the 256-byte slot index array must be populated with "invalid
index" rather than zero. Experimentation has shown that compilers
tend to emit code that uselessly memsets that array twice. To avoid
pessimizing this path, swap the order of the slot_idxs[] and isset[]
arrays so we can initialize with two non-overlapping memset calls.

Reported by Tomas Vondra
Analysis and patch by Tom Lane, reviewed by Masahiko Sawada. I
investigated the behavior of memset calls to overlapping regions,
leading to the above tweaks to node48 as discussed in the thread.

Discussion: https://postgr.es/m/120c63ad-3d12-415f-a7bf-3da451c31bf6%40enterprisedb.com
This commit is contained in:
John Naylor 2024-06-21 14:59:11 +07:00
parent c5c82123d3
commit fd49e8f323
1 changed files with 14 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
src/include/lib/radixtree.h
View file

@ -541,15 +541,19 @@ typedef struct RT_NODE_48
{
RT_NODE base;
/* The index of slots for each fanout */
/* bitmap to track which slots are in use */
bitmapword isset[RT_BM_IDX(RT_FANOUT_48_MAX)];
/*
* Lookup table for indexes into the children[] array. We make this the
* last fixed-size member so that it's convenient to memset separately
* from the previous members.
*/
uint8 slot_idxs[RT_NODE_MAX_SLOTS];
/* Invalid index */
#define RT_INVALID_SLOT_IDX 0xFF
/* bitmap to track which slots are in use */
bitmapword isset[RT_BM_IDX(RT_FANOUT_48_MAX)];
/* number of children depends on size class */
RT_PTR_ALLOC children[FLEXIBLE_ARRAY_MEMBER];
} RT_NODE_48;
@ -845,27 +849,25 @@ RT_ALLOC_NODE(RT_RADIX_TREE * tree, const uint8 kind, const RT_SIZE_CLASS size_c
/* initialize contents */
memset(node, 0, sizeof(RT_NODE));
switch (kind)
{
case RT_NODE_KIND_4:
memset(node, 0, offsetof(RT_NODE_4, children));
break;
case RT_NODE_KIND_16:
memset(node, 0, offsetof(RT_NODE_16, children));
break;
case RT_NODE_KIND_48:
{
RT_NODE_48 *n48 = (RT_NODE_48 *) node;
memset(n48->isset, 0, sizeof(n48->isset));
memset(n48, 0, offsetof(RT_NODE_48, slot_idxs));
memset(n48->slot_idxs, RT_INVALID_SLOT_IDX, sizeof(n48->slot_idxs));
break;
}
case RT_NODE_KIND_256:
{
RT_NODE_256 *n256 = (RT_NODE_256 *) node;
memset(n256->isset, 0, sizeof(n256->isset));
break;
}
memset(node, 0, offsetof(RT_NODE_256, children));
break;
default:
pg_unreachable();
}