.gitlab-ci.yml

@@ -4,6 +4,7 @@ variables:
   DEPS: meson ninja-build make
         gcc pkg-config glib2-devel
         mingw64-gcc mingw64-pkg-config mingw64-glib2
+        clang-analyzer
 
 before_script:
   - dnf install -y $DEPS
@@ -13,6 +14,7 @@ build:
     - meson build || (cat build/meson-logs/meson-log.txt && exit 1)
     - ninja -C build
     - (cd build && meson test) || (cat build/meson-logs/testlog.txt && exit 1)
+    - ninja -C build scan-build
 
 build-mingw64:
   script:

CHANGELOG.md

+# Changelog
+
+All notable changes to this project will be documented in this file.
+
+The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
+and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
+
+## [4.1.0] - 2019-12-02
+
+### Added
+
+ - The `slirp_new()` API, simpler and more extensible than `slirp_init()`.
+ - Allow custom MTU configuration.
+ - Option to disable host loopback connections.
+ - CI now runs scan-build too.
+
+### Changed
+
+ - Disable `tcp_emu()` by default. `tcp_emu()` is known to have caused
+   several CVEs, and not useful today in most cases. The feature can
+   be still enabled by setting `SlirpConfig.enable_emu` to true.
+ - meson build system is now `subproject()` friendly.
+ - Replace remaining `malloc()`/`free()` with glib (which aborts on OOM)
+ - Various code cleanups.
+
+### Deprecated
+
+ - The `slirp_init()` API.
+
+### Fixed
+
+ - `getpeername()` error after `shutdown(SHUT_WR)`.
+ - Exec forward: correctly parse command lines that contain spaces.
+ - Allow 0.0.0.0 destination address.
+ - Make host receive broadcast packets.
+ - Various memory related fixes (heap overflow, leaks, NULL
+   dereference).
+ - Compilation warnings, dead code.
+
+## [4.0.0] - 2019-05-24
+
+### Added
+
+ - Installable as a shared library.
+ - meson build system
+   (& make build system for in-tree QEMU integration)
+
+### Changed
+
+ - Standalone project, removing any QEMU dependency.
+ - License clarifications.
+
+[unreleased]: https://gitlab.freedesktop.org/slirp/libslirp/compare/v4.1.0...master
+[4.1.0]: https://gitlab.freedesktop.org/slirp/libslirp/compare/v4.0.0...v4.1.0
+[4.0.0]: https://gitlab.freedesktop.org/slirp/libslirp/commits/v4.0.0

Makefile

@@ -3,7 +3,7 @@ BUILD_DIR ?= .
 
 LIBSLIRP = $(BUILD_DIR)/libslirp.a
 SLIRP_MAJOR_VERSION = 4
-SLIRP_MINOR_VERSION = 0
+SLIRP_MINOR_VERSION = 1
 SLIRP_MICRO_VERSION = 0
 
 all: $(LIBSLIRP)

README.md

@@ -14,13 +14,15 @@ of dependencies on Fedora)
 
 ### Building
 
-QEMU uses a static library built with ``make``. But you may also build
-and install the shared library with meson:
+You may build and install the shared library with meson:
 
 ``` sh
 meson build
 ninja -C build install
 ```
+And configure QEMU with --enable-slirp=system to link against it.
+
+(QEMU may build with the submodule static library using --enable-slirp=git)
 
 ### Testing
 

meson.build

 project('libslirp', 'c',
-  version : '4.0.0',
+  version : '4.1.0',
   license : 'BSD-3-Clause',
   default_options : ['warning_level=1', 'c_std=gnu99']
 )
@@ -32,10 +32,10 @@ conf.set('SLIRP_MICRO_VERSION', micro_version)
 # - If the interface is the same as the previous version, but bugs are
 #   fixed, change:
 #      REVISION += 1
-lt_current = '0'
-lt_revision = '0'
-lt_age = '0'
-lt_version = '@0@.@1@.@2@'.format(lt_current, lt_age, lt_revision)
+lt_current = 1
+lt_revision = 0
+lt_age = 1
+lt_version = '@0@.@1@.@2@'.format(lt_current - lt_age, lt_age, lt_revision)
 
 host_system = host_machine.system()
 
