Describe the bug
In VRANDMEMBER_RedisCommand (modules/vector-sets/vset.c), hnsw_random_node() can return NULL (when the graph is empty or the enter point is unset), and one call site correctly checks for this before use. Two other call sites in the same function, the allow_duplicates path and the use_dict path, dereference the return value without checking:
hnswNode *random_node = hnsw_random_node(vset->hnsw, 0);
struct vsetNodeVal *nv = random_node->value; // no NULL check
v/s the guarded call site elsewhere in the same function:
hnswNode *random_node = hnsw_random_node(vset->hnsw, 0);
if (random_node) {
...
}
To reproduce
Not currently reproducible as a crash. Under the present locking model, set_size == 0 is checked once at function entry and node_count can't change mid-command (single-threaded dispatch, all graph mutations happen on the main thread). This is a static inconsistency found by code inspection, not an observed failure.
Expected behavior
All call sites that use hnsw_random_node()'s return value should guard against NULL, consistent with the existing check earlier in the same function, so a future change to the locking/threading model doesn't silently reintroduce a NULL-dereference crash.
Additional information
Found while auditing modules/vector-sets/ at commit 3acc0c49cf5ad2af9425d333e62728342dd6159b
Describe the bug
In
VRANDMEMBER_RedisCommand(modules/vector-sets/vset.c),hnsw_random_node()can returnNULL(when the graph is empty or the enter point is unset), and one call site correctly checks for this before use. Two other call sites in the same function, theallow_duplicatespath and theuse_dictpath, dereference the return value without checking:v/s the guarded call site elsewhere in the same function:
To reproduce
Not currently reproducible as a crash. Under the present locking model,
set_size == 0is checked once at function entry andnode_countcan't change mid-command (single-threaded dispatch, all graph mutations happen on the main thread). This is a static inconsistency found by code inspection, not an observed failure.Expected behavior
All call sites that use
hnsw_random_node()'s return value should guard againstNULL, consistent with the existing check earlier in the same function, so a future change to the locking/threading model doesn't silently reintroduce a NULL-dereference crash.Additional information
Found while auditing
modules/vector-sets/at commit3acc0c49cf5ad2af9425d333e62728342dd6159b