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

[Stefano Garzarella]

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