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

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

[Paolo Bonzini]

Up until now, a CFI-enabled build would require slirp to be compiled as
a static library, using the version that is bundled together with QEMU.
This is because slirp registers functions as callbacks for QEMU Timers
and, when using a system-wide shared libslirp, the type information for
the callback is missing.  Thus, the timer call produces a false positive
with CFI.

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.

Supersedes: <20220412121337.207203-1-pbonzini@redhat.com>
Paolo

Paolo Bonzini (5):
  slirp: bump submodule to 4.7 release
  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 | 31 ++++++++++---------
 net/slirp.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++-------
 slirp       |  2 +-
 3 files changed, 92 insertions(+), 26 deletions(-)

-- 
2.35.1

[PATCH 1/5] slirp: bump submodule to 4.7 release

[Paolo Bonzini]

Version 4.7 of slirp provides a new timer API that works better with CFI,
together with several other improvements:

* Allow disabling the internal DHCP server !22
* Support Unix sockets in hostfwd !103
* IPv6 DNS proxying support !110
* bootp: add support for UEFI HTTP boot !111

and bugfixes.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 slirp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/slirp b/slirp
index a88d9ace23..3ad1710a96 160000
--- a/slirp
+++ b/slirp
@@ -1 +1 @@
-Subproject commit a88d9ace234a24ce1c17189642ef9104799425e0
+Subproject commit 3ad1710a96678fe79066b1469cead4058713a1d9
-- 
2.35.1

Re: [PATCH 1/5] slirp: bump submodule to 4.7 release

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 896 bytes --]

On Thu, May 5, 2022 at 1:06 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> Version 4.7 of slirp provides a new timer API that works better with CFI,
> together with several other improvements:
>
> * Allow disabling the internal DHCP server !22
> * Support Unix sockets in hostfwd !103
> * IPv6 DNS proxying support !110
> * bootp: add support for UEFI HTTP boot !111
>
> and bugfixes.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  slirp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/slirp b/slirp
> index a88d9ace23..3ad1710a96 160000
> --- a/slirp
> +++ b/slirp
> @@ -1 +1 @@
> -Subproject commit a88d9ace234a24ce1c17189642ef9104799425e0
> +Subproject commit 3ad1710a96678fe79066b1469cead4058713a1d9
> --
> 2.35.1
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 1609 bytes --]

Re: [PATCH 1/5] slirp: bump submodule to 4.7 release

[Paolo Bonzini]

On Thu, May 5, 2022 at 10:21 AM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
>
>
> On Thu, May 5, 2022 at 1:06 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> Version 4.7 of slirp provides a new timer API that works better with CFI,
>> together with several other improvements:
>>
>> * Allow disabling the internal DHCP server !22
>> * Support Unix sockets in hostfwd !103
>> * IPv6 DNS proxying support !110
>> * bootp: add support for UEFI HTTP boot !111
>>
>> and bugfixes.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Unfortunately this fails to build on Windows due to a libslirp bug
(https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/121).
We could include the couple extra commits in QEMU, since there are no
other changes to libslirp since 4.7.0 and they only fix a compiler
warning; or wait for 4.7.1 to be released; or resuscitate the patches
that switch libslirp to a subproject, but that's my least favorite
because we're going to remove the bundled source code anyway sooner or
later.

Paolo

Re: [PATCH 1/5] slirp: bump submodule to 4.7 release

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1468 bytes --]

Hi

On Thu, May 5, 2022 at 12:34 PM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On Thu, May 5, 2022 at 10:21 AM Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> >
> >
> >
> > On Thu, May 5, 2022 at 1:06 AM Paolo Bonzini <pbonzini@redhat.com>
> wrote:
> >>
> >> Version 4.7 of slirp provides a new timer API that works better with
> CFI,
> >> together with several other improvements:
> >>
> >> * Allow disabling the internal DHCP server !22
> >> * Support Unix sockets in hostfwd !103
> >> * IPv6 DNS proxying support !110
> >> * bootp: add support for UEFI HTTP boot !111
> >>
> >> and bugfixes.
> >>
> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> >
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Unfortunately this fails to build on Windows due to a libslirp bug
> (https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/121).
> We could include the couple extra commits in QEMU, since there are no
> other changes to libslirp since 4.7.0 and they only fix a compiler
> warning; or wait for 4.7.1 to be released; or resuscitate the patches
> that switch libslirp to a subproject, but that's my least favorite
> because we're going to remove the bundled source code anyway sooner or
> later.
>

