On 05/03/21 17:52, Daniele Buono wrote:
diff --git a/net/slirp.c b/net/slirp.c index be914c0be0..82e05d2c01 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -174,23 +174,42 @@ static int64_t net_slirp_clock_get_ns(void *opaque) return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); }+typedef struct SlirpTimer { + QEMUTimer t; + SlirpTimerCb cb; + void *cb_opaque; +} SlirpTimer; + +static void slirp_timer_cb(void *opaque) +{ + SlirpTimer *st = opaque; + st->cb(st->cb_opaque); +}This call is still violating CFI (st->cb is a pointer in libslirp), but now that we have a specific callback for slirp, we can easily add a "QEMU_DISABLE_CFI" decorator to disable CFI only on `slirp_timer_cb`.
Ok, so let's go with your patch for now. Independently we could change libslirp to:
- add a separate callback (timer_new_v2?) in SlirpCb. If it is set, we use it and pass an enum instead of the SlirpTimerCb cb.
- add a function slirp_timer_expired(enum SlirpTimerId timer_id, void *cb_opaque) that does the indirection but without passing around an arbitrary function pointer.
In 6.1 we will update the internal libslirp to a version that supports the new API, through a patch very similar to mine above. By requiring that new version as the minimum supported one, enabling CFI will be possible even with system slirp.
You can open a slirp merge request at https://gitlab.freedesktop.org/slirp/libslirp.
The problem is that an attack on the timer is as easy now as it was without CFI. You just have to change the timer entry to call slirp_timer_cb, and the opaque pointer to a SlirpTimer struct you created, anywhere in memory, with a pointer to your own function and your own parameters. The call to slirp_timer_cb is valid for CFI, while the call to st->cb is not checked anymore.
I understand better now the situation, thanks for taking the time to explain it.
Paolo
