[RFC PATCH 0/4] net: support for CFI with libslirp >= 4.7

[RFC PATCH 0/4] net: support for CFI with libslirp >= 4.7

[Paolo Bonzini]

A system libslirp (either static or dynamic) cannot be used with QEMU if
QEMU is compiled with control-flow instrumentation, because of the way
timers are implemented in libslirp.   libslirp passes a function pointer
to the timer_new callback but the type information for the callback is
missing; invoking the timer callback produces a CFI false positive.

The fix requires the introduction of new interfaces in
libslirp.  This series is an example of how QEMU would use
the new interfaces introduced by libslirp merge request at
https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/117.
It is RFC-only because the new interfaces have not been accepted yet.

Paolo Bonzini (4):
  net: slirp: introduce a wrapper struct for QemuTimer
  net: slirp: switch to slirp_new
  net: slirp: add support for CFI-friendly timer API
  net: slirp: allow CFI with libslirp >= 4.7

 meson.build | 24 +++++++--------
 net/slirp.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 86 insertions(+), 23 deletions(-)

-- 
2.35.1

[RFC PATCH 1/4] net: slirp: introduce a wrapper struct for QemuTimer

[Paolo Bonzini]

This struct will be extended in the next few patches to support the
new slirp_handle_timer() call.  For that we need to store an additional
"int" for each SLIRP timer, in addition to the cb_opaque.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 net/slirp.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index bc5e9e4f77..f1e25d741f 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -184,23 +184,32 @@ static int64_t net_slirp_clock_get_ns(void *opaque)
     return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 }
 
+struct SlirpTimer {
+    QEMUTimer timer;
+}
+
 static void *net_slirp_timer_new(SlirpTimerCb cb,
                                  void *cb_opaque, void *opaque)
 {
-    return timer_new_full(NULL, QEMU_CLOCK_VIRTUAL,
-                          SCALE_MS, QEMU_TIMER_ATTR_EXTERNAL,
-                          cb, cb_opaque);
+    SlirpTimer *t = g_new(SlirpTimer, 1);
+    timer_init_full(&t->timer, NULL, QEMU_CLOCK_VIRTUAL,
+                    SCALE_MS, QEMU_TIMER_ATTR_EXTERNAL,
+                    cb, cb_opaque);
+    return t;
 }
 
 static void net_slirp_timer_free(void *timer, void *opaque)
 {
-    timer_free(timer);
+    SlirpTimer *t = timer;
+    timer_del(&t->timer);
+    g_free(t);
 }
 
 static void net_slirp_timer_mod(void *timer, int64_t expire_timer,
                                 void *opaque)
 {
-    timer_mod(timer, expire_timer);
+    SlirpTimer *t = timer;
+    timer_mod(&t->timer, expire_timer);
 }
 
 static void net_slirp_register_poll_fd(int fd, void *opaque)
-- 
2.35.1

Re: [RFC PATCH 1/4] net: slirp: introduce a wrapper struct for QemuTimer

[Samuel Thibault]

Paolo Bonzini, le mar. 12 avril 2022 14:13:34 +0200, a ecrit:
> This struct will be extended in the next few patches to support the
> new slirp_handle_timer() call.  For that we need to store an additional
> "int" for each SLIRP timer, in addition to the cb_opaque.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

