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

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.

> @@ -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>