Re: [PATCH for-9.1 v2 03/11] libvhost-user: mask F_INFLIGHT_SHMFD if memfd is not supported
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 --- 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
Re: [PATCH for-9.1 v2 03/11] libvhost-user: mask F_INFLIGHT_SHMFD if memfd is not supported
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 > --- > 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
[PATCH for-9.1 v2 03/11] libvhost-user: mask F_INFLIGHT_SHMFD if memfd is not supported
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 --- 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
