kern/38992: kernel requires incorrect alignment for cmsghdr cmsg_len field

[Taylor R Campbell <campbell%mumble.net@localhost>]

>Number:         38992
>Category:       kern
>Synopsis:       kernel requires incorrect alignment for cmsghdr cmsg_len field
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Jun 20 00:30:00 +0000 2008
>Originator:     Taylor R Campbell <campbell%mumble.net@localhost>
>Release:        NetBSD 4.0_STABLE
>Organization:
>Environment:
System: NetBSD slate.localdomain 4.0_STABLE NetBSD 4.0_STABLE (GENERIC) #0: Sat 
Jun 7 06:55:02 UTC 2008 
riastradh%slate.localdomain@localhost:/home/riastradh/netbsd/4/obj/sys/arch/macppc/compile/GENERIC
 macppc
Architecture: powerpc
Machine: macppc
>Description:

        Note: The machine I am using to submit this PR runs NetBSD
        4.0_STABLE, but the problem persists in -current as of a couple
        days ago.

        sendmsg(2) yields EINVAL for correctly aligned message header
        lengths, and proceeds happpily with incorrectly aligned ones.
        In a msghdr, msg_controllen should be set to the actual size of
        the control data buffer computed with CMSG_SPACE, and in a
        cmsghdr within the control data buffer, cmsg_len should be set
        to the number of bytes actually used by that header's control
        datum, including the header, computed with CMSG_LEN.
        sendmsg(2) yields EINVAL if msg_controllen and cmesg_len
        disagree, however, which is often the case -- specifically, on
        architectures whose alignment exceeds the width of a file
        descriptor, so that CMSG_SPACE (sizeof (int)) differs from
        CMSG_LEN (sizeof (int)).

>How-To-Repeat:

        Construct a msghdr with ancillary data for sending a file
        descriptor to another process with sendmsg(2) as follows:
        
          struct msghdr msg;
          union {
            struct cmsghdr align_me_please;
            char data [CMSG_SPACE (sizeof (int))];
          } control_buffer;
          struct cmsghdr *cmsg_ptr;
          char one = 1;
          struct iovec iov;

          iov.iov_base = &one;
          iov.iov_len = 1;
          msg.msg_name = NULL;
          msg.msg_namelen = 0;
          msg.msg_iov = &iov;
          msg.msg_iovlen = 1;
          msg.msg_control = (void *) &control_buffer;
          msg.msg_controllen = sizeof (control_buffer);
          cmsg_ptr = CMSG_FIRSTHDR (&msg);
          cmsg_ptr->cmsg_len = CMSG_LEN (sizeof (int));
          cmsg_ptr->cmsg_level = SOL_SOCKET;
          cmsg_ptr->cmsg_type = SCM_RIGHTS;
          *((int *) CMSG_DATA (cmsg_ptr)) = fd;

        Then pass the msghdr to sendmsg(2).  On platforms whose
        alignment is wider than the size of an int (e.g., PowerPC,
        SPARC, &c.), sendmsg(2) will fail with EINVAL.  Different
        choices of CMSG_SPACE and CMSG_LEN in different places will
        cause different problems.

        (The above fragment blithely uses CMSG_SPACE, which expands to
        a non-constant expression, to calculate the size of a stack-
        allocated array.  Perhaps it should use dynamic allocation
        instead, but I wrote the above to reflect directly the idiom
        that is described in Stevens' book and that is presently
        supported and documented in OpenBSD and elsewhere.)

        For a complete program that exhibits the problem, fetch

          <http://mumble.net/~campbell/tmp/passfd.c>
          <http://mumble.net/~campbell/tmp/passfd-main.c>

        and compile with

          gcc -g -Wall -c passfd-main.c -o passfd-main.o
          gcc -DMSGHDR_CMSG_SPACE -DCMSGHDR_CMSG_LEN \
            -DCMSG_BUFFER_STATIC_UNION -g -Wall \
            -c passfd.c -o passfd.o
          gcc passfd-main.o passfd.o -o passfd-test

        Finally, run `passfd-test' with an argument, any random message
        to send between processes.  sendmsg(2) will fail with EINVAL
        (which is returned from `send_fd').  Currently, I believe there
        are no definitions of MSGHDR_MSG_*, CMSGHDR_CMSG_*, or
        CMSG_BUFFER_* for passfd.c that will make the program run
        correctly everywhere; run `gcc -c passfd.c -o passfd.o' to see
        what options one has, or read the source.  Although for me

          -DMSGHDR_CMSG_SPACE -DCMSGHDR_CMSG_SPACE -DCMSG_BUFFER_MALLOC

        will cause no output to be written to stderr -- the goal --, I
        believe that the kernel is examining a bogus, random file
        descriptor in the process of transmitting the message, which
        could be a security vulnerability; using ALLOCA, STATIC_UNION,
        or STATIC_ARRAY instead of MALLOC causes sendmsg(2) to yield
        EBADF for me.  I believe the correct options should be

          -DMSGHDR_CMSG_SPACE -DCMSGHDR_MSG_LEN -DCMSG_BUFFER_MALLOC

        but on my PowerPC, these cause sendmsg(2) to yield EINVAL.

>Fix:

        Apply the following patch to src/sys/kern/uipc_usrreq.c
        (OpenBSD checks for two possible values of control->m_len,
        either cm->cmsg_len or CMSG_ALIGN (cm_cmsg_len); FreeBSD does
        this more lenient test):

--- uipc_usrreq.c.~1.115.~      2008-06-15 01:53:13.000000000 +0000
+++ uipc_usrreq.c       2008-06-19 23:13:49.000000000 +0000
@@ -1291,7 +1291,7 @@
 
        /* Sanity check the control message header. */
        if (cm->cmsg_type != SCM_RIGHTS || cm->cmsg_level != SOL_SOCKET ||
-           cm->cmsg_len != control->m_len)
+           cm->cmsg_len > control->m_len)
                return (EINVAL);
 
        /*