Re: [PATCH] meson: Stop if cfi is enabled with system slirp

[Paolo Bonzini <pbonzini@redhat.com>]

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