On 6 July 2013 15:17, Alexander Graf <ag...@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 <ag...@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 >