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

[Alexander Graf]

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 <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