NetBSD Problem Report #38992

From riastradh@slate.localdomain  Fri Jun 20 00:28:43 2008
Return-Path: <riastradh@slate.localdomain>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id 686FE63B880
	for <gnats-bugs@gnats.NetBSD.org>; Fri, 20 Jun 2008 00:28:43 +0000 (UTC)
Message-Id: <20080620002831.7012576CBB@slate.localdomain>
Date: Fri, 20 Jun 2008 00:28:31 +0000 (UTC)
From: Taylor R Campbell <campbell@mumble.net>
Reply-To: Taylor R Campbell <campbell@mumble.net>
To: gnats-bugs@gnats.NetBSD.org
Subject: kernel requires incorrect alignment for cmsghdr cmsg_len field
X-Send-Pr-Version: 3.95

>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:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Jun 20 00:30:00 +0000 2008
>Closed-Date:    Mon Sep 02 00:28:19 +0000 2019
>Last-Modified:  Mon Sep 02 00:28:19 +0000 2019
>Originator:     Taylor R Campbell <campbell@mumble.net>
>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:/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);

 	/*

>Release-Note:

>Audit-Trail:
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/38992: CMSG_* fun
Date: Fri, 20 Jun 2008 10:57:27 +0200

 --SLDf9lqlvOQaIe6s
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline

 This check should be added to take care of the "proceeds happily"
 part.

 Martin

 --SLDf9lqlvOQaIe6s
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: attachment; filename=patch

 Index: uipc_usrreq.c
 ===================================================================
 RCS file: /cvsroot/src/sys/kern/uipc_usrreq.c,v
 retrieving revision 1.115
 diff -u -r1.115 uipc_usrreq.c
 --- uipc_usrreq.c	10 Jun 2008 11:49:11 -0000	1.115
 +++ uipc_usrreq.c	20 Jun 2008 08:47:55 -0000
 @@ -1299,6 +1299,9 @@
  	 * a reference to each.
  	 */
  	nfds = (cm->cmsg_len - CMSG_ALIGN(sizeof(*cm))) / sizeof(int);
 +	if (nfds <= 0)
 +		return EINVAL;
 +
  	fdp = (int *)CMSG_DATA(cm);
  	for (i = 0; i < nfds; i++) {
  		fd = *fdp++;

 --SLDf9lqlvOQaIe6s--

From: Taylor R Campbell <campbell@mumble.net>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/38992: kernel requires incorrect alignment for cmsghdr cmsg_len field
Date: Mon, 15 Sep 2008 11:47:57 -0400

 Can this be back-ported to the netbsd-4 branch and closed?

State-Changed-From-To: open->feedback
State-Changed-By: maya@NetBSD.org
State-Changed-When: Sun, 01 Sep 2019 23:51:28 +0000
State-Changed-Why:
Curious if this last statement means we can close it
"Can this be back-ported to the netbsd-4 branch and closed?"


State-Changed-From-To: feedback->closed
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Mon, 02 Sep 2019 00:28:19 +0000
State-Changed-Why:
yes, I don't think anyone needs a pullup to netbsd-4 any more


>Unformatted:

Home	PR Database Search