Page MenuHomeFreeBSD

kqueue timer: Remove detached knotes from the process stop queue
ClosedPublic

Authored by markj on May 13 2021, 8:36 PM.

Details

Summary

There are some scenarios where a timer event may be detached when it is
on the process' kqueue timer stop queue. If kqtimer_proc_continue() is
called after that point, it will iterate over the queue and access freed
timer structures.

Suppose a process has a periodic timer running. Here are a couple of
problematic scenarios:

  • The process is killed by the OOM killer, so timer events are placed on the process queue (because we check for P_KILLED()). The process closes its fds, so knotes are detached and elements on the queue are freed but not removed. Later during exit1() it is possible for a signal to be raised against the process (e.g., if a resource limit is exceeded), in which case kqtimer_proc_continue() is called.
  • fork_norfproc() suspends all other threads in the process, and in this case we will suspend timers too. Then it may free all fds in the process' table, but the process queue is not initialized.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markj requested review of this revision.May 13 2021, 8:36 PM
sys/kern/kern_event.c
868

Do you need to clear the flag there as well? I think it is safer to do, even if the timer cannot be reached by p_kqtim_stop iteration.

sys/kern/kern_event.c
868

kc is freed immediately after this, so a thread holding a reference to it would perform a use-after-free. Do you see some need for reference counting?

kib added inline comments.
sys/kern/kern_event.c
868

No, I misread the code. I do not think that so far kc's need refcounting.

This revision is now accepted and ready to land.May 13 2021, 11:07 PM

Handle double enqueues as well. It is possible for a timer event to be on the
suspend queue, and before it is removed, another running thread can force the
callout to be rescheduled with kevent().

This revision now requires review to proceed.May 13 2021, 11:54 PM
This revision is now accepted and ready to land.May 14 2021, 12:26 AM