> ---
>  net/slirp.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index bc5e9e4f77..f1e25d741f 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -184,23 +184,32 @@ static int64_t net_slirp_clock_get_ns(void *opaque)
>      return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>  }
>  
> +struct SlirpTimer {
> +    QEMUTimer timer;
> +}
> +
>  static void *net_slirp_timer_new(SlirpTimerCb cb,
>                                   void *cb_opaque, void *opaque)
>  {
> -    return timer_new_full(NULL, QEMU_CLOCK_VIRTUAL,
> -                          SCALE_MS, QEMU_TIMER_ATTR_EXTERNAL,
> -                          cb, cb_opaque);
> +    SlirpTimer *t = g_new(SlirpTimer, 1);
> +    timer_init_full(&t->timer, NULL, QEMU_CLOCK_VIRTUAL,
> +                    SCALE_MS, QEMU_TIMER_ATTR_EXTERNAL,
> +                    cb, cb_opaque);
> +    return t;
>  }
>  
>  static void net_slirp_timer_free(void *timer, void *opaque)
>  {
> -    timer_free(timer);
> +    SlirpTimer *t = timer;
> +    timer_del(&t->timer);
> +    g_free(t);
>  }
>  
>  static void net_slirp_timer_mod(void *timer, int64_t expire_timer,
>                                  void *opaque)
>  {
> -    timer_mod(timer, expire_timer);
> +    SlirpTimer *t = timer;
> +    timer_mod(&t->timer, expire_timer);
>  }
>  
>  static void net_slirp_register_poll_fd(int fd, void *opaque)
> -- 
> 2.35.1
> 
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.

[RFC PATCH 2/4] net: slirp: switch to slirp_new

[Paolo Bonzini]

Replace slirp_init with slirp_new, so that a more recent cfg.version
can be specified.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 net/slirp.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index f1e25d741f..b3a92d6e38 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -389,6 +389,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
 #if defined(CONFIG_SMBD_COMMAND)
     struct in_addr smbsrv = { .s_addr = 0 };
 #endif
+    SlirpConfig cfg = { 0 };
     NetClientState *nc;
     SlirpState *s;
     char buf[20];
