Changeset 53448 in vbox for trunk

Timestamp:

Dec 5, 2014 9:12:22 AM (10 years ago)

Author:

vboxsync

Message:

NAT: trac ticket 13630 - use generation numbers as poor man's weak
references to prevent dnsproxy from using stale request::dns_server.

This commit has some extra LogRel() for the user to confirm the fix,
which are to be changed to Log2.

XXX: TOO: Clean up in dnsproxy_query() needs more investigation.
Existing code just punts with an "XXX" comment on sendto() failure,
which doesn't doesn't look right.

Location:

trunk/src/VBox/Devices/Network/slirp

Files:

• dnsproxy/dnsproxy.c (modified) (5 diffs)
• dnsproxy/dnsproxy.h (modified) (1 diff)
• slirp_dns.c (modified) (1 diff)
• slirp_state.h (modified) (1 diff)

trunk/src/VBox/Devices/Network/slirp/dnsproxy/dnsproxy.c

@@ -121 +121 @@
     AssertPtrReturnVoid(arg);
 
-    if (   req->dns_server == NULL
+    if (   req->dnsgen != pData->dnsgen
+        || req->dns_server == NULL
         || (de = TAILQ_PREV(req->dns_server, dns_list_head, de_list)) == NULL)
     {
+        if (req->dnsgen != pData->dnsgen)
+        {
+            /* XXX: Log2 */
+            LogRel(("NAT: dnsproxy: timeout: req %p dnsgen %u != %u on %R[natsock]\n",
+                    req, req->dnsgen, pData->dnsgen, so));
+        }
         hash_remove_request(pData, req);
         RTMemFree(req);
@@ -356 +363 @@
         memcpy(&req->clientid, &buf[0], 2);
         req->dns_server = TAILQ_LAST(&pData->pDnsList, dns_list_head);
+        req->dnsgen = pData->dnsgen;
         if (req->dns_server == NULL)
         {
@@ -375 +383 @@
     else
     {
-         retransmit = 1;
+        if (req->dnsgen != pData->dnsgen)
+        {
+            /* XXX: Log2 */
+            LogRel(("NAT: dnsproxy: query: req %p dnsgen %u != %u on %R[natsock]\n",
+                    req, req->dnsgen, pData->dnsgen, so));
+            /*
+             * XXX: TODO: this probably requires more cleanup.
+             * Cf. XXX comment for sendto() failure below, but that
+             * error leg is probably untested since ~never taken.
+             */
+            ++dropped_queries;
+            return;
+        }
+        retransmit = 1;
     }
 
@@ -404 +425 @@
 
     /* send it to our authoritative server */
-    Log2(("NAT: request will be sent to %RTnaipv4 on %R[natsock]\n", addr.sin_addr, so));
+    Log2(("NAT: request will be %ssent to %RTnaipv4 on %R[natsock]\n",
+          retransmit ? "re" : "", addr.sin_addr, so));
 
     byte = sendto(so->s, buf, (unsigned int)byte, 0,
@@ -418 +440 @@
 
     so->so_state = SS_ISFCONNECTED; /* now it's selected */
-    Log2(("NAT: request was sent to %RTnaipv4 on %R[natsock]\n", addr.sin_addr, so));
+    Log2(("NAT: request was %ssent to %RTnaipv4 on %R[natsock]\n",
+          retransmit ? "re" : "", addr.sin_addr, so));
 
     ++authoritative_queries;

trunk/src/VBox/Devices/Network/slirp/dnsproxy/dnsproxy.h

@@ -82 +82 @@
      * it's value on next server. And dnsproxy_query should
      * initializate with first server in the list
+     *
+     * dnsgen is a generation number - a copy of pData->dnsgen at the
+     * time of request creation (poor man's weak reference).
+     * dns_server must not be used if pData->dnsgen changed.
      */
     struct dns_entry    *dns_server;
+    uint32_t            dnsgen; 
     int nbyte; /* length of dns request */
     char byte[1]; /* copy of original request */

trunk/src/VBox/Devices/Network/slirp/slirp_dns.c

@@ -304 +304 @@
         RTMemFree(pDomain);
     }
+
+    /* tell any pending dnsproxy requests their copy is expired */
+    ++pData->dnsgen;
+
     LogFlowFuncLeaveRC(rc);
     return rc;

trunk/src/VBox/Devices/Network/slirp/slirp_state.h

@@ -151 +151 @@
     struct dns_list_head pDnsList;
     struct dns_domain_list_head pDomainList;
+    uint32_t dnsgen;            /* XXX: merge with dnsLastUpdate? */
     struct in_addr tftp_server;
     struct in_addr loopback_addr;