kern/183792: Infinite loop in libalias

[Valery Ushakov]

>Number:         183792
>Category:       kern
>Synopsis:       Infinite loop in libalias
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Fri Nov 08 17:20:01 UTC 2013
>Closed-Date:
>Last-Modified:
>Originator:     Valery Ushakov
>Release:        N/A
>Organization:
>Environment:
>Description:
_attach_handler() in libalias/alias_mod.c looks like it was originally written with hand-rolled list code and later converted to BSD <sys/queue.h> macros incorrectly.  Wrong comment after LIST_FOREACH loop is a strong hint.  The fact that _detach_handler() uses a loop is another indication: LISTs are double-linked, so LIST_REMOVE can be done directly.

What was intended there in _attach_handler() is to append to the list, but b != NULL is unreachable since b is always NULL after the loop.  So the new element that should have been appended is prepended instead, breaking the ordering of handler_chain that the loop assumes.

Under certain order of calls this may lead to creating of infinite loop in the handler_chain.

Consider adding a handler with priority 1, then another one with priority 2, than the first handler again.  The list will be:

{ 1 }
{ 2, 1 } -- BUG: 2 is prepended instead of appended
{ 1, 2, 1, ... } - 1 is inserted before 2, creating infinite loop

The problem was originally reported by Yohanes Nugroho on vbox-dev mailing list:
https://www.virtualbox.org/pipermail/vbox-dev/2013-November/011936.html
though the suggested fix provided there is incorrect - it just hides the bug for the particular ordering of the calls involved in that scenario.

The proper fix is to change handler_chain to a queue so that appending to it is possible.  While there, _detach_handler() should drop the loop and just use remove operation directly since double-linked lists/queues support that.

>How-To-Repeat:

>Fix:
Change handler_chain to a queue.


>Release-Note:
>Audit-Trail:
>Unformatted: