Re: [PATCH for-9.1 v2 02/11] libvhost-user: fail vu_message_write() if sendmsg() is failing

[Stefano Garzarella]

On Tue, Mar 26, 2024 at 03:36:52PM +0100, David Hildenbrand wrote:
> On 26.03.24 15:34, Eric Blake wrote:
> > On Tue, Mar 26, 2024 at 02:39:27PM +0100, Stefano Garzarella wrote:
> > > In vu_message_write() we use sendmsg() to send the message header,
> > > then a write() to send the payload.
> > >
> > > If sendmsg() fails we should avoid sending the payload, since we
> > > were unable to send the header.
> > >
> > > Discovered before fixing the issue with the previous patch, where
> > > sendmsg() failed on macOS due to wrong parameters, but the frontend
> > > still sent the payload which the backend incorrectly interpreted
> > > as a wrong header.
> > >
> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > ---
> > >  subprojects/libvhost-user/libvhost-user.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/subprojects/libvhost-user/libvhost-user.c 
> > > b/subprojects/libvhost-user/libvhost-user.c
> > > index 22bea0c775..a11afd1960 100644
> > > --- a/subprojects/libvhost-user/libvhost-user.c
> > > +++ b/subprojects/libvhost-user/libvhost-user.c
> > > @@ -639,6 +639,11 @@ vu_message_write(VuDev *dev, int conn_fd, VhostUserMsg 
> > > *vmsg)
> > >          rc = sendmsg(conn_fd, &msg, 0);
> > >      } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
> > > +    if (rc <= 0) {
> >
> > Is rejecting a 0 return value correct?  Technically, a 0 return means
> > a successful write of 0 bytes - but then again, it is unwise to
> > attempt to send an fd or other auxilliary ddata without at least one
> > regular byte, so we should not be attempting a write of 0 bytes.  So I
> > guess this one is okay, although I might have used < instead of <=.

I blindly copied what they already do at the end of the same function.
However, your observation is correct. That said they have a sendmsg()
to send the header. After this we have a write() loop to send the
payload.

Now if sendmsg() returns 0, but we have not sent all the header, what
should we do? Try sendmsg() again? For how many times?

> I was wondering if we could see some partial sendmsg()/write 
> succeeding. Meaning, we transferred some bytes but not all, and we'd 
> actually need to loop ...

Yep, true, but I would fix it in another patch/series if you agree.

Thanks,
Stefano