From 632dd719b385016eb62273f12fe51512453624c1 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 9 Aug 2018 23:52:59 +0200 Subject: [PATCH 1/3] slirp: document mbuf pointers and sizes and fix confusing datasize name into gapsize in m_inc. Signed-off-by: Peter Maydell Signed-off-by: Samuel Thibault --- slirp/mbuf.c | 14 +++++++------- slirp/mbuf.h | 13 +++++++++++++ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/slirp/mbuf.c b/slirp/mbuf.c index 1b7868355a..aa1f28afb1 100644 --- a/slirp/mbuf.c +++ b/slirp/mbuf.c @@ -151,7 +151,7 @@ m_cat(struct mbuf *m, struct mbuf *n) void m_inc(struct mbuf *m, int size) { - int datasize; + int gapsize; /* some compilers throw up on gotos. This one we can fake. */ if (M_ROOM(m) > size) { @@ -159,17 +159,17 @@ m_inc(struct mbuf *m, int size) } if (m->m_flags & M_EXT) { - datasize = m->m_data - m->m_ext; - m->m_ext = g_realloc(m->m_ext, size + datasize); + gapsize = m->m_data - m->m_ext; + m->m_ext = g_realloc(m->m_ext, size + gapsize); } else { - datasize = m->m_data - m->m_dat; - m->m_ext = g_malloc(size + datasize); + gapsize = m->m_data - m->m_dat; + m->m_ext = g_malloc(size + gapsize); memcpy(m->m_ext, m->m_dat, m->m_size); m->m_flags |= M_EXT; } - m->m_data = m->m_ext + datasize; - m->m_size = size + datasize; + m->m_data = m->m_ext + gapsize; + m->m_size = size + gapsize; } diff --git a/slirp/mbuf.h b/slirp/mbuf.h index 33b84485d6..bfdf8c4577 100644 --- a/slirp/mbuf.h +++ b/slirp/mbuf.h @@ -47,6 +47,19 @@ * free the m_ext. This is inefficient memory-wise, but who cares. */ +/* + * mbufs allow to have a gap between the start of the allocated buffer (m_ext if + * M_EXT is set, m_dat otherwise) and the in-use data: + * + * |--gapsize----->|---m_len-------> + * |----------m_size------------------------------> + * |----M_ROOM--------------------> + * |-M_FREEROOM--> + * + * ^ ^ ^ + * m_dat/m_ext m_data end of buffer + */ + /* * How much room is in the mbuf, from m_data to the end of the mbuf */ From 3d090aefe29846f8606d06bf55526ebb91f4d725 Mon Sep 17 00:00:00 2001 From: Andrew Oates Date: Wed, 15 Aug 2018 20:18:45 -0400 Subject: [PATCH 2/3] slirp: fix ICMP handling on macOS hosts On Linux, SOCK_DGRAM+IPPROTO_ICMP sockets give only the ICMP packet when read from. On macOS, however, the socket acts like a SOCK_RAW socket and includes the IP header as well. This change strips the extra IP header from the received packet on macOS before sending it to the guest. SOCK_DGRAM ICMP sockets aren't supported on other BSDs, but we enable this behavior for them as well to treat the sockets the same as raw sockets. Signed-off-by: Andrew Oates Signed-off-by: Samuel Thibault --- slirp/ip_icmp.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c index 0b667a429a..da100d1f55 100644 --- a/slirp/ip_icmp.c +++ b/slirp/ip_icmp.c @@ -420,7 +420,32 @@ void icmp_receive(struct socket *so) icp = mtod(m, struct icmp *); id = icp->icmp_id; - len = qemu_recv(so->s, icp, m->m_len, 0); + len = qemu_recv(so->s, icp, M_ROOM(m), 0); + /* + * The behavior of reading SOCK_DGRAM+IPPROTO_ICMP sockets is inconsistent + * between host OSes. On Linux, only the ICMP header and payload is + * included. On macOS/Darwin, the socket acts like a raw socket and + * includes the IP header as well. On other BSDs, SOCK_DGRAM+IPPROTO_ICMP + * sockets aren't supported at all, so we treat them like raw sockets. It + * isn't possible to detect this difference at runtime, so we must use an + * #ifdef to determine if we need to remove the IP header. + */ +#ifdef CONFIG_BSD + if (len >= sizeof(struct ip)) { + struct ip *inner_ip = mtod(m, struct ip *); + int inner_hlen = inner_ip->ip_hl << 2; + if (inner_hlen > len) { + len = -1; + errno = -EINVAL; + } else { + len -= inner_hlen; + memmove(icp, (unsigned char *)icp + inner_hlen, len); + } + } else { + len = -1; + errno = -EINVAL; + } +#endif icp->icmp_id = id; m->m_data -= hlen; From 93a972f8548571d35c718ca3a94d5ab1507b2443 Mon Sep 17 00:00:00 2001 From: Gavin Grant Date: Thu, 30 Aug 2018 16:57:57 +0100 Subject: [PATCH 3/3] slirp: Propagate host TCP RST packet to the guest after socket disconnected Commit 27d92ebc5ed1bb0b518d0ebc4c609182ad20a799 handled the case where the TCP connection is abruptly closed via a RST packet, by checking for the ECONNRESET errno. However it does not consider the case where the connection has been half-closed by the host (FIN/ACK), then the host socket is disconnected. For example, if the host application calls close() on the socket, then the application exits. In this case, the socket still exists due to the file descriptor in SLIRP, but it is disconnected. recv() does not indicate an error since an orderly socket close has previously occurred. The socket will then be stuck in FIN_WAIT_2, until the peer sends FIN/ACK or a timeout occurs. Instead we can send a RST to the peer and transition to the CLOSED state. Signed-off-by: Gavin Grant Signed-off-by: Samuel Thibault --- slirp/socket.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/slirp/socket.c b/slirp/socket.c index 08fe98907d..322383a1f9 100644 --- a/slirp/socket.c +++ b/slirp/socket.c @@ -204,12 +204,19 @@ soread(struct socket *so) return 0; else { int err; - socklen_t slen = sizeof err; + socklen_t elen = sizeof err; + struct sockaddr_storage addr; + struct sockaddr *paddr = (struct sockaddr *) &addr; + socklen_t alen = sizeof addr; err = errno; if (nn == 0) { - getsockopt(so->s, SOL_SOCKET, SO_ERROR, - &err, &slen); + if (getpeername(so->s, paddr, &alen) < 0) { + err = errno; + } else { + getsockopt(so->s, SOL_SOCKET, SO_ERROR, + &err, &elen); + } } DEBUG_MISC((dfd, " --- soread() disconnected, nn = %d, errno = %d-%s\n", nn, errno,strerror(errno)));