[v2] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD)

[PATCH for-9.1 v2 03/11] libvhost-user: mask F_INFLIGHT_SHMFD if memfd is not supported

Posted by Stefano Garzarella 1 year, 10 months ago

libvhost-user will panic when receiving VHOST_USER_GET_INFLIGHT_FD
message if MFD_ALLOW_SEALING is not defined, since it's not able
to create a memfd.

VHOST_USER_GET_INFLIGHT_FD is used only if
VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD is negotiated. So, let's mask
that feature if the backend is not able to properly handle these
messages.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 subprojects/libvhost-user/libvhost-user.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index a11afd1960..1c361ffd51 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -1674,6 +1674,16 @@ vu_get_protocol_features_exec(VuDev *dev, VhostUserMsg *vmsg)
         features |= dev->iface->get_protocol_features(dev);
     }
 
+    /*
+     * If MFD_ALLOW_SEALING is not defined, we are not able to handle
+     * VHOST_USER_GET_INFLIGHT_FD messages, since we can't create a memfd.
+     * Those messages are used only if VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD
+     * is negotiated. A device implementation can enable it, so let's mask
+     * it to avoid a runtime panic.
+     */
+#ifndef MFD_ALLOW_SEALING
+    features &= ~(1ULL << VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD);
+#endif
     vmsg_set_reply_u64(vmsg, features);
     return true;
 }
-- 
2.44.0

Re: [PATCH for-9.1 v2 03/11] libvhost-user: mask F_INFLIGHT_SHMFD if memfd is not supported

Posted by Eric Blake 1 year, 10 months ago

On Tue, Mar 26, 2024 at 02:39:28PM +0100, Stefano Garzarella wrote:
> libvhost-user will panic when receiving VHOST_USER_GET_INFLIGHT_FD
> message if MFD_ALLOW_SEALING is not defined, since it's not able
> to create a memfd.
> 
> VHOST_USER_GET_INFLIGHT_FD is used only if
> VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD is negotiated. So, let's mask
> that feature if the backend is not able to properly handle these
> messages.
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  subprojects/libvhost-user/libvhost-user.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
> index a11afd1960..1c361ffd51 100644
> --- a/subprojects/libvhost-user/libvhost-user.c
> +++ b/subprojects/libvhost-user/libvhost-user.c
> @@ -1674,6 +1674,16 @@ vu_get_protocol_features_exec(VuDev *dev, VhostUserMsg *vmsg)
>          features |= dev->iface->get_protocol_features(dev);
>      }
>  
> +    /*
> +     * If MFD_ALLOW_SEALING is not defined, we are not able to handle
> +     * VHOST_USER_GET_INFLIGHT_FD messages, since we can't create a memfd.
> +     * Those messages are used only if VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD
> +     * is negotiated. A device implementation can enable it, so let's mask
> +     * it to avoid a runtime panic.
> +     */
> +#ifndef MFD_ALLOW_SEALING
> +    features &= ~(1ULL << VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD);
> +#endif

Masking the feature out of advertisement is obviously correct. But
should we also fix the code for handling
VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD to return an error to any client
that requests it in error when the feature was not advertised, instead
of panicking?

>      vmsg_set_reply_u64(vmsg, features);
>      return true;
>  }
> -- 
> 2.44.0
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH for-9.1 v2 03/11] libvhost-user: mask F_INFLIGHT_SHMFD if memfd is not supported

Posted by Stefano Garzarella 1 year, 10 months ago

On Tue, Mar 26, 2024 at 09:36:54AM -0500, Eric Blake wrote:
>On Tue, Mar 26, 2024 at 02:39:28PM +0100, Stefano Garzarella wrote:
>> libvhost-user will panic when receiving VHOST_USER_GET_INFLIGHT_FD
>> message if MFD_ALLOW_SEALING is not defined, since it's not able
>> to create a memfd.
>>
>> VHOST_USER_GET_INFLIGHT_FD is used only if
>> VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD is negotiated. So, let's mask
>> that feature if the backend is not able to properly handle these
>> messages.
>>
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> ---
>>  subprojects/libvhost-user/libvhost-user.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
>> index a11afd1960..1c361ffd51 100644
>> --- a/subprojects/libvhost-user/libvhost-user.c
>> +++ b/subprojects/libvhost-user/libvhost-user.c
>> @@ -1674,6 +1674,16 @@ vu_get_protocol_features_exec(VuDev *dev, VhostUserMsg *vmsg)
>>          features |= dev->iface->get_protocol_features(dev);
>>      }
>>
>> +    /*
>> +     * If MFD_ALLOW_SEALING is not defined, we are not able to handle
>> +     * VHOST_USER_GET_INFLIGHT_FD messages, since we can't create a memfd.
>> +     * Those messages are used only if VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD
>> +     * is negotiated. A device implementation can enable it, so let's mask
>> +     * it to avoid a runtime panic.
>> +     */
>> +#ifndef MFD_ALLOW_SEALING
>> +    features &= ~(1ULL << VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD);
>> +#endif
>
>Masking the feature out of advertisement is obviously correct. But
>should we also fix the code for handling
>VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD to return an error to any client
>that requests it in error when the feature was not advertised, instead
>of panicking?

Totally agree!

Do I send a separate patch from this series or include it in this
series?
I would do the former because this one is already long enough.

Thanks,
Stefano