[4/9] linux-user: Clean up sendrecvmsg message parsing

The cmsg handling in the linux-user code is very hard to read as it tries
to follow glibc's unreadable code closely. Let's clean it up, converting
all helpers into static inline functions, so that QEMU developers have a
chance to understand what's going on.

While at it, this also allows us to make the next target header lookup more
obvious and GUEST_BASE safe. We only compare host mapped pointers between each
other now.

During the rewrite I also saw that we never advance our target cmsg structure
to the next one. This behavior is identical to the previous code, but looks
very bogus to me.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 linux-user/syscall.c      |   25 ++++++++++++-------
 linux-user/syscall_defs.h |   58 ++++++++++++++++++++++++++++++--------------
 2 files changed, 55 insertions(+), 28 deletions(-)

On 6 July 2013 15:17, Alexander Graf <agraf@suse.de> wrote:
> The cmsg handling in the linux-user code is very hard to read as it tries
> to follow glibc's unreadable code closely. Let's clean it up, converting
> all helpers into static inline functions, so that QEMU developers have a
> chance to understand what's going on.
>
> While at it, this also allows us to make the next target header lookup more
> obvious and GUEST_BASE safe. We only compare host mapped pointers between each
> other now.
>
> During the rewrite I also saw that we never advance our target cmsg structure
> to the next one. This behavior is identical to the previous code, but looks
> very bogus to me.

recvmsg01 and sendmsg01 both segfault (arm target and x86_64 host)
with both current linux-user code and after this patch. So there is
more to fix here. On the bright side this patch is not an regression
:)

> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  linux-user/syscall.c      |   25 ++++++++++++-------
>  linux-user/syscall_defs.h |   58 ++++++++++++++++++++++++++++++--------------
>  2 files changed, 55 insertions(+), 28 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 89b7698..8eb5c31 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1120,6 +1120,7 @@ static inline abi_long target_to_host_cmsg(struct msghdr *msgh,
>      abi_long msg_controllen;
>      abi_ulong target_cmsg_addr;
>      struct target_cmsghdr *target_cmsg;
> +    struct target_cmsghdr *first_target_cmsg;
>      socklen_t space = 0;
>
>      msg_controllen = tswapal(target_msgh->msg_controllen);
> @@ -1129,13 +1130,14 @@ static inline abi_long target_to_host_cmsg(struct msghdr *msgh,
>      target_cmsg = lock_user(VERIFY_READ, target_cmsg_addr, msg_controllen, 1);
>      if (!target_cmsg)
>          return -TARGET_EFAULT;
> +    first_target_cmsg = target_cmsg;
>
>      while (cmsg && target_cmsg) {
>          void *data = CMSG_DATA(cmsg);
> -        void *target_data = TARGET_CMSG_DATA(target_cmsg);
> +        void *target_data = target_cmsg_data(target_cmsg);
>
>          int len = tswapal(target_cmsg->cmsg_len)
> -                  - TARGET_CMSG_ALIGN(sizeof (struct target_cmsghdr));
> +                  - target_cmsg_align(sizeof (struct target_cmsghdr));
>
>          space += CMSG_SPACE(len);
>          if (space > msgh->msg_controllen) {
> @@ -1161,7 +1163,8 @@ static inline abi_long target_to_host_cmsg(struct msghdr *msgh,
>          }
>
>          cmsg = CMSG_NXTHDR(msgh, cmsg);
> -        target_cmsg = TARGET_CMSG_NXTHDR(target_msgh, target_cmsg);
> +        target_cmsg = target_cmsg_nxthdr(target_msgh, target_cmsg,
> +                                         first_target_cmsg);
>      }
>      unlock_user(target_cmsg, target_cmsg_addr, 0);
>   the_end:
> @@ -1176,6 +1179,7 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh,
>      abi_long msg_controllen;
>      abi_ulong target_cmsg_addr;
>      struct target_cmsghdr *target_cmsg;
> +    struct target_cmsghdr *first_target_cmsg;
>      socklen_t space = 0;
>
>      msg_controllen = tswapal(target_msgh->msg_controllen);
> @@ -1183,25 +1187,27 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh,
>          goto the_end;
>      target_cmsg_addr = tswapal(target_msgh->msg_control);
>      target_cmsg = lock_user(VERIFY_WRITE, target_cmsg_addr, msg_controllen, 0);
> -    if (!target_cmsg)
> +    if (!target_cmsg) {
>          return -TARGET_EFAULT;
> +    }
> +    first_target_cmsg = target_cmsg;
>
>      while (cmsg && target_cmsg) {
>          void *data = CMSG_DATA(cmsg);
> -        void *target_data = TARGET_CMSG_DATA(target_cmsg);
> +        void *target_data = target_cmsg_data(target_cmsg);
>
>          int len = cmsg->cmsg_len - CMSG_ALIGN(sizeof (struct cmsghdr));
>
> -        space += TARGET_CMSG_SPACE(len);
> +        space += target_cmsg_space(len);
>          if (space > msg_controllen) {
> -            space -= TARGET_CMSG_SPACE(len);
> +            space -= target_cmsg_space(len);
>              gemu_log("Target cmsg overflow\n");
>              break;
>          }
>
>          target_cmsg->cmsg_level = tswap32(cmsg->cmsg_level);
>          target_cmsg->cmsg_type = tswap32(cmsg->cmsg_type);
> -        target_cmsg->cmsg_len = tswapal(TARGET_CMSG_LEN(len));
> +        target_cmsg->cmsg_len = tswapal(target_cmsg_len(len));
>
>          if ((cmsg->cmsg_level == TARGET_SOL_SOCKET) &&
>                                  (cmsg->cmsg_type == SCM_RIGHTS)) {
> @@ -1228,7 +1234,8 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh,
>          }
>
>          cmsg = CMSG_NXTHDR(msgh, cmsg);
> -        target_cmsg = TARGET_CMSG_NXTHDR(target_msgh, target_cmsg);
> +        target_cmsg = target_cmsg_nxthdr(target_msgh, target_cmsg,
> +                                         first_target_cmsg);
>      }
>      unlock_user(target_cmsg, target_cmsg_addr, space);
>   the_end:
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 92c01a9..84da7f7 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -199,26 +199,46 @@ struct target_cmsghdr {
>      int          cmsg_type;
>  };
>
> -#define TARGET_CMSG_DATA(cmsg) ((unsigned char *) ((struct target_cmsghdr *) (cmsg) + 1))
> -#define TARGET_CMSG_NXTHDR(mhdr, cmsg) __target_cmsg_nxthdr (mhdr, cmsg)
> -#define TARGET_CMSG_ALIGN(len) (((len) + sizeof (abi_long) - 1) \
> -                               & (size_t) ~(sizeof (abi_long) - 1))
> -#define TARGET_CMSG_SPACE(len) (TARGET_CMSG_ALIGN (len) \
> -                               + TARGET_CMSG_ALIGN (sizeof (struct target_cmsghdr)))
> -#define TARGET_CMSG_LEN(len)   (TARGET_CMSG_ALIGN (sizeof (struct target_cmsghdr)) + (len))
> -
> -static __inline__ struct target_cmsghdr *
> -__target_cmsg_nxthdr (struct target_msghdr *__mhdr, struct target_cmsghdr *__cmsg)
> +static inline unsigned char *target_cmsg_data(struct target_cmsghdr *cmsg)
>  {
> -  struct target_cmsghdr *__ptr;
> -
> -  __ptr = (struct target_cmsghdr *)((unsigned char *) __cmsg
> -                                    + TARGET_CMSG_ALIGN (tswapal(__cmsg->cmsg_len)));
> -  if ((unsigned long)((char *)(__ptr+1) - (char *)(size_t)tswapal(__mhdr->msg_control))
> -      > tswapal(__mhdr->msg_controllen))
> -    /* No more entries.  */
> -    return (struct target_cmsghdr *)0;
> -  return __cmsg;
> +    return ((unsigned char *) ((struct target_cmsghdr *) (cmsg) + 1));
> +}
> +
> +static inline abi_ulong target_cmsg_align(abi_ulong len)
> +{
> +    return ((len) + sizeof(abi_long) - 1) & (size_t)~(sizeof(abi_long) - 1);
> +}
> +
> +static inline abi_ulong target_cmsg_len(abi_ulong len)
> +{
> +    return target_cmsg_align(sizeof (struct target_cmsghdr)) + len;
> +}
> +
> +static inline abi_ulong target_cmsg_space(abi_ulong len)
> +{
> +    return target_cmsg_len(target_cmsg_align(len));
> +}
> +
> +static inline struct target_cmsghdr *target_cmsg_nxthdr(
> +        struct target_msghdr *mhdr, struct target_cmsghdr *cmsg,
> +        struct target_cmsghdr *first_cmsg)
> +{
> +    abi_ulong len;
> +
> +    /* length of all entries since the first one */
> +    len = ((uintptr_t)first_cmsg - (uintptr_t)cmsg);
> +    /* plus length of this header */
> +    len += sizeof(struct target_cmsghdr);
> +    /* plus length of this entry's data */
> +    len += target_cmsg_align(tswapal(cmsg->cmsg_len));
> +
> +    /* no more entries */
> +    if (tswapal(mhdr->msg_controllen) < len) {
> +        return NULL;
> +    }
> +
> +    /* XXX: return this header, this looks broken */
> +    return cmsg;
>  }
>
>
> --
> 1.6.0.2
>