@@ -577,12 +578,26 @@ static int net_slirp_init(NetClientState *peer, const char *model,
 
     s = DO_UPCAST(SlirpState, nc, nc);
 
-    s->slirp = slirp_init(restricted, ipv4, net, mask, host,
-                          ipv6, ip6_prefix, vprefix6_len, ip6_host,
-                          vhostname, tftp_server_name,
-                          tftp_export, bootfile, dhcp,
-                          dns, ip6_dns, dnssearch, vdomainname,
-                          &slirp_cb, s);
+    cfg.version = 3;
+    cfg.restricted = restricted;
+    cfg.in_enabled = ipv4;
+    cfg.vnetwork = net;
+    cfg.vnetmask = mask;
+    cfg.vhost = host;
+    cfg.in6_enabled = ipv6;
+    cfg.vprefix_addr6 = ip6_prefix;
+    cfg.vprefix_len = vprefix6_len;
+    cfg.vhost6 = ip6_host;
+    cfg.vhostname = vhostname;
+    cfg.tftp_server_name = tftp_server_name;
+    cfg.tftp_path = tftp_export;
+    cfg.bootfile = bootfile;
+    cfg.vdhcp_start = dhcp;
+    cfg.vnameserver = dns;
+    cfg.vnameserver6 = ip6_dns;
+    cfg.vdnssearch = dnssearch;
+    cfg.vdomainname = vdomainname;
+    s->slirp = slirp_new(&cfg, &slirp_cb, s);
     QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
 
     /*
-- 
2.35.1

Re: [RFC PATCH 2/4] net: slirp: switch to slirp_new

[Samuel Thibault]

Paolo Bonzini, le mar. 12 avril 2022 14:13:35 +0200, a ecrit:
> Replace slirp_init with slirp_new, so that a more recent cfg.version
> can be specified.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

> ---
>  net/slirp.c | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index f1e25d741f..b3a92d6e38 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -389,6 +389,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>  #if defined(CONFIG_SMBD_COMMAND)
>      struct in_addr smbsrv = { .s_addr = 0 };
>  #endif
> +    SlirpConfig cfg = { 0 };
>      NetClientState *nc;
>      SlirpState *s;
>      char buf[20];
> @@ -577,12 +578,26 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>  
>      s = DO_UPCAST(SlirpState, nc, nc);
>  
> -    s->slirp = slirp_init(restricted, ipv4, net, mask, host,
> -                          ipv6, ip6_prefix, vprefix6_len, ip6_host,
> -                          vhostname, tftp_server_name,
> -                          tftp_export, bootfile, dhcp,
> -                          dns, ip6_dns, dnssearch, vdomainname,
> -                          &slirp_cb, s);
> +    cfg.version = 3;
> +    cfg.restricted = restricted;
> +    cfg.in_enabled = ipv4;
> +    cfg.vnetwork = net;
> +    cfg.vnetmask = mask;
> +    cfg.vhost = host;
> +    cfg.in6_enabled = ipv6;
> +    cfg.vprefix_addr6 = ip6_prefix;
> +    cfg.vprefix_len = vprefix6_len;
> +    cfg.vhost6 = ip6_host;
> +    cfg.vhostname = vhostname;
> +    cfg.tftp_server_name = tftp_server_name;
> +    cfg.tftp_path = tftp_export;
> +    cfg.bootfile = bootfile;
> +    cfg.vdhcp_start = dhcp;
> +    cfg.vnameserver = dns;
> +    cfg.vnameserver6 = ip6_dns;
> +    cfg.vdnssearch = dnssearch;
> +    cfg.vdomainname = vdomainname;
> +    s->slirp = slirp_new(&cfg, &slirp_cb, s);
>      QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
>  
>      /*
> -- 
> 2.35.1
> 
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.

[RFC PATCH 3/4] net: slirp: add support for CFI-friendly timer API

[Paolo Bonzini]

libslirp 4.7 introduces a CFI-friendly version of the .timer_new callback.
The new callback replaces the function pointer with an enum; invoking the
callback is done with a new function slirp_handle_timer.

Support the new API so that CFI can be made compatible with using a system
libslirp.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 net/slirp.c | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/net/slirp.c b/net/slirp.c
index b3a92d6e38..57af42299d 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -184,10 +184,43 @@ static int64_t net_slirp_clock_get_ns(void *opaque)
     return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 }
 
+typedef struct SlirpTimer SlirpTimer;
 struct SlirpTimer {
     QEMUTimer timer;
+#if SLIRP_CHECK_VERSION(4,7,0)
+    Slirp *slirp;
+    SlirpTimerId id;
+    void *cb_opaque;
+#endif
+};
+
+#if SLIRP_CHECK_VERSION(4,7,0)
+static void net_slirp_init_completed(Slirp *slirp, void *opaque)
+{
+    SlirpState *s = opaque;
+    s->slirp = slirp;
 }
 
+static void net_slirp_timer_cb(void *opaque)
+{
+    SlirpTimer *t = opaque;
+    slirp_handle_timer(t->slirp, t->id, t->cb_opaque);
+}
+
+static void *net_slirp_timer_new_opaque(SlirpTimerId id,
+                                        void *cb_opaque, void *opaque)
+{
+    SlirpState *s = opaque;
+    SlirpTimer *t = g_new(SlirpTimer, 1);
+    t->slirp = s->slirp;
+    t->id = id;
+    t->cb_opaque = cb_opaque;
+    timer_init_full(&t->timer, NULL, QEMU_CLOCK_VIRTUAL,
+                    SCALE_MS, QEMU_TIMER_ATTR_EXTERNAL,
+                    net_slirp_timer_cb, t);
+    return t;
+}
+#else
 static void *net_slirp_timer_new(SlirpTimerCb cb,
                                  void *cb_opaque, void *opaque)
 {
@@ -197,6 +230,7 @@ static void *net_slirp_timer_new(SlirpTimerCb cb,
                     cb, cb_opaque);
     return t;
 }
+#endif
 
 static void net_slirp_timer_free(void *timer, void *opaque)
 {
@@ -231,7 +265,12 @@ static const SlirpCb slirp_cb = {
     .send_packet = net_slirp_send_packet,
     .guest_error = net_slirp_guest_error,
     .clock_get_ns = net_slirp_clock_get_ns,
+#if SLIRP_CHECK_VERSION(4,7,0)
+    .init_completed = net_slirp_init_completed,
+    .timer_new_opaque = net_slirp_timer_new_opaque,
+#else
     .timer_new = net_slirp_timer_new,
+#endif
     .timer_free = net_slirp_timer_free,
     .timer_mod = net_slirp_timer_mod,
     .register_poll_fd = net_slirp_register_poll_fd,
@@ -578,7 +617,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
 
     s = DO_UPCAST(SlirpState, nc, nc);
 
-    cfg.version = 3;
+    cfg.version = SLIRP_CHECK_VERSION(4,7,0) ? 4 : 3;
     cfg.restricted = restricted;
     cfg.in_enabled = ipv4;
     cfg.vnetwork = net;
-- 
2.35.1

Re: [RFC PATCH 3/4] net: slirp: add support for CFI-friendly timer API

[Samuel Thibault]

Paolo Bonzini, le mar. 12 avril 2022 14:13:36 +0200, a ecrit:
> libslirp 4.7 introduces a CFI-friendly version of the .timer_new callback.
> The new callback replaces the function pointer with an enum; invoking the
> callback is done with a new function slirp_handle_timer.
> 
> Support the new API so that CFI can be made compatible with using a system
> libslirp.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

> ---
>  net/slirp.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index b3a92d6e38..57af42299d 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -184,10 +184,43 @@ static int64_t net_slirp_clock_get_ns(void *opaque)
>      return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>  }
>  
> +typedef struct SlirpTimer SlirpTimer;
>  struct SlirpTimer {
>      QEMUTimer timer;
> +#if SLIRP_CHECK_VERSION(4,7,0)
> +    Slirp *slirp;
> +    SlirpTimerId id;
> +    void *cb_opaque;
> +#endif
> +};
> +
> +#if SLIRP_CHECK_VERSION(4,7,0)
> +static void net_slirp_init_completed(Slirp *slirp, void *opaque)
> +{
> +    SlirpState *s = opaque;
> +    s->slirp = slirp;
>  }
>  
> +static void net_slirp_timer_cb(void *opaque)
> +{
> +    SlirpTimer *t = opaque;
> +    slirp_handle_timer(t->slirp, t->id, t->cb_opaque);
> +}
> +
> +static void *net_slirp_timer_new_opaque(SlirpTimerId id,
> +                                        void *cb_opaque, void *opaque)
> +{
> +    SlirpState *s = opaque;
> +    SlirpTimer *t = g_new(SlirpTimer, 1);
> +    t->slirp = s->slirp;
> +    t->id = id;
> +    t->cb_opaque = cb_opaque;
> +    timer_init_full(&t->timer, NULL, QEMU_CLOCK_VIRTUAL,
> +                    SCALE_MS, QEMU_TIMER_ATTR_EXTERNAL,
> +                    net_slirp_timer_cb, t);
> +    return t;
> +}
> +#else
>  static void *net_slirp_timer_new(SlirpTimerCb cb,
>                                   void *cb_opaque, void *opaque)
>  {
> @@ -197,6 +230,7 @@ static void *net_slirp_timer_new(SlirpTimerCb cb,
>                      cb, cb_opaque);
>      return t;
>  }
> +#endif
>  
>  static void net_slirp_timer_free(void *timer, void *opaque)
>  {
> @@ -231,7 +265,12 @@ static const SlirpCb slirp_cb = {
>      .send_packet = net_slirp_send_packet,
>      .guest_error = net_slirp_guest_error,
>      .clock_get_ns = net_slirp_clock_get_ns,
> +#if SLIRP_CHECK_VERSION(4,7,0)
> +    .init_completed = net_slirp_init_completed,
> +    .timer_new_opaque = net_slirp_timer_new_opaque,
> +#else
>      .timer_new = net_slirp_timer_new,
> +#endif
>      .timer_free = net_slirp_timer_free,
>      .timer_mod = net_slirp_timer_mod,
>      .register_poll_fd = net_slirp_register_poll_fd,
> @@ -578,7 +617,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>  
>      s = DO_UPCAST(SlirpState, nc, nc);
>  
> -    cfg.version = 3;
> +    cfg.version = SLIRP_CHECK_VERSION(4,7,0) ? 4 : 3;
>      cfg.restricted = restricted;
>      cfg.in_enabled = ipv4;
>      cfg.vnetwork = net;
> -- 
> 2.35.1
> 
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.

[RFC PATCH 4/4] net: slirp: allow CFI with libslirp >= 4.7

[Paolo Bonzini]

slirp 4.7 introduces a new CFI-friendly timer callback that does
not pass function pointers within libslirp as callbacks for timers.
Check the version number and, if it is new enough, allow using CFI
even with a system libslirp.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/meson.build b/meson.build
index 861de93c4f..92a83580a3 100644
--- a/meson.build
+++ b/meson.build
@@ -2485,21 +2485,21 @@ if have_system
     slirp = declare_dependency(link_with: libslirp,
                                dependencies: slirp_deps,
                                include_directories: slirp_inc)
+  else
+    # slirp <4.7 is incompatible with CFI support in QEMU.  This is because
+    # it passes function pointers within libslirp as callbacks for timers.
+    # When using a system-wide shared libslirp, the type information for the
+    # callback is missing and the timer call produces a false positive with CFI.
+    #
+    # Now that slirp_opt has been defined, check if the selected slirp is compatible
+    # with control-flow integrity.
+    if get_option('cfi') and slirp.found() and slirp.version().version_compare('<4.7')
+      error('Control-Flow Integrity is not compatible with system-wide slirp.' \
+             + ' Please configure with --enable-slirp=git or upgrade to libslirp 4.7')
+    endif
   endif
 endif
 
-# For CFI, we need to compile slirp as a static library together with qemu.
-# This is because we register slirp functions as callbacks for QEMU Timers.
-# When using a system-wide shared libslirp, the type information for the
-# callback is missing and the timer call produces a false positive with CFI.
-#
-# Now that slirp_opt has been defined, check if the selected slirp is compatible
-# with control-flow integrity.
-if get_option('cfi') and slirp_opt == 'system'
-  error('Control-Flow Integrity is not compatible with system-wide slirp.' \
-         + ' Please configure with --enable-slirp=git')
-endif
-
 fdt = not_found
 if have_system
   fdt_opt = get_option('fdt')
-- 
2.35.1

Re: [RFC PATCH 4/4] net: slirp: allow CFI with libslirp >= 4.7

[Samuel Thibault]

Paolo Bonzini, le mar. 12 avril 2022 14:13:37 +0200, a ecrit:
> slirp 4.7 introduces a new CFI-friendly timer callback that does
> not pass function pointers within libslirp as callbacks for timers.
> Check the version number and, if it is new enough, allow using CFI
> even with a system libslirp.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

> ---
>  meson.build | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 861de93c4f..92a83580a3 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2485,21 +2485,21 @@ if have_system
>      slirp = declare_dependency(link_with: libslirp,
>                                 dependencies: slirp_deps,
>                                 include_directories: slirp_inc)
> +  else
> +    # slirp <4.7 is incompatible with CFI support in QEMU.  This is because
> +    # it passes function pointers within libslirp as callbacks for timers.
> +    # When using a system-wide shared libslirp, the type information for the
> +    # callback is missing and the timer call produces a false positive with CFI.
> +    #
> +    # Now that slirp_opt has been defined, check if the selected slirp is compatible
> +    # with control-flow integrity.
> +    if get_option('cfi') and slirp.found() and slirp.version().version_compare('<4.7')
> +      error('Control-Flow Integrity is not compatible with system-wide slirp.' \
> +             + ' Please configure with --enable-slirp=git or upgrade to libslirp 4.7')
> +    endif
>    endif
>  endif
>  
> -# For CFI, we need to compile slirp as a static library together with qemu.
> -# This is because we register slirp functions as callbacks for QEMU Timers.
> -# When using a system-wide shared libslirp, the type information for the
> -# callback is missing and the timer call produces a false positive with CFI.
> -#
> -# Now that slirp_opt has been defined, check if the selected slirp is compatible
> -# with control-flow integrity.
> -if get_option('cfi') and slirp_opt == 'system'
> -  error('Control-Flow Integrity is not compatible with system-wide slirp.' \
> -         + ' Please configure with --enable-slirp=git')
> -endif
> -
>  fdt = not_found
>  if have_system
>    fdt_opt = get_option('fdt')
> -- 
> 2.35.1
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.