[vbox-dev] [PATCH] libalias bug

[Yohanes Nugroho]

Hi,

I have found a bug in libalias, this is a small subtle bug, less than
should be greater than in the _attach_handler:

Index: src/VBox/Devices/Network/slirp/libalias/alias_mod.c
===================================================================
--- src/VBox/Devices/Network/slirp/libalias/alias_mod.c (revision 49383)
+++ src/VBox/Devices/Network/slirp/libalias/alias_mod.c (working copy)
@@ -165,7 +165,7 @@
             (b->dir == p->dir) &&
             (b->proto == p->proto))
             return (EEXIST); /* Priority conflict. */
-        if (b->pri > p->pri) {
+        if (b->pri < p->pri) {
             LIST_INSERT_BEFORE(b, p, entries);
             return (0);
         }

This bug doesn't usually appear, because in normal case handlers are
inserted in reverse order (inserted from smallest priority number to
the largest ) and it will always reach the code where  we do insert to
head.

I found this bug with a strange manifestation: on my Linux laptop,
when I turned off the WIFI, a Virtual Machine started with  NAT
networking will not get an IP address from DHCP, and the CPU usage of
the host is 100%.

After looking at the log and the source code, the high CPU load is
caused by infinite loop in the find_handler() in libalias
(alias_mod.c). The cause of the infinite loop is: a same handler was
inserted at the beginning and the end of the linked list because of
the wrong less than/greater than logic.

This problem will only happen if get_dns_addr_domain() (in
slirp_dns.c) returns -1. This happen only in some specific case (I
tried in Mac and Windows, and it never seems to return -1). In my
case, I use NetworkManager (default Debian Jessie installation) which
will overwrite the content of /etc/resolv.conf with the string
"#NetworkManager" when we are in offline mode. An empty or invalid
resolv.conf (or if the resolv.conf files only contains comments) will
cause this function to fail. Note: this is not distribution specific,
anyone that uses NetworkManager will have this problem (I started
investigating this with a report from Arch user).

When get_dns_addr_domain failed (returns -1),
slirpInitializeDnsSettings() will call dns_alias_load(), which will
attach the handler for dns packet (alias_dns.c). This same handler is
again inserted by the call  in slirp_init()  by calling in
dns_alias_load(), but this time there are already other handlers
inserted, and because of the wrong logic, this handler is again
inserted at the head (by this time the previously inserted handler
already moved to the end of the linked list). In the normal case, the
first handler is not inserted, so the bug doesn't appear.

My current workaround for the release version of VirtualBox is to set
a dummy "nameserver IP" line in resolv.conf.

Just in case it's also needed for tiny changes like this: I release it
under MIT license

-- 
Regards
Yohanes
http://yohan.es/