@@ -100,8 +100,7 @@ configure_file(
   configuration : conf
 )
 
-lib = shared_library('slirp', sources,
-  soversion : lt_current,
+lib = library('slirp', sources,
   version : lt_version,
   c_args : cargs,
   link_args : vflag,
@@ -110,6 +109,10 @@ lib = shared_library('slirp', sources,
   install : true
 )
 
+libslirp_dep = declare_dependency(
+  include_directories: include_directories('.', 'src'),
+  link_with: lib)
+
 install_headers(['src/libslirp.h'], subdir : 'slirp')
 
 pkg = import('pkgconfig')

src/arp_table.c

@@ -71,7 +71,7 @@ bool arp_table_search(Slirp *slirp, uint32_t ip_addr,
     DEBUG_ARG("ip = %s", inet_ntoa((struct in_addr){ .s_addr = ip_addr }));
 
     /* If broadcast address */
-    if (ip_addr == 0xffffffff || ip_addr == broadcast_addr) {
+    if (ip_addr == 0 || ip_addr == 0xffffffff || ip_addr == broadcast_addr) {
         /* return Ethernet broadcast address */
         memset(out_ethaddr, 0xff, ETH_ALEN);
         return 1;

src/bootp.c

@@ -350,14 +350,12 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
         q += sizeof(nak_msg) - 1;
     }
     assert(q < end);
-    *q =
-RFC1533_END
-;
+    *q = RFC1533_END;
 
-daddr.sin_addr.s_addr = 0xffffffffu;
+    daddr.sin_addr.s_addr = 0xffffffffu;
 
-m->m_len = sizeof(struct bootp_t) - sizeof(struct ip) - sizeof(struct udphdr);
-udp_output(NULL, m, &saddr, &daddr, IPTOS_LOWDELAY);
+    m->m_len = sizeof(struct bootp_t) - sizeof(struct ip) - sizeof(struct udphdr);
+    udp_output(NULL, m, &saddr, &daddr, IPTOS_LOWDELAY);
 }
 
 void bootp_input(struct mbuf *m)

src/dhcpv6.c

@@ -178,7 +178,7 @@ static void dhcpv6_info_request(Slirp *slirp, struct sockaddr_in6 *srcsas,
 
         *resp++ = OPTION_BOOTFILE_URL >> 8; /* option-code high byte */
         *resp++ = OPTION_BOOTFILE_URL; /* option-code low byte */
-        smaxlen = (uint8_t *)m->m_data + IF_MTU - (resp + 2);
+        smaxlen = (uint8_t *)m->m_data + slirp->if_mtu - (resp + 2);
         slen = snprintf((char *)resp + 2, smaxlen,
                         "tftp://[%02x%02x:%02x%02x:%02x%02x:%02x%02x:"
                         "%02x%02x:%02x%02x:%02x%02x:%02x%02x]/%s",

src/dnssearch.c

@@ -238,13 +238,8 @@ int translate_dnssearch(Slirp *s, const char **names)
     size_t i, num_domains, memreq = 0;
     uint8_t *result = NULL, *outptr;
     CompactDomain *domains = NULL;
-    const char **nameptr = names;
 
-    while (*nameptr != NULL) {
-        nameptr++;
-    }
-
-    num_domains = nameptr - names;
+    num_domains = g_strv_length((GStrv)names);
     if (num_domains == 0) {
         return -2;
     }

src/if.h

@@ -11,8 +11,12 @@
 #define IF_AUTOCOMP 0x04 /* Autodetect (default) */
 #define IF_NOCIDCOMP 0x08 /* CID compression */
 
-#define IF_MTU 1500
-#define IF_MRU 1500
+#define IF_MTU_DEFAULT 1500
+#define IF_MTU_MIN 68
+#define IF_MTU_MAX 65521
+#define IF_MRU_DEFAULT 1500
+#define IF_MRU_MIN 68
+#define IF_MRU_MAX 65521
 #define IF_COMP IF_AUTOCOMP /* Flags for compression */
 
 /* 2 for alignment, 14 for ethernet */

src/ip6_icmp.c

@@ -94,8 +94,8 @@ void icmp6_send_error(struct mbuf *m, uint8_t type, uint8_t code)
     DEBUG_ARG("target = %s", addrstr);
 
     rip->ip_nh = IPPROTO_ICMPV6;
-    const int error_data_len =
-        MIN(m->m_len, IF_MTU - (sizeof(struct ip6) + ICMP6_ERROR_MINLEN));
+    const int error_data_len = MIN(
+        m->m_len, slirp->if_mtu - (sizeof(struct ip6) + ICMP6_ERROR_MINLEN));
     rip->ip_pl = htons(ICMP6_ERROR_MINLEN + error_data_len);
     t->m_len = sizeof(struct ip6) + ntohs(rip->ip_pl);
 
@@ -112,7 +112,7 @@ void icmp6_send_error(struct mbuf *m, uint8_t type, uint8_t code)
         ricmp->icmp6_err.unused = 0;
         break;
     case ICMP6_TOOBIG:
-        ricmp->icmp6_err.mtu = htonl(IF_MTU);
+        ricmp->icmp6_err.mtu = htonl(slirp->if_mtu);
         break;
     case ICMP6_PARAMPROB:
         /* TODO: Handle this case */

src/ip6_input.c

@@ -44,7 +44,7 @@ void ip6_input(struct mbuf *m)
         goto bad;
     }
 
-    if (ntohs(ip6->ip_pl) > IF_MTU) {
+    if (ntohs(ip6->ip_pl) > slirp->if_mtu) {
         icmp6_send_error(m, ICMP6_TOOBIG, 0);
         goto bad;
     }

src/ip_icmp.c

@@ -190,7 +190,12 @@ void icmp_input(struct mbuf *m, int hlen)
 
             /* Send the packet */
             addr = so->fhost.ss;
-            sotranslate_out(so, &addr);
+            if (sotranslate_out(so, &addr) < 0) {
+                icmp_send_error(m, ICMP_UNREACH, ICMP_UNREACH_NET, 0,
+                                strerror(errno));
+                udp_detach(so);
+                return;
+            }
 
             if (sendto(so->s, icmp_ping_msg, strlen(icmp_ping_msg), 0,
                        (struct sockaddr *)&addr, sockaddr_size(&addr)) == -1) {

src/ip_input.c

@@ -292,6 +292,7 @@ static struct ip *ip_reass(Slirp *slirp, struct ip *ip, struct ipq *fp)
      */
     while (q != (struct ipasfrag *)&fp->frag_link &&
            ip->ip_off + ip->ip_len > q->ipf_off) {
+        struct ipasfrag *prev;
         i = (ip->ip_off + ip->ip_len) - q->ipf_off;
         if (i < q->ipf_len) {
             q->ipf_len -= i;
@@ -299,9 +300,10 @@ static struct ip *ip_reass(Slirp *slirp, struct ip *ip, struct ipq *fp)
             m_adj(dtom(slirp, q), i);
             break;
         }
+        prev = q;
         q = q->ipf_next;
-        m_free(dtom(slirp, q->ipf_prev));
-        ip_deq(q->ipf_prev);
+        ip_deq(prev);
+        m_free(dtom(slirp, prev));
     }
 
 insert:
@@ -326,6 +328,8 @@ insert:
     q = fp->frag_link.next;
     m = dtom(slirp, q);
 
+    int was_ext = m->m_flags & M_EXT;
+
     q = (struct ipasfrag *)q->ipf_next;
     while (q != (struct ipasfrag *)&fp->frag_link) {
         struct mbuf *t = dtom(slirp, q);
@@ -342,13 +346,12 @@ insert:
     q = fp->frag_link.next;
 
     /*
-     * If the fragments concatenated to an mbuf that's
-     * bigger than the total size of the fragment, then and
-     * m_ext buffer was alloced. But fp->ipq_next points to
-     * the old buffer (in the mbuf), so we must point ip
-     * into the new buffer.
+     * If the fragments concatenated to an mbuf that's bigger than the total
+     * size of the fragment and the mbuf was not already using an m_ext buffer,
+     * then an m_ext buffer was alloced. But fp->ipq_next points to the old
+     * buffer (in the mbuf), so we must point ip into the new buffer.
      */
-    if (m->m_flags & M_EXT) {
+    if (!was_ext && m->m_flags & M_EXT) {
         int delta = (char *)q - m->m_dat;
         q = (struct ipasfrag *)(m->m_ext + delta);
     }

src/ip_output.c

@@ -72,7 +72,7 @@ int ip_output(struct socket *so, struct mbuf *m0)
     /*
      * If small enough for interface, can just send directly.
      */
-    if ((uint16_t)ip->ip_len <= IF_MTU) {
+    if ((uint16_t)ip->ip_len <= slirp->if_mtu) {
         ip->ip_len = htons((uint16_t)ip->ip_len);
         ip->ip_off = htons((uint16_t)ip->ip_off);
         ip->ip_sum = 0;
@@ -91,7 +91,7 @@ int ip_output(struct socket *so, struct mbuf *m0)
         goto bad;
     }
 
-    len = (IF_MTU - hlen) & ~7; /* ip databytes per packet */
+    len = (slirp->if_mtu - hlen) & ~7; /* ip databytes per packet */
     if (len < 8) {
         error = -1;
         goto bad;

src/libslirp.h

@@ -66,7 +66,52 @@ typedef struct SlirpCb {
     void (*notify)(void *opaque);
 } SlirpCb;
 
+#define SLIRP_CONFIG_VERSION_MIN 1
+#define SLIRP_CONFIG_VERSION_MAX 1
 
+typedef struct SlirpConfig {
+    /* Version must be provided */
+    uint32_t version;
+    /*
+     * Fields introduced in SlirpConfig version 1 begin
+     */
+    int restricted;
+    bool in_enabled;
+    struct in_addr vnetwork;
+    struct in_addr vnetmask;
+    struct in_addr vhost;
+    bool in6_enabled;
+    struct in6_addr vprefix_addr6;
+    uint8_t vprefix_len;
+    struct in6_addr vhost6;
+    const char *vhostname;
+    const char *tftp_server_name;
+    const char *tftp_path;
+    const char *bootfile;
+    struct in_addr vdhcp_start;
+    struct in_addr vnameserver;
+    struct in6_addr vnameserver6;
+    const char **vdnssearch;
+    const char *vdomainname;
+    /* Default: IF_MTU_DEFAULT */
+    size_t if_mtu;
+    /* Default: IF_MRU_DEFAULT */
+    size_t if_mru;
+    /* Prohibit connecting to 127.0.0.1:* */
+    bool disable_host_loopback;
+    /*
+     * Enable emulation code (*warning*: this code isn't safe, it is not
+     * recommended to enable it)
+     */
+    bool enable_emu;
+    /*
+     * Fields introduced in SlirpConfig version 2 begin
+     */
+} SlirpConfig;
+
+Slirp *slirp_new(const SlirpConfig *cfg, const SlirpCb *callbacks,
+                 void *opaque);
+/* slirp_init is deprecated in favor of slirp_new */
 Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
                   struct in_addr vnetmask, struct in_addr vhost,
                   bool in6_enabled, struct in6_addr vprefix_addr6,

src/libslirp.map

@@ -19,3 +19,7 @@ global:
 local:
     *;
 };
+
+SLIRP_4.1 {
+    slirp_new;
+} SLIRP_4.0;

src/mbuf.c

@@ -20,8 +20,8 @@
 /*
  * Find a nice value for msize
  */
-#define SLIRP_MSIZE \
-    (offsetof(struct mbuf, m_dat) + IF_MAXLINKHDR + TCPIPHDR_DELTA + IF_MTU)
+#define SLIRP_MSIZE(mtu) \
+    (offsetof(struct mbuf, m_dat) + IF_MAXLINKHDR + TCPIPHDR_DELTA + (mtu))
 
 void m_init(Slirp *slirp)
 {
@@ -66,7 +66,7 @@ struct mbuf *m_get(Slirp *slirp)
     DEBUG_CALL("m_get");
 
     if (slirp->m_freelist.qh_link == &slirp->m_freelist) {
-        m = g_malloc(SLIRP_MSIZE);
+        m = g_malloc(SLIRP_MSIZE(slirp->if_mtu));
         slirp->mbuf_alloced++;
         if (slirp->mbuf_alloced > MBUF_THRESH)
             flags = M_DOFREE;
@@ -81,7 +81,7 @@ struct mbuf *m_get(Slirp *slirp)
     m->m_flags = (flags | M_USEDLIST);
 
     /* Initialise it */
-    m->m_size = SLIRP_MSIZE - offsetof(struct mbuf, m_dat);
+    m->m_size = SLIRP_MSIZE(slirp->if_mtu) - offsetof(struct mbuf, m_dat);
     m->m_data = m->m_dat;
     m->m_len = 0;
     m->m_nextpkt = NULL;

src/misc.c

@@ -168,7 +168,8 @@ g_spawn_async_with_fds_slirp(const gchar *working_directory, gchar **argv,
 int fork_exec(struct socket *so, const char *ex)
 {
     GError *err = NULL;
-    char **argv;
+    gint argc = 0;
+    gchar **argv = NULL;
     int opt, sp[2];
 
     DEBUG_CALL("fork_exec");
@@ -179,7 +180,12 @@ int fork_exec(struct socket *so, const char *ex)
         return 0;
     }
 
-    argv = g_strsplit(ex, " ", -1);
+    if (!g_shell_parse_argv(ex, &argc, &argv, &err)) {
+        g_critical("fork_exec invalid command: %s\nerror: %s", ex, err->message);
+        g_error_free(err);
+        return 0;
+    }
+
     g_spawn_async_with_fds(NULL /* cwd */, argv, NULL /* env */,
                            G_SPAWN_SEARCH_PATH, fork_exec_child_setup,
                            NULL /* data */, NULL /* child_pid */, sp[1], sp[1],

src/sbuf.c

@@ -9,19 +9,17 @@ static void sbappendsb(struct sbuf *sb, struct mbuf *m);
 
 void sbfree(struct sbuf *sb)
 {
-    free(sb->sb_data);
+    g_free(sb->sb_data);
 }
 
-bool sbdrop(struct sbuf *sb, int num)
+bool sbdrop(struct sbuf *sb, size_t num)
 {
     int limit = sb->sb_datalen / 2;
 
-    /*
-     * We can only drop how much we have
-     * This should never succeed
-     */
+    g_warn_if_fail(num <= sb->sb_cc);
     if (num > sb->sb_cc)
         num = sb->sb_cc;
+
     sb->sb_cc -= num;
     sb->sb_rptr += num;
     if (sb->sb_rptr >= sb->sb_data + sb->sb_datalen)
@@ -34,27 +32,11 @@ bool sbdrop(struct sbuf *sb, int num)
     return false;
 }
 
-void sbreserve(struct sbuf *sb, int size)
+void sbreserve(struct sbuf *sb, size_t size)
 {
-    if (sb->sb_data) {
-        /* Already alloced, realloc if necessary */
-        if (sb->sb_datalen != size) {
-            sb->sb_wptr = sb->sb_rptr = sb->sb_data =
-                (char *)realloc(sb->sb_data, size);
-            sb->sb_cc = 0;
-            if (sb->sb_wptr)
-                sb->sb_datalen = size;
-            else
-                sb->sb_datalen = 0;
-        }
-    } else {
-        sb->sb_wptr = sb->sb_rptr = sb->sb_data = (char *)malloc(size);
-        sb->sb_cc = 0;
-        if (sb->sb_wptr)
-            sb->sb_datalen = size;
-        else
-            sb->sb_datalen = 0;
-    }
+    sb->sb_wptr = sb->sb_rptr = sb->sb_data = g_realloc(sb->sb_data, size);
+    sb->sb_cc = 0;
+    sb->sb_datalen = size;
 }
 
 /*
@@ -161,17 +143,17 @@ static void sbappendsb(struct sbuf *sb, struct mbuf *m)
  * Don't update the sbuf rptr, this will be
  * done in sbdrop when the data is acked
  */
-void sbcopy(struct sbuf *sb, int off, int len, char *to)
+void sbcopy(struct sbuf *sb, size_t off, size_t len, char *to)
 {
     char *from;
 
+    g_assert(len + off <= sb->sb_cc);
+
     from = sb->sb_rptr + off;
     if (from >= sb->sb_data + sb->sb_datalen)
         from -= sb->sb_datalen;
 
     if (from < sb->sb_wptr) {
-        if (len > sb->sb_cc)
-            len = sb->sb_cc;
         memcpy(to, from, len);
     } else {
         /* re-use off */