[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/9] linux-user: Clean up sendrecvmsg message pa
From: |
Riku Voipio |
Subject: |
Re: [Qemu-devel] [PATCH 4/9] linux-user: Clean up sendrecvmsg message parsing |
Date: |
Tue, 23 Jul 2013 10:31:02 +0300 |
On 6 July 2013 15:17, Alexander Graf <address@hidden> 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 <address@hidden>
> ---
> 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
>
- [Qemu-devel] [PATCH 7/9] linux-user: Enable NPTL for i386, (continued)
- [Qemu-devel] [PATCH 7/9] linux-user: Enable NPTL for i386, Alexander Graf, 2013/07/06
- [Qemu-devel] [PATCH 6/9] linux-user: Add i386 TLS setter, Alexander Graf, 2013/07/06
- [Qemu-devel] [PATCH 3/9] linux-user: Reset copied CPUs in cpu_copy() always, Alexander Graf, 2013/07/06
- [Qemu-devel] [PATCH 2/9] user-exec.c: Set is_write correctly in the ARM cpu_signal_handler(), Alexander Graf, 2013/07/06
- [Qemu-devel] [PATCH 8/9] linux-user: Default to 64k guest base, Alexander Graf, 2013/07/06
- [Qemu-devel] [PATCH 1/9] linux-user: fix segmentation fault passing with h2g(x) != x, Alexander Graf, 2013/07/06
- [Qemu-devel] [PATCH 5/9] linux-user: Fix epoll on ARM hosts, Alexander Graf, 2013/07/06
- [Qemu-devel] [PATCH 9/9] linux-user: Unlock mmap_lock when resuming guest from page_unprotect, Alexander Graf, 2013/07/06
- [Qemu-devel] [PATCH 4/9] linux-user: Clean up sendrecvmsg message parsing, Alexander Graf, 2013/07/06
- Re: [Qemu-devel] [PATCH 4/9] linux-user: Clean up sendrecvmsg message parsing,
Riku Voipio <=