Does FreeBSD have sendmmsg or recvmmsg system calls?

[Konstantin Belousov]

On Thu, Jan 07, 2016 at 03:29:23PM +0200, Boris Astardzhiev wrote:
> First of all thanks for the insightful code review.
> 
> >> +     struct file *fp;
> >> +     cap_rights_t rights;
> >> +
> >> +     if (fget(td, uap->s, cap_rights_init(&rights, CAP_RECV), &fp) != 0)
> >> +             return (EBADF);
> >> +
> >> +     so = fp->f_data;
> >What if uap->s filedescriptor does not reference a socket ?
> 
> kern_sendit/recvit() eventually call getsock_cap().
> To me this means that send/recv(m)msg have to be limited only to sockets.
> I was in a need to grab access to so_error due to the error handling later.
> Probably I need to use something similar like kern_sendit/recvit's
> getsock_cap approach.
I missed this check, so my comment is not applicable.

Hm, why do you need to fget() the uap->s filedescriptor at all ?
Rather, I see why would you need it, but what is done is not completely
correct.  Note that userspace may close uap->s between two calls to
kern_sendit(), or close and reopen for different kind of file,
so your check at the start of the function is mostly useless.

You should change the kern_sendit() interface to accept socket for this
race to be eliminated, or move the guts of kern_sendit() into new function
which takes socket as an argument.  Then getsock_cap() would be done
by your new syscall code and by kern_sendit() etc.

After looking closer at it, additional argument to not use kern_sendit()
is because you miss conditions like nullification of EINTR/ERESTART, which
should stop the iteration.

> Comments or a better approach to so_error?
> 
> >>> +
> >>> +     vlen = uap->vlen;
> >>> +     if (vlen > UIO_MAXIOV)
> >>> +             vlen = UIO_MAXIOV;
> >>> +
> >>> +     len = vlen * sizeof(*uap->msgvec);
> >>> +     vec = malloc(len, M_TEMP, M_WAITOK);
> >>This is a user-controlled malloc(9), i.e. an easy kernel panic due to
> >>impossible to satisfy request, or KVA and memory exhaustion.
> 
> Having a glance in the manpage there it is stated that:
>      M_WAITOK
>              Indicates that it is OK to wait for resources.  If the request
>              cannot be immediately fulfilled, the current process is put to
>              sleep to wait for resources to be released by other processes.
>              The malloc(), realloc(), and reallocf() functions cannot return
>              NULL if M_WAITOK is specified.
> I.e we'll be put to sleep and no NULL will be returned. I don't see how I'll
> get a panic here. Or you're stressing the malloc type M_TEMP here?
> More clarifications?
Suppose that malicious user code passed a value of 10 000 000 for uap->vlen.
How many bytes for kernel malloc would be consumed ?  This is local DoS.

What would happen if the requested size is bigger than the kmem size, so
that the allocation cannot succeed even theoretically ?

> >What are the reasons to go with the trivial kernel implementation
> >right now, instead of trivial and simpler libc wrapper ?  I think the
> >speed would be same, but the code _much_ simpler.
> 
> My initial idea was such that with the kernel approach we get closer to the
> driver itself and if it has mm optimizations we may have a benefit in
> future.
> And thus one day we won't need to redo the work scrubbing the libc-only
> implementation.

I suspect that the actual syscall interface would be more rich than the
direct translation of the function signature into the syscall.  IMO it
would be easier for you to get the pure libc implementation correct
for the first step.

With such changes, I alway much prefer to have the hard part prototyped
before making the bound changes in the API/ABI, which in this case means
that some protocol should be enchanced to the multi-message interface.