Changeset 30399 in vbox for trunk/src/VBox/Devices/Network/slirp/ip_icmp.c

Timestamp:

Jun 23, 2010 3:52:15 PM (15 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

63019

Message:

NAT: icmp_input releasing mbuf and intermediate buffer.

File:

• trunk/src/VBox/Devices/Network/slirp/ip_icmp.c (modified) (9 diffs)

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

@@ -330 +330 @@
 {
     register struct icmp *icp;
+    int fIcpOnMbuf = 1; /* if icp pointer to m->m_data */
     register struct ip *ip = mtod(m, struct ip *);
     int icmplen = ip->ip_len;
@@ -354 +355 @@
         /* min 8 bytes payload */
         icmpstat.icps_tooshort++;
-freeit:
-        m_freem(pData, m);
         goto end_error;
     }
@@ -361 +360 @@
     m->m_len -= hlen;
     m->m_data += hlen;
+
+    if (cksum(m, icmplen))
+    {
+        icmpstat.icps_checksum++;
+        goto end_error;
+    }
+
     if (m->m_next != NULL)
     {
         char *buf = RTMemAlloc(icmplen);
+        if (!buf)
+        {
+            LogRel(("NAT: not enought memory to allocate the buffer\n"));
+            goto end_error;
+        }
         m_copydata(m, 0, icmplen, buf);
         icp = (struct icmp *)buf;
+        fIcpOnMbuf = 0;
     }
     else
-    {
         icp = mtod(m, struct icmp *);
-    }
-    if (cksum(m, icmplen))
-    {
-        icmpstat.icps_checksum++;
-        goto freeit;
-    }
+
+
     m->m_len += hlen;
     m->m_data -= hlen;
@@ -394 +401 @@
                 ip->ip_src.s_addr = dst;
                 icmp_reflect(pData, m);
+                if (!fIcpOnMbuf)
+                    RTMemFree(icp);
+                return;
             }
             else
@@ -421 +431 @@
                 {
                     ssize_t rc;
-                    m->m_so = &pData->icmp_socket;
-                    icmp_attach(pData, m);
                     ttl = ip->ip_ttl;
                     Log(("NAT/ICMP: try to set TTL(%d)\n", ttl));
@@ -432 +440 @@
                     rc = sendto(pData->icmp_socket.s, icp, icmplen, 0,
                               (struct sockaddr *)&addr, sizeof(addr));
-                    if (rc < 0)
+                    if (rc >= 0)
                     {
-                        LogRel((dfd,"icmp_input udp sendto tx errno = %d-%s\n",
-                                    errno, strerror(errno)));
-                        icmp_error(pData, m, ICMP_UNREACH, ICMP_UNREACH_NET, 0, strerror(errno));
+                        icmp_attach(pData, m);
+                        m->m_so = &pData->icmp_socket;
+                        /* don't let m_freem at the end free atached buffer */
+                        return;
                     }
-                }
-                else
-                {
-                    /*
-                     * We're freeing the ICMP message, which unable sent or process.
-                     * That behavior described in rfc 793, we shouldn't notify sender about
-                     * fail of processing it's ICMP packets
-                     */
-                    m_freem(pData, m);
-                    return;
+                    
+                    LogRel((dfd,"icmp_input udp sendto tx errno = %d-%s\n",
+                                errno, strerror(errno)));
+                    icmp_error(pData, m, ICMP_UNREACH, ICMP_UNREACH_NET, 0, strerror(errno));
                 }
 #else /* RT_OS_WINDOWS */
-                icmp_attach(pData, m);
                 pData->icmp_socket.so_laddr.s_addr = ip->ip_src.s_addr; /* XXX: hack*/
                 pData->icmp_socket.so_icmp_id = icp->icmp_id;
                 pData->icmp_socket.so_icmp_seq = icp->icmp_seq;
-                m->m_so = &pData->icmp_socket;
                 memset(&ipopt, 0, sizeof(IP_OPTION_INFORMATION));
                 ipopt.Ttl = ip->ip_ttl;
                 status = ICMP_SEND_ECHO(pData->phEvents[VBOX_ICMP_EVENT_INDEX], notify_slirp, addr.sin_addr.s_addr,
                                 icp->icmp_data, icmplen - ICMP_MINLEN, &ipopt);
-                if (status == 0 && (error = GetLastError()) != ERROR_IO_PENDING)
+                error = GetLastError();
+                if (   status != 0 
+                    || error == ERROR_IO_PENDING)
                 {
-                    error = GetLastError();
-                    LogRel(("NAT: Error (%d) occurred while sending ICMP (", error));
-                    switch (error)
-                    {
-                        case ERROR_INVALID_PARAMETER:
-                            LogRel(("icmp_socket:%lx is invalid)\n", pData->icmp_socket.s));
-                            break;
-                        case ERROR_NOT_SUPPORTED:
-                            LogRel(("operation is unsupported)\n"));
-                            break;
-                        case ERROR_NOT_ENOUGH_MEMORY:
-                            LogRel(("OOM!!!)\n"));
-                            break;
-                        case IP_BUF_TOO_SMALL:
-                            LogRel(("Buffer too small)\n"));
-                            break;
-                        default:
-                            LogRel(("Other error!!!)\n"));
-                            break;
-                    }
+                    icmp_attach(pData, m);
+                    m->m_so = &pData->icmp_socket;
+                    /* don't let m_freem at the end free atached buffer */
+                    return;
+                }
+                LogRel(("NAT: Error (%d) occurred while sending ICMP (", error));
+                switch (error)
+                {
+                    case ERROR_INVALID_PARAMETER:
+                        LogRel(("icmp_socket:%lx is invalid)\n", pData->icmp_socket.s));
+                        break;
+                    case ERROR_NOT_SUPPORTED:
+                        LogRel(("operation is unsupported)\n"));
+                        break;
+                    case ERROR_NOT_ENOUGH_MEMORY:
+                        LogRel(("OOM!!!)\n"));
+                        break;
+                    case IP_BUF_TOO_SMALL:
+                        LogRel(("Buffer too small)\n"));
+                        break;
+                    default:
+                        LogRel(("Other error!!!)\n"));
+                        break;
                 }
 #endif /* RT_OS_WINDOWS */
@@ -486 +492 @@
             break;
         case ICMP_UNREACH:
-            /* XXX? report error? close socket? */
         case ICMP_TIMXCEED:
+            /* @todo(vvl): both up cases comes from guest, 
+             *  indeed right solution would be find the socket 
+             *  corresponding to ICMP data and close it.  
+             */
         case ICMP_PARAMPROB:
         case ICMP_SOURCEQUENCH:
@@ -494 +503 @@
         case ICMP_REDIRECT:
             icmpstat.icps_notsupp++;
-            m_freem(pData, m);
             break;
 
         default:
             icmpstat.icps_badtype++;
-            m_freem(pData, m);
     } /* switch */
-    if (m->m_next != NULL && icp != NULL)
+    
+end_error:
+    m_freem(pData, m);
+    if (   !fIcpOnMbuf
+        && !icp)
         RTMemFree(icp);
-
-end_error:
-    /* m is m_free()'d xor put in a socket xor or given to ip_send */
-    ;
 }
 
@@ -684 +691 @@
 /*
  * Reflect the ip packet back to the source
+ * Note: m isn't duplicated by this method and more delivered to ip_output then.
  */
 void