diff options
author | Samuel Thibault <samuel.thibault@ens-lyon.org> | 2021-06-06 15:07:47 +0000 |
---|---|---|
committer | Samuel Thibault <samuel.thibault@ens-lyon.org> | 2021-06-06 15:07:47 +0000 |
commit | 9c1ea76f28330a5413fce293e121ccdc5ec14202 (patch) | |
tree | 78d78bcbd4e627d371ace4002ad315946e8158f4 | |
parent | 5695ba4dcf388f17079b706dfbc984a42cd667ea (diff) | |
parent | 5853f708864c969cf930ad863fef4286865e0310 (diff) | |
download | slirp-9c1ea76f28330a5413fce293e121ccdc5ec14202.zip slirp-9c1ea76f28330a5413fce293e121ccdc5ec14202.tar.gz slirp-9c1ea76f28330a5413fce293e121ccdc5ec14202.tar.bz2 |
Merge branch 'valgrind' into 'master'
mbuf: Add debugging helpers for allocation
See merge request slirp/libslirp!90
-rw-r--r-- | meson.build | 4 | ||||
-rw-r--r-- | src/if.c | 2 | ||||
-rw-r--r-- | src/ip6_icmp.c | 7 | ||||
-rw-r--r-- | src/ip6_input.c | 5 | ||||
-rw-r--r-- | src/ip6_output.c | 3 | ||||
-rw-r--r-- | src/ip_icmp.c | 7 | ||||
-rw-r--r-- | src/ip_input.c | 2 | ||||
-rw-r--r-- | src/ip_output.c | 2 | ||||
-rw-r--r-- | src/mbuf.c | 40 | ||||
-rw-r--r-- | src/mbuf.h | 62 | ||||
-rw-r--r-- | src/tcp_input.c | 13 | ||||
-rw-r--r-- | src/udp.c | 5 | ||||
-rw-r--r-- | src/udp6.c | 5 |
13 files changed, 152 insertions, 5 deletions
diff --git a/meson.build b/meson.build index 19d6775..4407a77 100644 --- a/meson.build +++ b/meson.build @@ -72,6 +72,10 @@ cargs = [ '-DG_LOG_DOMAIN="Slirp"', ] +if cc.check_header('valgrind/valgrind.h') + cargs += [ '-DHAVE_VALGRIND=1' ] +endif + sources = [ 'src/arp_table.c', 'src/bootp.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 119c8be..738b40f 100644 --- a/src/ip6_icmp.c +++ b/src/ip6_icmp.c @@ -321,6 +321,8 @@ static void ndp_send_na(Slirp *slirp, struct ip6 *ip, struct icmp6 *icmp) static void ndp_input(struct mbuf *m, Slirp *slirp, struct ip6 *ip, struct icmp6 *icmp) { + g_assert(M_ROOMBEFORE(m) >= ETH_HLEN); + m->m_len += ETH_HLEN; m->m_data -= ETH_HLEN; struct ethhdr *eth = mtod(m, struct ethhdr *); @@ -383,9 +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 */ + 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 a83e4f8..b3d9865 100644 --- a/src/ip6_input.c +++ b/src/ip6_input.c @@ -23,8 +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 */ + 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 e86bed6..a29c324 100644 --- a/src/ip_input.c +++ b/src/ip_input.c @@ -70,6 +70,8 @@ void ip_cleanup(Slirp *slirp) void ip_input(struct mbuf *m) { Slirp *slirp = m->slirp; + 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); @@ -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; +} @@ -73,6 +73,13 @@ */ #define M_FREEROOM(m) (M_ROOM(m) - (m)->m_len) +/* + * How much free room there is before m_data + */ +#define M_ROOMBEFORE(m) \ + (((m)->m_flags & M_EXT) ? (m)->m_data - (m)->m_ext \ + : (m)->m_data - (m)->m_dat) + struct mbuf { /* XXX should union some of these! */ /* header at beginning of each mbuf: */ @@ -117,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) @@ -124,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 01e4c19..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; @@ -233,6 +236,16 @@ void tcp_input(struct mbuf *m, int iphlen, struct socket *inso, goto cont_conn; } slirp = m->slirp; + switch (af) { + case AF_INET: + M_DUP_DEBUG(slirp, m, 0, + sizeof(struct tcpiphdr) - sizeof(struct ip) - sizeof(struct tcphdr)); + break; + case AF_INET6: + M_DUP_DEBUG(slirp, m, 0, + sizeof(struct tcpiphdr) - sizeof(struct ip6) - sizeof(struct tcphdr)); + break; + } ip = mtod(m, struct ip *); ip6 = mtod(m, struct ip6 *); @@ -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,6 +247,9 @@ bad: int udp_output(struct socket *so, struct mbuf *m, struct sockaddr_in *saddr, struct sockaddr_in *daddr, int iptos) { + Slirp *slirp = m->slirp; + M_DUP_DEBUG(slirp, m, 0, sizeof(struct udpiphdr)); + register struct udpiphdr *ui; int error = 0; @@ -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,6 +155,9 @@ bad: int udp6_output(struct socket *so, struct mbuf *m, struct sockaddr_in6 *saddr, struct sockaddr_in6 *daddr) { + Slirp *slirp = m->slirp; + M_DUP_DEBUG(slirp, m, 0, sizeof(struct ip6) + sizeof(struct udphdr)); + struct ip6 *ip; struct udphdr *uh; |