Date: Fri, 8 Nov 2013 17:13:28 GMT From: Valery Ushakov <uwe@NetBSD.org> To: freebsd-gnats-submit@FreeBSD.org Subject: kern/183792: Infinite loop in libalias Message-ID: <201311081713.rA8HDSuO055522@oldred.freebsd.org> Resent-Message-ID: <201311081720.rA8HK2Wn019762@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201311081713.rA8HDSuO055522>