Re: [PATCH 2/2] linux-user: Allow sendmsg() without IOV

[Helge Deller]

On 1/31/23 14:34, Helge Deller wrote:
> On 1/31/23 13:28, Laurent Vivier wrote:
> > Le 12/12/2022 à 18:34, Helge Deller a écrit :
> > > Applications do call sendmsg() without any IOV, e.g.:
> > >   sendmsg(4, {msg_name=NULL, msg_namelen=0, msg_iov=NULL, msg_iovlen=0,
> > >              msg_control=[{cmsg_len=36, cmsg_level=SOL_ALG, cmsg_type=0x2}],
> > >              msg_controllen=40, msg_flags=0}, MSG_MORE) = 0
> > >   sendmsg(4, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="The quick brown fox 
> > > jumps over t"..., iov_len=183}],
> > >              msg_iovlen=1, msg_control=[{cmsg_len=20, cmsg_level=SOL_ALG, 
> > > cmsg_type=0x3}],
> > >              msg_controllen=24, msg_flags=0}, 0) = 183
> > >
> > > The function do_sendrecvmsg_locked() is used for sndmsg() and recvmsg()
> > > and calls lock_iovec() to lock the IOV into memory. For the first
> > > sendmsg() above it returns NULL and thus wrongly skips the call the host
> > > sendmsg() syscall, which will break the calling application.
> > >
> > > Fix this issue by:
> > > - allowing sendmsg() even with empty IOV
> > > - skip recvmsg() if IOV is NULL
> > > - skip both if the return code of do_sendrecvmsg_locked() != 0, which
> > >    indicates some failure like EFAULT on the IOV
> > >
> > > Tested with the debian "ell" package with hppa guest on x86_64 host.
> > >
> > > Signed-off-by: Helge Deller <deller@gmx.de>
> > > ---
> > >   linux-user/syscall.c | 9 +++++++--
> > >   1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > > index a365903a3a..9e2c0a18fc 100644
> > > --- a/linux-user/syscall.c
> > > +++ b/linux-user/syscall.c
> > > @@ -3330,7 +3330,10 @@ static abi_long do_sendrecvmsg_locked(int fd, struct 
> > > target_msghdr *msgp,
> > >                        target_vec, count, send);
> > >       if (vec == NULL) {
> > >           ret = -host_to_target_errno(errno);
> > > -        goto out2;
> > > +        /* allow sending packet without any iov, e.g. with MSG_MORE flag */
> >
> > why don't you check only for count is 0?
> > Somehing like:
> >
> > if (vec == NULL && (count || !send)) {
>
> You mean:
> if (vec == NULL && (!count || !send)) {
>      goto out2;
>
> lock_iovec() sets errno=0 if count==0, but for invalid addresses it sets 
> errno!=0,
> so my current check (for which ret is the same as errno):
>          if (!send || ret) {
>              goto out2;
> exits on memfaults too, while a check for just count==0 would succeed.

forget the above...
Maybe just checking for count==0 would be sufficient. I don't know yet if
there are any other corner cases where replacing the check of
          if (!send || ret) {
with
          if (!send || count==0) {
would behave different.
Checking against errno which is reported back from lock_iovec() seemed more
logical for me. This patch has been as-is in the buildd trees now since weeks
without any problems so far at least.

Helge