mbuf: Add debugging helpers for allocation

This adds a few helpers for debugging mbuf allocations when running in
debugging mode (lsan, valgrind, etc.)

- We do not want to cache allocations, so always set M_DOFREE to prevent
  us from putting any mbuf in it.
- We want to update the mbuf allocation owner on function call for more
  precise leak reporting.

Based on Jeremy Marchand's fuzzing work.

Signed-off-by: jeremy marchand <jeremy.marchand@etu.u-bordeaux.fr>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

12 files changed, 125 insertions, 12 deletions

diff --git a/src/if.c b/src/if.c
index 83dcae3..9a1eec9 100644
--- a/src/if.c
+++ b/src/if.c
@@ -41,6 +41,8 @@ void if_init(Slirp *slirp)
 void if_output(struct socket *so, struct mbuf *ifm)
 {
     Slirp *slirp = ifm->slirp;
+    M_DUP_DEBUG(slirp, ifm, 0, 0);
+
     struct mbuf *ifq;
     int on_fastq = 1;
 
diff --git a/src/ip6_icmp.c b/src/ip6_icmp.c
index 21b1407..738b40f 100644
--- a/src/ip6_icmp.c
+++ b/src/ip6_icmp.c
@@ -385,12 +385,12 @@ static void ndp_input(struct mbuf *m, Slirp *slirp, struct ip6 *ip,
  */
 void icmp6_input(struct mbuf *m)
 {
+    Slirp *slirp = m->slirp;
     /* NDP reads the ethernet header for gratuitous NDP */
-    g_assert(M_ROOMBEFORE(m) >= ETH_HLEN);
+    M_DUP_DEBUG(slirp, m, 1, ETH_HLEN);
 
     struct icmp6 *icmp;
     struct ip6 *ip = mtod(m, struct ip6 *);
-    Slirp *slirp = m->slirp;
     int hlen = sizeof(struct ip6);
 
     DEBUG_CALL("icmp6_input");
diff --git a/src/ip6_input.c b/src/ip6_input.c
index 10c42d6..b3d9865 100644
--- a/src/ip6_input.c
+++ b/src/ip6_input.c
@@ -23,10 +23,11 @@ void ip6_cleanup(Slirp *slirp)
 
 void ip6_input(struct mbuf *m)
 {
-    struct ip6 *ip6;
     Slirp *slirp = m->slirp;
     /* NDP reads the ethernet header for gratuitous NDP */
-    g_assert(M_ROOMBEFORE(m) >= TCPIPHDR_DELTA + 2 + ETH_HLEN);
+    M_DUP_DEBUG(slirp, m, 1, TCPIPHDR_DELTA + 2 + ETH_HLEN);
+
+    struct ip6 *ip6;
 
     if (!slirp->in6_enabled) {
         goto bad;
diff --git a/src/ip6_output.c b/src/ip6_output.c
index 2f62cc9..834f1c0 100644
--- a/src/ip6_output.c
+++ b/src/ip6_output.c
@@ -15,6 +15,9 @@
  */
 int ip6_output(struct socket *so, struct mbuf *m, int fast)
 {
+    Slirp *slirp = m->slirp;
+    M_DUP_DEBUG(slirp, m, 0, 0);
+
     struct ip6 *ip = mtod(m, struct ip6 *);
 
     DEBUG_CALL("ip6_output");
diff --git a/src/ip_icmp.c b/src/ip_icmp.c
index 7d8b60f..5ffadd6 100644
--- a/src/ip_icmp.c
+++ b/src/ip_icmp.c
@@ -85,6 +85,9 @@ void icmp_cleanup(Slirp *slirp)
 
 static int icmp_send(struct socket *so, struct mbuf *m, int hlen)
 {
+    Slirp *slirp = m->slirp;
+    M_DUP_DEBUG(slirp, m, 0, 0);
+
     struct ip *ip = mtod(m, struct ip *);
     struct sockaddr_in addr;
 
@@ -136,10 +139,12 @@ void icmp_detach(struct socket *so)
  */
 void icmp_input(struct mbuf *m, int hlen)
 {
+    Slirp *slirp = m->slirp;
+    M_DUP_DEBUG(slirp, m, 0, 0);
+
     register struct icmp *icp;
     register struct ip *ip = mtod(m, struct ip *);
     int icmplen = ip->ip_len;
-    Slirp *slirp = m->slirp;
 
     DEBUG_CALL("icmp_input");
     DEBUG_ARG("m = %p", m);
diff --git a/src/ip_input.c b/src/ip_input.c
index 7ccec8d..a29c324 100644
--- a/src/ip_input.c
+++ b/src/ip_input.c
@@ -70,7 +70,7 @@ void ip_cleanup(Slirp *slirp)
 void ip_input(struct mbuf *m)
 {
     Slirp *slirp = m->slirp;
-    g_assert(M_ROOMBEFORE(m) >= TCPIPHDR_DELTA);
+    M_DUP_DEBUG(slirp, m, 0, TCPIPHDR_DELTA);
 
     register struct ip *ip;
     int hlen;
diff --git a/src/ip_output.c b/src/ip_output.c
index 22916a3..4f62605 100644
--- a/src/ip_output.c
+++ b/src/ip_output.c
@@ -51,6 +51,8 @@
 int ip_output(struct socket *so, struct mbuf *m0)
 {
     Slirp *slirp = m0->slirp;
+    M_DUP_DEBUG(slirp, m0, 0, 0);
+
     register struct ip *ip;
     register struct mbuf *m = m0;
     register int hlen = sizeof(struct ip);
diff --git a/src/mbuf.c b/src/mbuf.c
index 24aad00..b47f64e 100644
--- a/src/mbuf.c
+++ b/src/mbuf.c
@@ -69,10 +69,10 @@ struct mbuf *m_get(Slirp *slirp)
 
     DEBUG_CALL("m_get");
 
-    if (slirp->m_freelist.qh_link == &slirp->m_freelist) {
+    if (MBUF_DEBUG || slirp->m_freelist.qh_link == &slirp->m_freelist) {
         m = g_malloc(SLIRP_MSIZE(slirp->if_mtu));
         slirp->mbuf_alloced++;
-        if (slirp->mbuf_alloced > MBUF_THRESH)
+        if (MBUF_DEBUG || slirp->mbuf_alloced > MBUF_THRESH)
             flags = M_DOFREE;
         m->slirp = slirp;
     } else {
@@ -227,3 +227,39 @@ struct mbuf *dtom(Slirp *slirp, void *dat)
 
     return (struct mbuf *)0;
 }
+
+/*
+ * Duplicate the mbuf
+ *
+ * copy_header specifies whether the bytes before m_data should also be copied.
+ * header_size specifies how many bytes are to be reserved before m_data.
+ */
+struct mbuf *m_dup(Slirp *slirp, struct mbuf *m,
+                   bool copy_header,
+                   size_t header_size)
+{
+    struct mbuf *n;
+    int mcopy_result;
+
+    /* The previous mbuf was supposed to have it already, we can check it along
+     * the way */
+    assert(M_ROOMBEFORE(m) >= header_size);
+
+    n = m_get(slirp);
+    m_inc(n, m->m_len + header_size);
+
+    if (copy_header) {
+        m->m_len += header_size;
+        m->m_data -= header_size;
+        mcopy_result = m_copy(n, m, 0, m->m_len + header_size);
+        n->m_data += header_size;
+        m->m_len -= header_size;
+        m->m_data += header_size;
+    } else {
+        n->m_data += header_size;
+        mcopy_result = m_copy(n, m, 0, m->m_len);
+    }
+    g_assert(mcopy_result == 0);
+
+    return n;
+}
diff --git a/src/mbuf.h b/src/mbuf.h
index 698c89b..831277d 100644
--- a/src/mbuf.h
+++ b/src/mbuf.h
@@ -124,6 +124,7 @@ void m_cat(register struct mbuf *, register struct mbuf *);
 void m_inc(struct mbuf *, int);
 void m_adj(struct mbuf *, int);
 int m_copy(struct mbuf *, struct mbuf *, int, int);
+struct mbuf *m_dup(Slirp *slirp, struct mbuf *m, bool copy_header, size_t header_size);
 struct mbuf *dtom(Slirp *, void *);
 
 static inline void ifs_init(struct mbuf *ifm)
@@ -131,4 +132,58 @@ static inline void ifs_init(struct mbuf *ifm)
     ifm->ifs_next = ifm->ifs_prev = ifm;
 }
 
+#ifdef DEBUG
+#  define MBUF_DEBUG 1
+#else
+#  ifdef HAVE_VALGRIND
+#    include <valgrind/valgrind.h>
+#    define MBUF_DEBUG RUNNING_ON_VALGRIND
+#  else
+#    define MBUF_DEBUG 0
+#  endif
+#endif
+
+/*
+ * When a function is given an mbuf as well as the responsibility to free it, we
+ * want valgrind etc. to properly identify the new responsible for the
+ * free. Achieve this by making a new copy. For instance:
+ *
+ * f0(void) {
+ *   struct mbuf *m = m_get(slirp);
+ *   [...]
+ *   switch (something) {
+ *   case 1:
+ *     f1(m);
+ *     break;
+ *   case 2:
+ *     f2(m);
+ *     break;
+ *   [...]
+ *   }
+ * }
+ *
+ * f1(struct mbuf *m) {
+ *   M_DUP_DEBUG(m->slirp, m);
+ *   [...]
+ *   m_free(m); // but author of f1 might be forgetting this
+ * }
+ *
+ * f0 transfers the freeing responsibility to f1, f2, etc.  Without the
+ * M_DUP_DEBUG call in f1, valgrind would tell us that it is f0 where the buffer
+ * was allocated, but it's difficult to know whether a leak is actually in f0,
+ * or in f1, or in f2, etc.  Duplicating the mbuf in M_DUP_DEBUG each time the
+ * responsibility is transferred allows to immediately know where the leak
+ * actually is.
+ */
+#define M_DUP_DEBUG(slirp, m, copy_header, header_size) do { \
+    if (MBUF_DEBUG) { \
+        struct mbuf *__n; \
+        __n = m_dup((slirp), (m), (copy_header), (header_size)); \
+        m_free(m); \
+        (m) = __n; \
+    } else { \
+        g_assert(M_ROOMBEFORE(m) >= (header_size)); \
+    } \
+} while(0)
+
 #endif
diff --git a/src/tcp_input.c b/src/tcp_input.c
index e6f5722..36a4844 100644
--- a/src/tcp_input.c
+++ b/src/tcp_input.c
@@ -82,6 +82,9 @@ static void tcp_xmit_timer(register struct tcpcb *tp, int rtt);
 static int tcp_reass(register struct tcpcb *tp, register struct tcpiphdr *ti,
                      struct mbuf *m)
 {
+    if (m)
+        M_DUP_DEBUG(m->slirp, m, 0, 0);
+
     register struct tcpiphdr *q;
     struct socket *so = tp->t_socket;
     int flags;
@@ -235,11 +238,11 @@ void tcp_input(struct mbuf *m, int iphlen, struct socket *inso,
     slirp = m->slirp;
     switch (af) {
     case AF_INET:
-        g_assert(M_ROOMBEFORE(m) >=
+        M_DUP_DEBUG(slirp, m, 0,
             sizeof(struct tcpiphdr) - sizeof(struct ip) - sizeof(struct tcphdr));
         break;
     case AF_INET6:
-        g_assert(M_ROOMBEFORE(m) >=
+        M_DUP_DEBUG(slirp, m, 0,
             sizeof(struct tcpiphdr) - sizeof(struct ip6) - sizeof(struct tcphdr));
         break;
     }
diff --git a/src/udp.c b/src/udp.c
index d23e72f..0dd1f67 100644
--- a/src/udp.c
+++ b/src/udp.c
@@ -67,6 +67,8 @@ void udp_cleanup(Slirp *slirp)
 void udp_input(register struct mbuf *m, int iphlen)
 {
     Slirp *slirp = m->slirp;
+    M_DUP_DEBUG(slirp, m, 0, 0);
+
     register struct ip *ip;
     register struct udphdr *uh;
     int len;
@@ -245,7 +247,8 @@ bad:
 int udp_output(struct socket *so, struct mbuf *m, struct sockaddr_in *saddr,
                struct sockaddr_in *daddr, int iptos)
 {
-    g_assert(M_ROOMBEFORE(m) >= sizeof(struct udpiphdr));
+    Slirp *slirp = m->slirp;
+    M_DUP_DEBUG(slirp, m, 0, sizeof(struct udpiphdr));
 
     register struct udpiphdr *ui;
     int error = 0;
diff --git a/src/udp6.c b/src/udp6.c
index 97156d1..18ce998 100644
--- a/src/udp6.c
+++ b/src/udp6.c
@@ -11,6 +11,8 @@
 void udp6_input(struct mbuf *m)
 {
     Slirp *slirp = m->slirp;
+    M_DUP_DEBUG(slirp, m, 0, 0);
+
     struct ip6 *ip, save_ip;
     struct udphdr *uh;
     int iphlen = sizeof(struct ip6);
@@ -153,7 +155,8 @@ bad:
 int udp6_output(struct socket *so, struct mbuf *m, struct sockaddr_in6 *saddr,
                 struct sockaddr_in6 *daddr)
 {
-    g_assert(M_ROOMBEFORE(m) >= sizeof(struct ip6) + sizeof(struct udphdr));
+    Slirp *slirp = m->slirp;
+    M_DUP_DEBUG(slirp, m, 0, sizeof(struct ip6) + sizeof(struct udphdr));
 
     struct ip6 *ip;
     struct udphdr *uh;