Re: [Qemu-devel] [PATCH] linux-user: fix cmsg conversion in case of multiple headers

[Riku Voipio]

Hi,

On Thu, Sep 03, 2015 at 07:27:26AM +0200, Jonathan Neuschäfer wrote:
> Currently, __target_cmsg_nxthdr compares a pointer derived from
> target_cmsg against the msg_control field of target_msgh (through
> subtraction).  This failed for me when emulating i386 code under x86_64,
> because pointers in the host address space and pointers in the guest
> address space were not the same.  This patch passes the initial value of
> target_cmsg into __target_cmsg_nxthdr.
> 
> I found and fixed two more related bugs:
> - __target_cmsg_nxthdr now returns the new cmsg pointer instead of the
>   old one.
> - tgt_space (in host_to_target_cmsg) doesn't count "sizeof (struct
>   target_cmsghdr)" twice anymore.

Applied to linux-user tree, thanks.

> Signed-off-by: Jonathan Neuschäfer <address@hidden>
> --

For next time, use three dashes "---" to separate version-to-version
change descriptions from commit messages. This way git will
automatically leave them out when doing git am.

> Changes since v2:
> - The patch is now clean WRT checkpatch.pl
> Changes since v1:
> - Follow Peter Maydell's advice on how to fix the first bug
> - The "two more related bugs"
> ---
>  linux-user/syscall.c      | 14 +++++++++-----
>  linux-user/syscall_defs.h | 14 +++++++++-----
>  2 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index f62c698..12a6cd2 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1181,7 +1181,7 @@ static inline abi_long target_to_host_cmsg(struct 
> msghdr *msgh,
>      struct cmsghdr *cmsg = CMSG_FIRSTHDR(msgh);
>      abi_long msg_controllen;
>      abi_ulong target_cmsg_addr;
> -    struct target_cmsghdr *target_cmsg;
> +    struct target_cmsghdr *target_cmsg, *target_cmsg_start;
>      socklen_t space = 0;
>      
>      msg_controllen = tswapal(target_msgh->msg_controllen);
> @@ -1189,6 +1189,7 @@ static inline abi_long target_to_host_cmsg(struct 
> msghdr *msgh,
>          goto the_end;
>      target_cmsg_addr = tswapal(target_msgh->msg_control);
>      target_cmsg = lock_user(VERIFY_READ, target_cmsg_addr, msg_controllen, 
> 1);
> +    target_cmsg_start = target_cmsg;
>      if (!target_cmsg)
>          return -TARGET_EFAULT;
>  
> @@ -1247,7 +1248,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,
> +                                         target_cmsg_start);
>      }
>      unlock_user(target_cmsg, target_cmsg_addr, 0);
>   the_end:
> @@ -1261,7 +1263,7 @@ static inline abi_long host_to_target_cmsg(struct 
> target_msghdr *target_msgh,
>      struct cmsghdr *cmsg = CMSG_FIRSTHDR(msgh);
>      abi_long msg_controllen;
>      abi_ulong target_cmsg_addr;
> -    struct target_cmsghdr *target_cmsg;
> +    struct target_cmsghdr *target_cmsg, *target_cmsg_start;
>      socklen_t space = 0;
>  
>      msg_controllen = tswapal(target_msgh->msg_controllen);
> @@ -1269,6 +1271,7 @@ 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);
> +    target_cmsg_start = target_cmsg;
>      if (!target_cmsg)
>          return -TARGET_EFAULT;
>  
> @@ -1382,14 +1385,15 @@ static inline abi_long host_to_target_cmsg(struct 
> target_msghdr *target_msgh,
>          }
>  
>          target_cmsg->cmsg_len = tswapal(tgt_len);
> -        tgt_space = TARGET_CMSG_SPACE(tgt_len);
> +        tgt_space = TARGET_CMSG_SPACE(len);
>          if (msg_controllen < tgt_space) {
>              tgt_space = msg_controllen;
>          }
>          msg_controllen -= tgt_space;
>          space += tgt_space;
>          cmsg = CMSG_NXTHDR(msgh, cmsg);
> -        target_cmsg = TARGET_CMSG_NXTHDR(target_msgh, target_cmsg);
> +        target_cmsg = TARGET_CMSG_NXTHDR(target_msgh, target_cmsg,
> +                                         target_cmsg_start);
>      }
>      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 edd5f3c..9d3c537 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -234,7 +234,8 @@ struct target_cmsghdr {
>  };
>  
>  #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_NXTHDR(mhdr, cmsg, cmsg_start) \
> +                               __target_cmsg_nxthdr(mhdr, cmsg, cmsg_start)
>  #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) \
> @@ -242,17 +243,20 @@ 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)
> +__target_cmsg_nxthdr(struct target_msghdr *__mhdr,
> +                     struct target_cmsghdr *__cmsg,
> +                     struct target_cmsghdr *__cmsg_start)
>  {
>    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))
> +  if ((unsigned long)((char *)(__ptr+1) - (char *)__cmsg_start)
> +      > tswapal(__mhdr->msg_controllen)) {
>      /* No more entries.  */
>      return (struct target_cmsghdr *)0;
> -  return __cmsg;
> +  }
> +  return __ptr;
>  }
>  
>  struct target_mmsghdr {
> -- 
> 2.5.0
>