tech-net archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Redoing CMSG_ALIGN to avoid portability issues with !GCC compilers



Hi all,
attached are two patches to redo the way control messages are handled.
The first part restores the original i386 ABI on amd64 and essentially
ensures that the same control messages are used by netbsd32 and native
32bit code. It also makes sure that the sysctl hw.alignbytes is the same
in that situation.

The second part drops the dynamic alignment part. Typical usage in
applications is something like

        union {
                struct cmsghdr hdr;
                char buf[CMSG_SPACE(sizeof(int))];
        } cmsgbuf;

(example from tmux).

This magically works as gcc allows types with a variable component
(internally CMSG_SPACE expands to calls to __cmsg_alignbytes()). This is
extremely nasty and not supported by pretty much any other compiler.
Side effect is that the patch avoids a bunch of function calls for ~all
code using control messages, so it saves space as well.

I'm not sure where the best place for the __ALIGNBYTES definition is.
machine/types.h pollutes the namespace, other headers are even worse.

Joerg
Index: netbsd32_socket.c
===================================================================
RCS file: /home/joerg/repo/netbsd/src/sys/compat/netbsd32/netbsd32_socket.c,v
retrieving revision 1.38
diff -u -p -r1.38 netbsd32_socket.c
--- netbsd32_socket.c   13 Jan 2012 21:02:03 -0000      1.38
+++ netbsd32_socket.c   16 Jan 2012 01:30:39 -0000
@@ -49,9 +49,111 @@ __KERNEL_RCSID(0, "$NetBSD: netbsd32_soc
 #include <compat/netbsd32/netbsd32_conv.h>
 
 /*
- * XXX Assumes that sockaddr is compatible.
- * XXX Assumes that copyout_msg_control uses identical alignment.
+ * XXX Assumes that struct sockaddr is compatible.
  */
+
+#define        CMSG32_ALIGN(n) (((n) + ALIGNBYTES32) & ~ALIGNBYTES32)
+#define        CMSG32_DATA(cmsg) \
+       ((u_char *)(void *)(cmsg) + CMSG32_ALIGN(sizeof(struct cmsghdr)))
+
+#define        CMSG32_NXTHDR(mhdr, cmsg)       \
+       (((char *)(cmsg) + CMSG32_ALIGN((cmsg)->cmsg_len) + \
+                           CMSG32_ALIGN(sizeof(struct cmsghdr)) > \
+           (((char *)(mhdr)->msg_control) + (mhdr)->msg_controllen)) ? \
+           (struct cmsghdr *)0 : \
+           (struct cmsghdr *)((char *)(cmsg) + \
+               CMSG32_ALIGN((cmsg)->cmsg_len)))
+#define        CMSG32_FIRSTHDR(mhdr) \
+       ((mhdr)->msg_controllen >= sizeof(struct cmsghdr) ? \
+        (struct cmsghdr *)(mhdr)->msg_control : \
+        (struct cmsghdr *)0)
+
+#define CMSG32_SPACE(l)        (CMSG32_ALIGN(sizeof(struct cmsghdr)) + 
CMSG32_ALIGN(l))
+#define CMSG32_LEN(l)  (CMSG32_ALIGN(sizeof(struct cmsghdr)) + (l))
+
+static int
+copyout32_msg_control_mbuf(struct lwp *l, struct msghdr *mp, int *len, struct 
mbuf *m, char **q, bool *truncated)
+{
+       struct cmsghdr *cmsg, cmsg32;
+       int i, j, error;
+
+       *truncated = false;
+       cmsg = mtod(m, struct cmsghdr *);
+       do {
+               if ((char *)cmsg == mtod(m, char *) + m->m_len)
+                       break;
+               if ((char *)cmsg > mtod(m, char *) + m->m_len - sizeof(*cmsg))
+                       return EINVAL;
+               cmsg32 = *cmsg;
+               j = cmsg->cmsg_len - CMSG_LEN(0);
+               i = cmsg32.cmsg_len = CMSG32_LEN(j);
+               if (i > *len) {
+                       mp->msg_flags |= MSG_CTRUNC;
+                       if (cmsg->cmsg_level == SOL_SOCKET
+                           && cmsg->cmsg_type == SCM_RIGHTS) {
+                               *truncated = true;
+                               return 0;
+                       }
+                       j -= i - *len;
+                       i = *len;
+               }
+
+               ktrkuser("msgcontrol", cmsg, cmsg->cmsg_len);
+               error = copyout(&cmsg32, *q, MAX(i, sizeof(cmsg32)));
+               if (error)
+                       return (error);
+               error = copyout(CMSG_DATA(cmsg), *q + CMSG32_LEN(0), MAX(0, i - 
CMSG32_LEN(0)));
+               if (error)
+                       return (error);
+               j = CMSG32_SPACE(cmsg->cmsg_len - CMSG_LEN(0));
+               if (*len >= j) {
+                       *len -= j;
+                       *q += j;
+               } else {
+                       *q += i;
+                       *len = 0;
+               }
+               cmsg = (void *)((char *)cmsg + CMSG_ALIGN(cmsg->cmsg_len));
+       } while (*len > 0);
+
+       return 0;
+}
+
+static int
+copyout32_msg_control(struct lwp *l, struct msghdr *mp, struct mbuf *control)
+{
+       int len, error = 0;
+       struct mbuf *m;
+       char *q;
+       bool truncated;
+
+       len = mp->msg_controllen;
+       if (len <= 0 || control == 0) {
+               mp->msg_controllen = 0;
+               free_control_mbuf(l, control, control);
+               return 0;
+       }
+
+       q = (char *)mp->msg_control;
+
+       for (m = control; m != NULL; m = m->m_next) {
+               error = copyout32_msg_control_mbuf(l, mp, &len, m, &q, 
&truncated);
+               if (truncated) {
+                       m = control;
+                       break;
+               }
+               if (error)
+                       break;
+               if (len <= 0)
+                       break;
+       }
+
+       free_control_mbuf(l, control, m);
+
+       mp->msg_controllen = q - (char *)mp->msg_control;
+       return error;
+}
+
 int
 netbsd32_recvmsg(struct lwp *l, const struct netbsd32_recvmsg_args *uap, 
register_t *retval)
 {
@@ -97,7 +199,7 @@ netbsd32_recvmsg(struct lwp *l, const st
                goto done;
 
        if (msg.msg_control != NULL)
-               error = copyout_msg_control(l, &msg, control);
+               error = copyout32_msg_control(l, &msg, control);
 
        if (error == 0)
                error = copyout_sockname(msg.msg_name, &msg.msg_namelen, 0,
@@ -118,6 +220,107 @@ netbsd32_recvmsg(struct lwp *l, const st
        return (error);
 }
 
+static int
+copyin32_msg_control(struct lwp *l, struct msghdr *mp)
+{
+       /*
+        * Handle cmsg if there is any.
+        */
+       struct cmsghdr *cmsg, cmsg32, *cc;
+       struct mbuf *ctl_mbuf;
+       ssize_t resid = mp->msg_controllen;
+       size_t clen, cidx = 0, cspace;
+       u_int8_t *control;
+       int error;
+
+       ctl_mbuf = m_get(M_WAIT, MT_CONTROL);
+       clen = MLEN;
+       control = mtod(ctl_mbuf, void *);
+       memset(control, 0, clen);
+
+       cc = CMSG32_FIRSTHDR(mp);
+       do {
+               error = copyin(cc, &cmsg32, sizeof(cmsg32));
+               if (error)
+                       goto failure;
+
+               /*
+                * Sanity check the control message length.
+                */
+               if (cmsg32.cmsg_len > resid ||
+                   cmsg32.cmsg_len < sizeof(cmsg32)) {
+                       error = EINVAL;
+                       goto failure;
+               }
+
+               cspace = CMSG_SPACE(cmsg32.cmsg_len - CMSG32_LEN(0));
+
+               /* Check the buffer is big enough */
+               if (__predict_false(cidx + cspace > clen)) {
+                       u_int8_t *nc;
+                       size_t nclen;
+
+                       nclen = cidx + cspace;
+                       if (nclen >= PAGE_SIZE) {
+                               error = EINVAL;
+                               goto failure;
+                       }
+                       nc = realloc(clen <= MLEN ? NULL : control,
+                                    nclen, M_TEMP, M_WAITOK);
+                       if (!nc) {
+                               error = ENOMEM;
+                               goto failure;
+                       }
+                       if (cidx <= MLEN) {
+                               /* Old buffer was in mbuf... */
+                               memcpy(nc, control, cidx);
+                               memset(nc + cidx, 0, nclen - cidx);
+                       } else {
+                               memset(nc + nclen, 0, nclen - clen);
+                       }
+                       control = nc;
+                       clen = nclen;
+               }
+
+               /* Copy header */
+               cmsg = (void *)&control[cidx];
+               cmsg->cmsg_len = CMSG_LEN(cmsg32.cmsg_len - CMSG32_LEN(0));
+               cmsg->cmsg_level = cmsg32.cmsg_level;
+               cmsg->cmsg_type = cmsg32.cmsg_type;
+
+               /* Copyin the data */
+               error = copyin(CMSG32_DATA(cc), CMSG_DATA(cmsg),
+                   cmsg32.cmsg_len - CMSG32_LEN(0));
+               if (error)
+                       goto failure;
+
+               resid -= CMSG32_ALIGN(cmsg32.cmsg_len);
+               cidx += cmsg->cmsg_len;
+       } while ((cc = CMSG32_NXTHDR(mp, cc)) && resid > 0);
+
+       /* If we allocated a buffer, attach to mbuf */
+       if (cidx > MLEN) {
+               MEXTADD(ctl_mbuf, control, clen, M_MBUF, NULL, NULL);
+               ctl_mbuf->m_flags |= M_EXT_RW;
+       }
+       control = NULL;
+       mp->msg_controllen = ctl_mbuf->m_len = CMSG_ALIGN(cidx);
+
+       mp->msg_control = ctl_mbuf;
+       mp->msg_flags |= MSG_CONTROLMBUF;
+
+       ktrkuser("msgcontrol", mtod(ctl_mbuf, void *),
+           mp->msg_controllen);
+
+       return 0;
+
+failure:
+       if (control != mtod(ctl_mbuf, void *))
+               free(control, M_MBUF);
+       m_free(ctl_mbuf);
+       return error;
+}
+
 int
 netbsd32_sendmsg(struct lwp *l, const struct netbsd32_sendmsg_args *uap, 
register_t *retval)
 {
@@ -137,6 +340,16 @@ netbsd32_sendmsg(struct lwp *l, const st
        if (error)
                return (error);
        netbsd32_to_msghdr(&msg32, &msg);
+       msg.msg_flags = 0;
+
+       if (CMSG32_FIRSTHDR(&msg)) {
+               error = copyin32_msg_control(l, &msg);
+               if (error)
+                       return (error);
+       } else {
+               msg.msg_control = NULL;
+               msg.msg_controllen = 0;
+       }
 
        iovsz = msg.msg_iovlen * sizeof(struct iovec);
        if ((u_int)msg.msg_iovlen > UIO_SMALLIOV) {
@@ -151,10 +364,7 @@ netbsd32_sendmsg(struct lwp *l, const st
        if (error)
                goto done;
        msg.msg_iov = iov;
-       msg.msg_flags = 0;
 
-       /* Luckily we can use this directly */
-       /* XXX: dsl (June'07) The cmsg alignment rules differ ! */
        error = do_sys_sendmsg(l, SCARG(uap, s), &msg, SCARG(uap, flags), 
retval);
 done:
        if (iov != aiov)
Index: netbsd32_sysctl.c
===================================================================
RCS file: /home/joerg/repo/netbsd/src/sys/compat/netbsd32/netbsd32_sysctl.c,v
retrieving revision 1.32
diff -u -p -r1.32 netbsd32_sysctl.c
--- netbsd32_sysctl.c   23 May 2011 21:34:01 -0000      1.32
+++ netbsd32_sysctl.c   15 Jan 2012 23:28:18 -0000
@@ -141,6 +141,11 @@ netbsd32_sysctl_init(void)
                       NULL, 0, NULL, 0,
                       CTL_HW, CTL_EOL);
        sysctl_createv(&netbsd32_clog, 0, &_root, NULL,
+                      CTLFLAG_PERMANENT|CTLFLAG_IMMEDIATE,
+                      CTLTYPE_INT, "alignbytes", NULL,
+                      NULL, ALIGNBYTES32, NULL, 0,
+                      CTL_HW, HW_ALIGNBYTES, CTL_EOL);
+       sysctl_createv(&netbsd32_clog, 0, &_root, NULL,
                       CTLFLAG_PERMANENT,
                       CTLTYPE_STRING, "machine", NULL,
                       NULL, 0, __UNCONST(&machine32), 0,
Index: common/lib/libc/net/__cmsg_alignbytes.c
===================================================================
RCS file: /home/joerg/repo/netbsd/src/common/lib/libc/net/__cmsg_alignbytes.c,v
retrieving revision 1.3
diff -u -p -r1.3 __cmsg_alignbytes.c
--- common/lib/libc/net/__cmsg_alignbytes.c     16 Mar 2009 05:59:21 -0000      
1.3
+++ common/lib/libc/net/__cmsg_alignbytes.c     16 Jan 2012 20:17:57 -0000
@@ -46,6 +46,8 @@ __RCSID("$NetBSD: __cmsg_alignbytes.c,v 
 #include <sys/socket.h>
 #endif
 
+int __cmsg_alignbytes(void);
+
 int
 __cmsg_alignbytes(void)
 {
Index: sys/arch/amd64/include/cdefs.h
===================================================================
RCS file: /home/joerg/repo/netbsd/src/sys/arch/amd64/include/cdefs.h,v
retrieving revision 1.2
diff -u -p -r1.2 cdefs.h
--- sys/arch/amd64/include/cdefs.h      26 Oct 2008 00:08:15 -0000      1.2
+++ sys/arch/amd64/include/cdefs.h      16 Jan 2012 20:28:22 -0000
@@ -3,6 +3,6 @@
 #ifndef        _X86_64_CDEFS_H_
 #define        _X86_64_CDEFS_H_
 
-/* No arch-specific cdefs. */
+#define __ALIGNBYTES           (sizeof(long) - 1)
 
 #endif /* !_X86_64_CDEFS_H_ */
Index: sys/arch/amd64/include/param.h
===================================================================
RCS file: /home/joerg/repo/netbsd/src/sys/arch/amd64/include/param.h,v
retrieving revision 1.14
diff -u -p -r1.14 param.h
--- sys/arch/amd64/include/param.h      26 Jul 2011 12:55:35 -0000      1.14
+++ sys/arch/amd64/include/param.h      16 Jan 2012 20:28:43 -0000
@@ -23,7 +23,7 @@
  * (within reasonable limits). 
  *
  */
-#define ALIGNBYTES             (sizeof(long) - 1)
+#define ALIGNBYTES             __ALIGNBYTES
 #define ALIGN(p)               (((u_long)(p) + ALIGNBYTES) &~ALIGNBYTES)
 #define ALIGNED_POINTER(p,t)   1
 
Index: sys/arch/i386/include/cdefs.h
===================================================================
RCS file: /home/joerg/repo/netbsd/src/sys/arch/i386/include/cdefs.h,v
retrieving revision 1.8
diff -u -p -r1.8 cdefs.h
--- sys/arch/i386/include/cdefs.h       16 Jun 2011 13:27:59 -0000      1.8
+++ sys/arch/i386/include/cdefs.h       16 Jan 2012 20:29:25 -0000
@@ -7,4 +7,6 @@
 #define        __compactcall   __attribute__((__regparm__(3)))
 #endif
 
+#define __ALIGNBYTES   (sizeof(int) - 1)
+
 #endif /* !_I386_CDEFS_H_ */
Index: sys/arch/i386/include/param.h
===================================================================
RCS file: /home/joerg/repo/netbsd/src/sys/arch/i386/include/param.h,v
retrieving revision 1.72
diff -u -p -r1.72 param.h
--- sys/arch/i386/include/param.h       8 Feb 2010 19:02:29 -0000       1.72
+++ sys/arch/i386/include/param.h       16 Jan 2012 20:29:08 -0000
@@ -62,7 +62,7 @@
  * (within reasonable limits). 
  *
  */
-#define ALIGNBYTES             (sizeof(int) - 1)
+#define ALIGNBYTES             __ALIGNBYTES
 #define ALIGN(p)               (((u_int)(u_long)(p) + ALIGNBYTES) &~ \
     ALIGNBYTES)
 #define ALIGNED_POINTER(p,t)   1
Index: sys/sys/socket.h
===================================================================
RCS file: /home/joerg/repo/netbsd/src/sys/sys/socket.h,v
retrieving revision 1.101
diff -u -p -r1.101 socket.h
--- sys/sys/socket.h    20 Dec 2011 23:56:29 -0000      1.101
+++ sys/sys/socket.h    16 Jan 2012 20:29:46 -0000
@@ -531,7 +531,7 @@ struct cmsghdr {
  * without (2), we can't guarantee binary compatibility in case of future
  * changes in ALIGNBYTES.
  */
-#define __CMSG_ALIGN(n)        (((n) + __cmsg_alignbytes()) & 
~__cmsg_alignbytes())
+#define __CMSG_ALIGN(n)        (((n) + __ALIGNBYTES) & ~__ALIGNBYTES)
 #ifdef _KERNEL
 #define CMSG_ALIGN(n)  __CMSG_ALIGN(n)
 #endif
@@ -574,10 +574,6 @@ struct cmsghdr {
 
 #include <sys/cdefs.h>
 
-__BEGIN_DECLS
-int    __cmsg_alignbytes(void);
-__END_DECLS
-
 #ifdef _KERNEL
 static inline socklen_t
 sockaddr_getlen(const struct sockaddr *sa)


Home | Main Index | Thread Index | Old Index