Re: git: 3a99aac66f8d - main - linux(4): Check the socket before any others sanity checks

On Wed, Sep 07, 2022 at 05:47:36PM +0200, Mateusz Guzik wrote:
> On 5/28/22, Dmitry Chagin <dchagin@freebsd.org> wrote:
> > The branch main has been updated by dchagin:
> >
> > URL:
> > https://cgit.FreeBSD.org/src/commit/?id=3a99aac66f8d12386e8382aaf29d2e82e6b5353b
> >
> > commit 3a99aac66f8d12386e8382aaf29d2e82e6b5353b
> > Author:     Dmitry Chagin <dchagin@FreeBSD.org>
> > AuthorDate: 2022-05-28 20:29:12 +0000
> > Commit:     Dmitry Chagin <dchagin@FreeBSD.org>
> > CommitDate: 2022-05-28 20:29:12 +0000
> >
> >     linux(4): Check the socket before any others sanity checks
> >
> >     Strictly speaking, this check is performed by the kern_recvit(), but in
> >     the Linux emulation layer before calling the kernel we do other sanity
> >     checks and conversions from Linux types to the native types. This
> > changes
> >     an order of the error returning that is critical for some buggy Linux
> >     applications.
> >
> >     For recvmmsg() syscall this fixes a panic in case when the
> > user-supplied
> >     vlen value is 0, then error is not initialized and garbage passed to
> > the
> >     bsd_to_linux_errno().
> >
> >     MFC after:              2 weeks
> > ---
> >  sys/compat/linux/linux_socket.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/sys/compat/linux/linux_socket.c
> > b/sys/compat/linux/linux_socket.c
> > index b5ec32835981..8aa425bc14c0 100644
> > --- a/sys/compat/linux/linux_socket.c
> > +++ b/sys/compat/linux/linux_socket.c
> > @@ -1731,7 +1731,14 @@ int
> >  linux_recvmsg(struct thread *td, struct linux_recvmsg_args *args)
> >  {
> >  	struct msghdr bsd_msg;
> > +	struct file *fp;
> > +	int error;
> >
> > +	error = getsock_cap(td, args->s, &cap_recv_rights,
> > +	    &fp, NULL, NULL);
> > +	if (error != 0)
> > +		return (error);
> > +	fdrop(fp, td);
> >  	return (linux_recvmsg_common(td, args->s, PTRIN(args->msg),
> >  	    args->flags, &bsd_msg));
> >  }
> 
> but linux_recvmsg_common starts with performing literally the same op,
> what's the point of this bit?
> 
> Note if it was really fixing anything it would be racy against
> malicious userspace which can replace fds in the middle.
> 

ihi, thanks, I'll look a little later, really busy preparing for the
competition 

> > @@ -1742,9 +1749,14 @@ linux_recvmmsg_common(struct thread *td, l_int s,
> > struct l_mmsghdr *msg,
> >  {
> >  	struct msghdr bsd_msg;
> >  	struct timespec ts;
> > +	struct file *fp;
> >  	l_uint retval;
> >  	int error, datagrams;
> >
> > +	error = getsock_cap(td, s, &cap_recv_rights,
> > +	    &fp, NULL, NULL);
> > +	if (error != 0)
> > +		return (error);
> >  	datagrams = 0;
> >  	while (datagrams < vlen) {
> >  		error = linux_recvmsg_common(td, s, &msg->msg_hdr,
> > @@ -1780,6 +1792,7 @@ linux_recvmmsg_common(struct thread *td, l_int s,
> > struct l_mmsghdr *msg,
> >  	}
> >  	if (error == 0)
> >  		td->td_retval[0] = datagrams;
> > +	fdrop(fp, td);
> >  	return (error);
> >  }
> >
> >
> 
> 
> -- 
> Mateusz Guzik <mjguzik gmail.com>