Sad the CI didn't catch it. What is missing to cover it?

To me it's fine to update the submodule to the fixed commit, with a comment
explaining why.

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 2397 bytes --]

Re: [PATCH 1/5] slirp: bump submodule to 4.7 release

[Paolo Bonzini]

On Thu, May 5, 2022 at 10:38 AM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
> Sad the CI didn't catch it. What is missing to cover it?

The QEMU CI did, but the slirp one didn't because -Wmissing-prototypes
is not included in -Wall.

> To me it's fine to update the submodule to the fixed commit, with a comment explaining why.

Ok, thanks.

[PATCH 2/5] 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: [PATCH 2/5] net: slirp: introduce a wrapper struct for QemuTimer

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1920 bytes --]

On Thu, May 5, 2022 at 1:02 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> 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: Marc-André Lureau <marcandre.lureau@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
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 2846 bytes --]

[PATCH 3/5] 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: [PATCH 3/5] net: slirp: switch to slirp_new

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 2182 bytes --]

Hi

On Thu, May 5, 2022 at 1:07 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

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

It was introduced in v4.1.0, but I don't see a minimum libslirp version
check.


>
> 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
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 3050 bytes --]

Re: [PATCH 3/5] net: slirp: switch to slirp_new

[Paolo Bonzini]

On Thu, May 5, 2022 at 10:20 AM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Thu, May 5, 2022 at 1:07 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> Replace slirp_init with slirp_new, so that a more recent cfg.version
>> can be specified.
>
>
> It was introduced in v4.1.0, but I don't see a minimum libslirp version check.

In fact 4.3.0 is needed for version 3 of the struct, so the following
needs to be
squashed:

diff --git a/meson.build b/meson.build
index 11876f5141..c902dfabd3 100644
--- a/meson.build
+++ b/meson.build
@@ -2638,7 +2638,7 @@ if have_system
   if slirp_opt in ['enabled', 'auto', 'system']
     have_internal = fs.exists(meson.current_source_dir() / 'slirp/meson.build')
     slirp = dependency('slirp', kwargs: static_kwargs,
-                       method: 'pkg-config',
+                       method: 'pkg-config', version: '>=4.3.0',
                        required: slirp_opt == 'system' or
                                  slirp_opt == 'enabled' and not have_internal)
     if slirp.found()


Paolo


>
>>
>>
>> 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
>>
>>
>
>
> --
> Marc-André Lureau

Re: [PATCH 3/5] net: slirp: switch to slirp_new

[Daniel P. Berrangé]

On Thu, May 05, 2022 at 10:29:07AM +0200, Paolo Bonzini wrote:
> On Thu, May 5, 2022 at 10:20 AM Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> >
> > Hi
> >
> > On Thu, May 5, 2022 at 1:07 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >>
> >> Replace slirp_init with slirp_new, so that a more recent cfg.version
> >> can be specified.
> >
> >
> > It was introduced in v4.1.0, but I don't see a minimum libslirp version check.
> 
> In fact 4.3.0 is needed for version 3 of the struct, so the following
> needs to be
> squashed:
> 
> diff --git a/meson.build b/meson.build
> index 11876f5141..c902dfabd3 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2638,7 +2638,7 @@ if have_system
>    if slirp_opt in ['enabled', 'auto', 'system']
>      have_internal = fs.exists(meson.current_source_dir() / 'slirp/meson.build')
>      slirp = dependency('slirp', kwargs: static_kwargs,
> -                       method: 'pkg-config',
> +                       method: 'pkg-config', version: '>=4.3.0',

Adding a dep on version 4.3.0 is not desirable. Ubuntu 20.04 has
version 4.1.0, and we want to be able to drop slirp submodule
when we drop Ubuntu 18.04 (hopfully in this dev cycle).


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 3/5] net: slirp: switch to slirp_new

