svn commit: r192373 - head/sys/compat/linux

[Ben Kaduk]

On Tue, May 19, 2009 at 5:10 AM, Dmitry Chagin <dchagin at freebsd.org> wrote:
> Author: dchagin
> Date: Tue May 19 09:10:53 2009
> New Revision: 192373
> URL: http://svn.freebsd.org/changeset/base/192373
>
> Log:
>  Validate user-supplied arguments values.
>  Args argument is a pointer to the structure located in user space in
>  which the socketcall arguments are packed. The structure must be
>  copied to the kernel instead of direct dereferencing.
>
>  Approved by:  kib (mentor)
>  MFC after:    1 week
>
> Modified:
>  head/sys/compat/linux/linux_socket.c
>
> Modified: head/sys/compat/linux/linux_socket.c
> ==============================================================================
> --- head/sys/compat/linux/linux_socket.c        Tue May 19 05:36:10 2009        (r192372)
> +++ head/sys/compat/linux/linux_socket.c        Tue May 19 09:10:53 2009        (r192373)
> @@ -1467,11 +1467,38 @@ linux_getsockopt(struct thread *td, stru
>        return (error);
>  }
>
> +/* Argument list sizes for linux_socketcall */
> +
> +#define LINUX_AL(x) ((x) * sizeof(l_ulong))
> +
> +static const unsigned char lxs_args[] = {
> +       LINUX_AL(0) /* unused*/,        LINUX_AL(3) /* socket */,
> +       LINUX_AL(3) /* bind */,         LINUX_AL(3) /* connect */,
> +       LINUX_AL(2) /* listen */,       LINUX_AL(3) /* accept */,
> +       LINUX_AL(3) /* getsockname */,  LINUX_AL(3) /* getpeername */,
> +       LINUX_AL(4) /* socketpair */,   LINUX_AL(4) /* send */,
> +       LINUX_AL(4) /* recv */,         LINUX_AL(6) /* sendto */,
> +       LINUX_AL(6) /* recvfrom */,     LINUX_AL(2) /* shutdown */,
> +       LINUX_AL(5) /* setsockopt */,   LINUX_AL(5) /* getsockopt */,
> +       LINUX_AL(3) /* sendmsg */,      LINUX_AL(3) /* recvmsg */
> +};
> +
> +#define        LINUX_AL_SIZE   sizeof(lxs_args) / sizeof(lxs_args[0]) - 1
> +
>  int
>  linux_socketcall(struct thread *td, struct linux_socketcall_args *args)
>  {
> -       void *arg = (void *)(intptr_t)args->args;
> +       l_ulong a[6];
> +       void *arg;
> +       int error;
> +
> +       if (args->what < LINUX_SOCKET || args->what > LINUX_AL_SIZE)
> +               return (EINVAL);
> +       error = copyin(PTRIN(args->args), a, lxs_args[args->what]);
> +       if (error)
> +               return (error);
>
> +       arg = a;
>        switch (args->what) {
>        case LINUX_SOCKET:
>                return (linux_socket(td, arg));


What factors go into deciding to do bounds-checking before the copyin versus
after the copyin?  Naively, I would be worried about the userland data changing
out from under the kernel, but I'm not terribly familiar with this area.

Thanks,

Ben Kaduk