Skip to content

[BUG] NULL dereference in usUntilEarliestTimer when all time events are pending deletion #4349

Description

@royenheart

Description

usUntilEarliestTimer() (src/ae.c) scans the time-event list for the earliest
timer that is not pending deletion. When every remaining time event is a
lazy-deleted zombie (id == AE_DELETED_EVENT_ID), the scan finds nothing,
earliest stays NULL, and the function dereferences it:

    aeTimeEvent *earliest = NULL;
    while (te) {
        if ((!earliest || te->when < earliest->when) && te->id != AE_DELETED_EVENT_ID) earliest = te;
        te = te->next;
    }

    monotime now = getMonotonicUs();
    return (now >= earliest->when) ? 0 : earliest->when - now;   /* earliest == NULL */

The empty-list case is guarded (if (te == NULL) return -1;), the
all-zombies case is not.

The function is called on every event-loop iteration from aeProcessEvents()
(on the !AE_DONT_WAIT path, to compute the poll timeout). The zombie-only
state is reachable whenever the last timer of an event loop fires AE_NOMORE,
or is deleted between two processTimeEvents runs — the zombie is only reaped
on the next processTimeEvents pass, while the lookup happens before it.
On the main loop serverCron makes this unlikely, but ae is a general-purpose
loop also used with custom/per-module loops and by downstream code, where a
loop can genuinely have zero permanent timers. Redis fixed the identical bug
in redis/redis#15391 (June 2026); valkey does not have the guard.

Reproduction

Verified on current unstable (28aa048). The driver below creates one timer,
deletes it, then drives aeProcessEvents() on the lookup path:

  • zombie_null_repro.c
#include "ae.h"
#include <unistd.h>
#include <stdio.h>

static long long timerProc(struct aeEventLoop *e, long long id, void *c) {
    (void)e; (void)id; (void)c;
    return AE_NOMORE;
}
static void fileProc(struct aeEventLoop *e, int fd, void *c, int m) {
    (void)e; (void)fd; (void)c; (void)m;
}

int main(void) {
    aeEventLoop *loop = aeCreateEventLoop(64);
    int fds[2];
    pipe(fds);
    write(fds[1], "x", 1);   /* ready file event: aeApiPoll returns immediately */
    aeCreateFileEvent(loop, fds[0], AE_READABLE, fileProc, NULL);
    long long id = aeCreateTimeEvent(loop, 60000, timerProc, NULL, NULL);
    aeDeleteTimeEvent(loop, id);          /* zombie-only list */
    aeProcessEvents(loop, AE_ALL_EVENTS); /* !AE_DONT_WAIT -> lookup -> crash */
    printf("no crash (fixed)\n");
    return 0;
}
  • Build & run (against a built unstable tree):
gcc -O2 -g -std=gnu11 -I$TREE/src -I$TREE/deps/jemalloc/include \
    -o zombie_null_repro zombie_null_repro.c \
    $TREE/src/ae.o $TREE/src/zmalloc.o $TREE/src/monotonic.o $TREE/src/anet.o \
    $TREE/src/serverassert.o $TREE/deps/jemalloc/lib/libjemalloc.a -ldl -lpthread -lm
./zombie_null_repro
  • Observed result on unpatched unstable:
driving aeProcessEvents (lookup on zombie-only list)...
Segmentation fault (core dumped)         # exit code 139

# gdb backtrace:
#0  usUntilEarliestTimer (eventLoop=0x...) at src/ae.c:316
#1  aeProcessEvents (eventLoop=..., flags=3) at src/ae.c:440
#2  main () at zombie_null_repro.c:49
  • Full repro.sh
#!/usr/bin/env bash
# repro.sh - reproduce the usUntilEarliestTimer NULL dereference and verify
# the fix. Usage: repro.sh <valkey-tree> [patch-file]
#   <valkey-tree>  a built valkey source tree (src/ae.o etc. present)
#   [patch-file]   optional 0001-fix patch; when given, a patched copy of
#                  ae.o is compiled and the PoC is re-run against it.
set -euo pipefail
TREE=$(realpath "$1")
PATCH=${2:-}
HERE=$(cd "$(dirname "$0")" && pwd)
WORK=$(mktemp -d)
trap 'rm -rf "$WORK"' EXIT

build_repro() { # <ae.o dir> <out>
    local T=$1 out=$2
    gcc -O2 -g -std=gnu11 -I"$T/src" -I"$T/deps/jemalloc/include" \
        -o "$out" "$HERE/zombie_null_repro.c" \
        "$T/src/ae.o" "$T/src/zmalloc.o" "$T/src/monotonic.o" "$T/src/anet.o" \
        "$T/src/serverassert.o" "$T/deps/jemalloc/lib/libjemalloc.a" \
        -ldl -lpthread -lm
}

echo "=== 1) unpatched tree: expect SIGSEGV ==="
build_repro "$TREE" "$WORK/repro-base"
set +e
"$WORK/repro-base"
rc=$?
set -e
echo "exit=$rc"
if [[ $rc -ne 139 && $rc -ne 134 && $rc -ne 136 ]]; then
    echo "UNEXPECTED: unpatched build did not crash (rc=$rc)" >&2
fi

echo "=== 1b) backtrace of the crash ==="
gdb -batch -ex run -ex bt --args "$WORK/repro-base" 2>/dev/null | grep -E "^#|SIGSEGV" | head -8 || true

if [[ -n "$PATCH" ]]; then
    echo "=== 2) patched tree: expect clean exit ==="
    cp -a "$TREE" "$WORK/tree-patch"
    patch -p1 --no-backup-if-mismatch -s -d "$WORK/tree-patch" < "$PATCH"
    make -C "$WORK/tree-patch/src" ae.o > /dev/null
    build_repro "$WORK/tree-patch" "$WORK/repro-patch"
    "$WORK/repro-patch"
    echo "exit=$? (0 = fixed)"
fi

Proposed fix

Return -1 when the rescan finds no valid timer:

+    /* All events are pending deletion (zombies only): no valid timer. */
+    if (earliest == NULL) return -1;

-1 is the existing "no timer" sentinel (same as the empty-list branch), so
the caller treats it as infinite wait — the zombies are reaped by the
immediately following processTimeEvents() in the same aeProcessEvents()
call, and the next iteration computes a real timeout again. PR with this fix
plus a src/unit/test_ae.cpp regression test follows.

After fix, the repro will observe the result:

driving aeProcessEvents (lookup on zombie-only list)...
OK: no crash (usUntilEarliestTimer returned -1)

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions