[Bug 293382] Dead lock and kernel crash around closefp_impl

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=293382

--- Comment #51 from Kyle Evans <kevans@freebsd.org> ---
(In reply to Paul from comment #49)

Just thinking out loud for a minute, we know these facts:

 1.) In slot 211098 of this kq we found this knote that thinks it is for fd
76954 and has an fp that matches fd 76954 in the fd table
 2.) The kq_knlist only ever grows, never shrinks, and in exactly one place:
kqueue_expand
 3.) kq_knlist / kn_link usage is actually pretty minimal and reasonably easy
to audit
 4.) knote_attach is the only place adding anything to kq_knlist

knote_attach() is incredibly straightforward and clearly under the kq lock:

2880         if (kn->kn_fop->f_isfd) {                                          
2881                 if (kn->kn_id >= kq->kq_knlistsize)                        
2882                         return (ENOMEM);                                   
2883                 list = &kq->kq_knlist[kn->kn_id];                          
2884         } else {                                                           
2885                 if (kq->kq_knhash == NULL)                                 
2886                         return (ENOMEM);                                   
2887                 list = &kq->kq_knhash[KN_HASH(kn->kn_id,
kq->kq_knhashmask)];                            
2888         }                                                                  
2889         SLIST_INSERT_HEAD(list, kn, kn_link);                              

That's clearly checking the size correctly beforre it inserts the knote.  The
only removal is in knote_drop_detached():

2925         if (kn->kn_fop->f_isfd)                                            
2926                 list = &kq->kq_knlist[kn->kn_id];                          
2927         else                                                               
2928                 list = &kq->kq_knhash[KN_HASH(kn->kn_id,
kq->kq_knhashmask)];                                                            
2929                                                                            
2930         if (!SLIST_EMPTY(list))                                            
2931                 SLIST_REMOVE(list, kn, knote, kn_link);                    

I think we all agree that !SLIST_EMPTY(list) should actually be an assertion at
best, there's no path before the call to knote_attach() in kqueue_register()
that would call knote_drop*() and you can't double-drop a knote.

I really don't see many plausible causes here, outside of kqueue_expand going
wrong somehow, but the disassembly seems sane (and not obviously eliding
important things) and the bzero calculations, while kind of convoluted, seem
fine.  I probably would've written it as bzero(&list[kq->kq_knlistsize], (size
- kq->kq_knlistsize) * sizeof(*list)) personally, but I don't see a world in
which those aren't equivalent.

-- 
You are receiving this mail because:
You are the assignee for the bug.