From 0a34672fd384e2c7c09aeaf1c08e7ad09e7b90a5 Mon Sep 17 00:00:00 2001 From: cheloha Date: Fri, 23 Feb 2024 16:51:39 +0000 Subject: [PATCH] timeout: make to_kclock validation more rigorous In kern_timeout.c, the to_kclock checks are not strict enough to catch all plausible programmer mistakes. Tighten them up: - timeout_set_flags: KASSERT that kclock is valid - timeout_abs_ts: KASSERT that to_kclock is KCLOCK_UPTIME We can also add to_kclock validation to softclock() and db_show_timeout(), which may help to debug memory corruption: - softclock: panic if to_kclock is not KCLOCK_NONE or KCLOCK_UPTIME - db_show_timeout: print warning if to_kclock is invalid Prompted by bluhm@ in response to a syzbot panic. Hopefully these changes help to narrow down the root cause. Link: https://syzkaller.appspot.com/bug?extid=49d3f7118413963f651a Reported-by: syzbot+49d3f7118413963f651a@syzkaller.appspotmail.com ok bluhm@ --- sys/kern/kern_timeout.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/sys/kern/kern_timeout.c b/sys/kern/kern_timeout.c index d0b4840fdf4..cdc30cb20c5 100644 --- a/sys/kern/kern_timeout.c +++ b/sys/kern/kern_timeout.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_timeout.c,v 1.96 2023/10/12 15:32:38 cheloha Exp $ */ +/* $OpenBSD: kern_timeout.c,v 1.97 2024/02/23 16:51:39 cheloha Exp $ */ /* * Copyright (c) 2001 Thomas Nordin * Copyright (c) 2000-2001 Artur Grabowski @@ -271,6 +271,7 @@ timeout_set_flags(struct timeout *to, void (*fn)(void *), void *arg, int kclock, int flags) { KASSERT(!ISSET(flags, ~(TIMEOUT_PROC | TIMEOUT_MPSAFE))); + KASSERT(kclock >= KCLOCK_NONE && kclock < KCLOCK_MAX); to->to_func = fn; to->to_arg = arg; @@ -404,7 +405,7 @@ timeout_abs_ts(struct timeout *to, const struct timespec *abstime) mtx_enter(&timeout_mutex); KASSERT(ISSET(to->to_flags, TIMEOUT_INITIALIZED)); - KASSERT(to->to_kclock != KCLOCK_NONE); + KASSERT(to->to_kclock == KCLOCK_UPTIME); old_abstime = to->to_abstime; to->to_abstime = *abstime; @@ -750,10 +751,14 @@ softclock(void *arg) CIRCQ_REMOVE(&to->to_list); if (to == first_new) new = 1; - if (to->to_kclock != KCLOCK_NONE) - softclock_process_kclock_timeout(to, new); - else + if (to->to_kclock == KCLOCK_NONE) softclock_process_tick_timeout(to, new); + else if (to->to_kclock == KCLOCK_UPTIME) + softclock_process_kclock_timeout(to, new); + else { + panic("%s: invalid to_clock: %d", + __func__, to->to_kclock); + } } tostat.tos_softclocks++; needsproc = !CIRCQ_EMPTY(&timeout_proc); @@ -951,26 +956,34 @@ db_show_timeout(struct timeout *to, struct circq *bucket) where = "thread-mp"; #endif else { - if (to->to_kclock != KCLOCK_NONE) + if (to->to_kclock == KCLOCK_UPTIME) wheel = timeout_wheel_kc; - else + else if (to->to_kclock == KCLOCK_NONE) wheel = timeout_wheel; + else + goto invalid; snprintf(buf, sizeof(buf), "%3ld/%1ld", (bucket - wheel) % WHEELSIZE, (bucket - wheel) / WHEELSIZE); where = buf; } - if (to->to_kclock != KCLOCK_NONE) { + if (to->to_kclock == KCLOCK_UPTIME) { kc = &timeout_kclock[to->to_kclock]; timespecsub(&to->to_abstime, &kc->kc_lastscan, &remaining); db_printf("%20s %8s %9s 0x%0*lx %s\n", db_timespec(&remaining), db_kclock(to->to_kclock), where, width, (ulong)to->to_arg, name); - } else { + } else if (to->to_kclock == KCLOCK_NONE) { db_printf("%20d %8s %9s 0x%0*lx %s\n", to->to_time - ticks, "ticks", where, width, (ulong)to->to_arg, name); - } + } else + goto invalid; + return; + + invalid: + db_printf("%s: timeout 0x%p: invalid to_kclock: %d", + __func__, to, to->to_kclock); } void -- 2.20.1