[vlc-devel] [PATCH v2 15/18] compat: Add custom implementation of recvmsg/sendmsg on NaCl

Filip Roséen filip at atch.se
Mon Mar 13 13:22:59 CET 2017


Hi Dennis,

As code within VLC should be able to assume that `recvmsg` and
`sendmsg`are *POSIX*-compliant, I vote that some adjustments should be
made to these implementations so that we do not run into issues with
unexpected behavior in the future.

See the below resource for the specification in full:

 - http://pubs.opengroup.org/onlinepubs/9699919799/functions/recvmsg.html

On 2017-03-13 12:37, Dennis Hamester wrote:

> From: Dennis Hamester <dennis.hamester at startmail.com>
> 
>  compat/recvmsg.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  compat/sendmsg.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  configure.ac     |  2 ++
>  3 files changed, 101 insertions(+), 2 deletions(-)
> 
> diff --git a/compat/recvmsg.c b/compat/recvmsg.c
> index 941249fef7..40ec935fc6 100644
> --- a/compat/recvmsg.c
> +++ b/compat/recvmsg.c
> @@ -1,7 +1,10 @@
>  /*****************************************************************************
>   * recvmsg.c: POSIX recvmsg() replacement
>   *****************************************************************************
> - * Copyright © 2016 Rémi Denis-Courmont
> + * Copyright © 2016-2017 VLC authors and VideoLAN
> + *
> + * Authors: Rémi Denis-Courmont
> + *          Dennis Hamester <dhamester at jusst.de>
>   *
>   * This program is free software; you can redistribute it and/or modify it
>   * under the terms of the GNU Lesser General Public License as published by
> @@ -79,3 +82,49 @@ ssize_t recvmsg(int fd, struct msghdr *msg, int flags)
>      return -1;
>  }
>  #endif
> +
> +#ifdef __native_client__
> +#include <errno.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/socket.h>
> +
> +ssize_t recvmsg(int fd, struct msghdr *msg, int flags)
> +{
> +    if (msg->msg_controllen != 0)
> +    {
> +        errno = ENOSYS;
> +        return -1;
> +    }
> +
> +    if (msg->msg_iovlen > IOV_MAX)
> +    {

This check should include `msg->msg_iovlen <= 0` together with `>
IOV_MAX`, and `errno` should be set to `EMSGSIZE` according to
POSIX.

> +        errno = EINVAL;
> +        return -1;
> +    }
> +
> +    size_t full_size = 0;
> +    for (int i = 0; i < msg->msg_iovlen; ++i)
> +        full_size += msg->msg_iov[i].iov_len;

According to *POSIX*, if the total sum of `msg->msg_iov[i].iov_len`
would overflow a `ssize_t` the function shall return `-1` and set
`errno` to `EINVAL`.
> +
> +    char *data = malloc(full_size);
> +    if (!data)

`errno = ENOMEM` on allocation failure, also note that if `full_size
== 0` it is *implementation-defined* whether `malloc` will return
`NULL` or not.

> +        return -1;
> +
> +    ssize_t res;
> +    if (msg->msg_name)
> +        res = recvfrom(fd, data, full_size, flags, msg->msg_name, &msg->msg_namelen);
> +    else
> +        res = recv(fd, data, full_size, flags);
> +

If less than `full_size` bytes of data is read you will `memcpy`
garbage in the loop that follows, which is undesirable.

> +    size_t tmp = 0;
> +    for (int i = 0; i < msg->msg_iovlen; ++i) {
> +        memcpy(msg->msg_iov[i].iov_base, data + tmp, msg->msg_iov[i].iov_len);
> +        tmp += msg->msg_iov[i].iov_len;
> +    }
> +
> +    msg->msg_flags = 0;
> +    free(data);
> +    return res;
> +}
> +#endif
> diff --git a/compat/sendmsg.c b/compat/sendmsg.c
> index 451ba298bf..5bbe8c413b 100644
> --- a/compat/sendmsg.c
> +++ b/compat/sendmsg.c
> @@ -1,7 +1,10 @@
>  /*****************************************************************************
>   * sendmsg.c: POSIX sendmsg() replacement
>   *****************************************************************************
> - * Copyright © 2016 Rémi Denis-Courmont
> + * Copyright © 2016-2017 VLC authors and VideoLAN
> + *
> + * Authors: Rémi Denis-Courmont
> + *          Dennis Hamester <dhamester at jusst.de>
>   *
>   * This program is free software; you can redistribute it and/or modify it
>   * under the terms of the GNU Lesser General Public License as published by
> @@ -69,3 +72,48 @@ ssize_t sendmsg(int fd, const struct msghdr *msg, int flags)
>      return -1;
>  }
>  #endif
> +
> +#ifdef __native_client__
> +#include <errno.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/socket.h>
> +
> +ssize_t sendmsg(int fd, const struct msghdr *msg, int flags)
> +{
> +    if (msg->msg_controllen != 0)
> +    {
> +        errno = ENOSYS;
> +        return -1;
> +    }
> +
> +    if (msg->msg_iovlen > IOV_MAX)
> +    {
> +        errno = EINVAL;
> +        return -1;
> +    }

See comments regarding `recvmsg`.

> +    size_t full_size = 0;
> +    for (int i = 0; i < msg->msg_iovlen; ++i)
> +        full_size += msg->msg_iov[i].iov_len;
> +
> +    char *data = malloc(full_size);
> +    if (!data)
> +        return -1;
> +
> +    size_t tmp = 0;
> +    for (int i = 0; i < msg->msg_iovlen; ++i) {
> +        memcpy(data + tmp, msg->msg_iov[i].iov_base, msg->msg_iov[i].iov_len);
> +        tmp += msg->msg_iov[i].iov_len;
> +    }
> +
> +    ssize_t res;
> +    if (msg->msg_name)
> +        res = sendto(fd, data, full_size, flags, msg->msg_name, msg->msg_namelen);
> +    else
> +        res = send(fd, data, full_size, flags);
> +
> +    free(data);
> +    return res;
> +}
> +#endif
> diff --git a/configure.ac b/configure.ac
> index c9713970fd..787421810e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -300,6 +300,8 @@ case "${host_os}" in
>    *nacl*)
>      SYS=nacl
>      AC_DEFINE([_XOPEN_SOURCE], [700], [POSIX and XPG 7th edition])
> +    AC_LIBOBJ([recvmsg])
> +    AC_LIBOBJ([sendmsg])
>      ;;
>    *)
>      SYS="${host_os}"
> -- 
> 2.12.0
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170313/e10addc3/attachment.html>


More information about the vlc-devel mailing list