Bug in kqueue_scan() while doing KN_LIST_LOCK/UNLOCK and a proposed fix

[siddharthtuli_gmail.com]

Summary

I have a process (procX) that has registered interest in NOTE_EXIT|NOTE_EXEC|NOTE_TRACK|NOTE_TRACKERR for all the non-system processes and it keeps doing kevent() syscall in its lifetime to retrieve the events.

KN_LIST_LOCK in context of a process doing kevent() is essentially p->p_mtx of the process whose events are to be delivered to process doing kevent().

222 #define KN_LIST_LOCK(kn) do { \
223 if (kn->kn_knlist != NULL) \
224 kn->kn_knlist->kl_lock(kn->kn_knlist->kl_lockarg); \

226 #define KN_LIST_UNLOCK(kn) do { \
227 if (kn->kn_knlist != NULL) \
228 kn->kn_knlist->kl_unlock(kn->kn_knlist->kl_lockarg); \
229 } while (0)

• Since the lock is guarded by condition (kn->kn_knlist != NULL) it is possible that during KN_LIST_LOCK the condition was true and hence the lock was held. However, during KN_LIST_UNLOCK the condition is no longer true and unlock will never happen.

> The following race can thus occur resulting in a crash,

1. ProcX does kern_kevent() -> kqueue_scan() -> KN_LIST_LOCK.
2. kn->kn_knlist != NULL is TRUE and knote "kn" is that of a process say ProcA. The lock could not be held and ProcX goes to sleep
3. ProcA is exiting AND it has acquired its PROC_LOCK. exit1() -> KNOTE_LOCKED -> knlist_remove_kq() resulted in kn->kn_knlist = NULL.
4. ProcX wakes up. When it calls KN_LIST_UNLOCK, kn->kn_knlist is now NULL and unlock never happens.

> Below is the timing sequence,

1         cpuA (procA)                                        cpuX (ProcX)
2
3       exit1()                                           kqueue_scan()
4         ......
5         set P_WEXIT, add proc to zombproc list
6         Lock p->p_mtx                                      ....      
7         KNOTE_LOCKED (send NOTE_EXIT)                      KN_LIST_LOCK (kn->kn_knlist ! = NULL. PROC_LOCK already acquired in 6. Context switch)
8         knlist_remove_kq(set kn->kn_knlist = NULL)                
9         .........

10 knlist_clear
11 ........
12 knlist_destroy
13 thread_exit
14 PROC_UNLOCK
15 Acquire the KN_LIST_LOCK
16 ......
17 KN_LIST_UNLOCK has kn->knlist == NULL (from 8). But while doing KN_LIST_LOCK it was not NULL. Hence the unlock will never happen and the box will panic because the lock is permanently held by ProcX due to lost unlock.

Fix:

>If knote is in kqueue_scan() and process is exiting, before setting kn_knlist to null we should allow kqueue_scan() to process the knote so that KN_LIST_LOCK and UNLOCK can succeed before kn_knlist is NULL.

Test Plan

Unit testing

[jtl]

I plan to commit this in about 48 hours if no feedback is received.

[jtl]

It looks like this is a general problem: Anytime knlist_remove_kq() is called while another function is trying to acquire the lock, you could have a problem.

As discussed in email, please update the patch to handle the general problem.

Thanks!