[Paolo Bonzini]

On Thu, May 5, 2022 at 12:07 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > It was introduced in v4.1.0, but I don't see a minimum libslirp version check.
> >
> > In fact 4.3.0 is needed for version 3 of the struct
>
> Adding a dep on version 4.3.0 is not desirable. Ubuntu 20.04 has
> version 4.1.0, and we want to be able to drop slirp submodule
> when we drop Ubuntu 18.04 (hopfully in this dev cycle).

Ok; slirp_init's functionality corresponds to version 1, so I'll use either
version 1 of the struct or 4.

Paolo

[PATCH 4/5] 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: [PATCH 4/5] net: slirp: add support for CFI-friendly timer API

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 3228 bytes --]

On Thu, May 5, 2022 at 1:01 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> 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: Marc-André Lureau <marcandre.lureau@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
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 4266 bytes --]

[PATCH 5/5] 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 | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/meson.build b/meson.build
index c26aa442d4..defe604065 100644
--- a/meson.build
+++ b/meson.build
@@ -2509,10 +2509,25 @@ if have_system
   slirp_opt = get_option('slirp')
   if slirp_opt in ['enabled', 'auto', 'system']
     have_internal = fs.exists(meson.current_source_dir() / 'slirp/meson.build')
+    slirp_dep_required = (slirp_opt == 'system' or
+                          slirp_opt == 'enabled' and not have_internal)
     slirp = dependency('slirp', kwargs: static_kwargs,
                        method: 'pkg-config',
-                       required: slirp_opt == 'system' or
-                                 slirp_opt == 'enabled' and not have_internal)
+                       required: slirp_dep_required)
+    # 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.
+    # Do not use the "version" keyword argument to produce a better error.
+    # with control-flow integrity.
+    if get_option('cfi') and slirp.found() and slirp.version().version_compare('<4.7')
+      if slirp_dep_required
+        error('Control-Flow Integrity requires libslirp 4.7.')
+      else
+        warning('Control-Flow Integrity requires libslirp 4.7, not using system-wide libslirp.')
+        slirp = not_found
+      endif
+    endif
     if slirp.found()
       slirp_opt = 'system'
     elif have_internal
@@ -2585,18 +2600,6 @@ if have_system
   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: [PATCH 5/5] net: slirp: allow CFI with libslirp >= 4.7

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 3120 bytes --]

Hi

On Thu, May 5, 2022 at 1:04 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> 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: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  meson.build | 31 +++++++++++++++++--------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index c26aa442d4..defe604065 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2509,10 +2509,25 @@ if have_system
>    slirp_opt = get_option('slirp')
>    if slirp_opt in ['enabled', 'auto', 'system']
>      have_internal = fs.exists(meson.current_source_dir() /
> 'slirp/meson.build')
> +    slirp_dep_required = (slirp_opt == 'system' or
> +                          slirp_opt == 'enabled' and not have_internal)
>      slirp = dependency('slirp', kwargs: static_kwargs,
>                         method: 'pkg-config',
> -                       required: slirp_opt == 'system' or
> -                                 slirp_opt == 'enabled' and not
> have_internal)
> +                       required: slirp_dep_required)
> +    # 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.
> +    # Do not use the "version" keyword argument to produce a better error.
> +    # with control-flow integrity.
> +    if get_option('cfi') and slirp.found() and
> slirp.version().version_compare('<4.7')
> +      if slirp_dep_required
> +        error('Control-Flow Integrity requires libslirp 4.7.')
> +      else
> +        warning('Control-Flow Integrity requires libslirp 4.7, not using
> system-wide libslirp.')
> +        slirp = not_found
> +      endif
> +    endif
>      if slirp.found()
>        slirp_opt = 'system'
>      elif have_internal
> @@ -2585,18 +2600,6 @@ if have_system
>    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
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 4193 bytes --]