Subject: kern/30370: MSG_CTRUNCed fds arrive anyway
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: der Mouse <mouse@Rodents.Montreal.QC.CA>
List: netbsd-bugs
Date: 05/29/2005 21:22:00
>Number:         30370
>Category:       kern
>Synopsis:       SCM_RIGHTS fds dropped for MSG_CTRUNC reasons arrive anyway
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun May 29 21:22:00 +0000 2005
>Originator:     der Mouse
>Release:        NetBSD 2.0 (and older, believed -current as well)
>Organization:
	Dis-
>Environment:
	Any, probably: 2.0 and 1.4T for sure, and UTSLing leads me to
	think -current has the same problem.
>Description:
	If a process S uses recvmsg() to receive messages that bear
	file descriptors in SCM_RIGHTS control messages, and a process
	C sends a messge bearing more file descriptors than S expects,
	S will get a message with MSG_CTRUNC set in its msg_flags - but
	the truncated-away file descriptors will still arrive in S's
	open file table, even though S is not given their fd numbers.

	This was also discussed briefly on tech-net, starting
	2004-04-12, under the subject "MSG_CTRUNC vs SCM_RIGHTS".  My
	second message in that thread includes a patch
	(http://mail-index.netbsd.org/tech-net/2004/04/12/0002.html is
	the archive copy of the message with the patch).  In the "Fix"
	section below is an updating of that patch for 2.0.

	A message bearing so many file descriptors the receiver can't
	receive them all will produce EMSGSIZE, but it won't put
	anything in the receiver's open file table, so that's OK.
	Paranoid receivers need to be prepared for EMSGSIZE anyway.

	I've marked this PR as low priority because it apperas to me
	that very few people actually use SCM_RIGHTS fd passing, and
	even fewer use it in contexts where malicious senders are an
	issue.  If I'm wrong, feel free to reprioritize it higher.
>How-To-Repeat:

	uudecode, bunzip2, compile, and run:

begin 644 fdpasstest.c.bz2
M0EIH.3%!62936<2S4DP``3!?@$`P>W_P7Z_OWPZ__]_^4`69>R0K;;6R0*D*
M(22F*3U/4VI^J-,:FAILH!H,0`-`&U`!S3(R&3!#1A,$::-&(&F3(P`!!(B3
M29,II3S(&IZILF0>FD:```:&@T'-,C(9,$-&$P1IHT8@:9,C``$"I)!,@"#$
MU/24_2GH#4]$:/4P1M0/)/0$I$DL'B+>#A_NAR"S"&D0TDVQA:SL:;%$N!*!
MB8Q2UP4M+&B*,*J*.]`%$DNOL\O2NT&`DVZ0J%E?+-X/D;^L5I.0-FG+(02/
MNH]6<[YBX#8_)9^#X/QQ7J-E2VBV.:V>SYMAEI<!P&I[UY!-(6Z;ZJ+"VA73
M;2MSQ;HE-;KP*E:C8%[0=3!E%3EI@.B:2@KENL+!\D09"T51K8XOU^3:[._#
M'),M=FY;5AAY(NI'S5*-PS-&^HF1OQ1A_/6C/'>MBHJOHJJM7I%':G%CD<-P
M$)\8DPT&+(S:G[#],JV!-GFHU%[NL"(T<<I2(P.=I2#9WAJ@*W?;Z>YQ+R-K
M@3.9T1!WY^3S_+*<BFD02,XMJ#"UI1?XDM<13?U1A`1,R2F."VIMV4KT56%;
M&/(42S[-QK/UP4T[(T]HW?_VVFWGRSG6KIH:_;D]NP;+N5:,S=YDX)JP2,^D
MS98W.O$+4!89;:D!_Z?$AMN(`Z$OLE+G@,$7WAG;M6NTP&FT]ZB2@H:^A%-]
MVK3#98%`PZ]%K:[\.#)BN7M"^ZY0Z%^*XT82==%'<A>T+B\G3(`VV'#KH*M&
M+:`@@6'U#5S=6LZOB#!#/E8J9,:RK.-8)DO-A/:<)UA%*)>Z]:<-JSJ]TZYD
MORFJ:S+*3NEKO]U$X)[U).YQJCO>=&MQ2BP1N**MD!E-0T!+&"L*Z^#.#1#2
M,RN$BU^5L&-=WGV0X@:)VSQ,Q14.1>!B?1K@FR&>_T8+W-Z?BLE<YUEQ[#H>
M[Z7U!2HG6%'UCD]D_)<XVZ1/H3'ZW]YX:GI+:D5BI\!9?'5:HR@=!&Q]LB(B
M(9UIC0V>9P+E'M);81^Q&2X4CQ1[VUMJ"YD-RA@P[V[!2E@=EX$_IW,D=CB,
MI)L\-@\'@NT-O_?UQ]6-JT0PDO=?.]:Z-,N:L[[-"\R:ZS0TQD_8,M]4H<SW
MZS%[9F:OT'NYDU)/22A1T1KUY;CY//=A:T6NE<WE+T.7")&J&R=9?Q(>@;_"
M/C&?PDU*VR4GVHY893G-R5US)CBMN>@>3K?)YE3:1<D>/7DP3H[RRYJ<;F=<
MOXPU.C>%F496P8<*R4;MW-!&\V]U0(MS%;"-`R1C95';P9QH_ME8?'K<-$2U
M_!1+-U/EI:D7*:KL,P]Y%C`JD,&*!A;66D.,\K+V"5Y*V.WN0&Z<:!E0)&2L
M\:1949(*.@D+$E+EPMT,*J[RXS4&F'<E#2.F!8\TEY7C3Y?%\6WUY;@SIV:A
MK?B>WM/X=7?V<5B=HYPMHD=?.HI7-OO++S@+[V*^:K?5OCBD?R:&G3%\,),+
MBZ^##!'6:?75NY<+I1LVNF3=N^G.)(C?TFPGFCI1*F2.FZ,0>9+"[G5E0.8N
M!Y%Q19CHY;/)+XI12IR:86(MC@DO>K!(S[I9DB[)HB^2D=32CO'SH[-O[1I;
GNVFQ0JDWMI:7,'`H]F"-?B>)P:0XS(1_'`T#?^%W)%.%"0Q+-23`
`
end

	Under an unpatched 2.0/sparc kernel, I see

Free fds: 5 6 7 8 9 10 11 12 13
Received fd count 8 (CMSG_CTRUNC set)
Received more than we provided space for
Got: 5 6 -572662307 -572662307 -572662307 -572662307 -572662307 -572662307
Open: 5 6 7 8 9 10 11 12
Closed: 13

	The "Free fds" line shows the next available fds in the
	receiving process (which is where extra fds will land).  The
	sender sends 8 fds; the receiver is written to ask for only
	one, though (because of cmsg padding) it has space for more on
	some architectures, such as sparc.  The "Got" line prints out
	the descriptor numbers in the received buffer; the -572662307
	entries are space that is present in the buffer but is not
	overwritten by the recvmsg().  The "Open" and "Closed" lines
	report which of the "Free fds" are actually open, based on
	fcntl(F_GETFD) probes.  Note that fds 7 through 12 are open
	even though the received message does not tell the receiver
	about them.

	Under a kernel with the patch below applied, I get

Free fds: 5 6 7 8 9 10 11 12 13
Received fd count 8 (CMSG_CTRUNC set)
Received more than we provided space for
Got: 5 6 -572662307 -572662307 -572662307 -572662307 -572662307 -572662307
Open: 5 6
Closed: 7 8 9 10 11 12 13

	which is much more reasonable.

>Fix:
	This patch is for 2.0 (uipc_syscalls.c,v 1.86.2.1).  I expect
	the changes required for -current to minimal-to-zero, though I
	haven't checked that myself.  "It works for me."

--- OLD/sys/kern/uipc_syscalls.c	2005-05-29 16:56:09.000000000 -0400
+++ NEW/sys/kern/uipc_syscalls.c	2005-05-29 16:56:40.000000000 -0400
@@ -641,6 +641,27 @@
 	return (error);
 }
 
+/*
+ * Adjust for a truncated SCM_RIGHTS control message.  This means
+ *  closing any file descriptors that aren't entirely present in the
+ *  returned buffer.  m is the mbuf holding the (already externalized)
+ *  SCM_RIGHTS message; len is the length it is being truncated to.  p
+ *  is the affected process.
+ */
+static void adjust_rights(struct mbuf *m, int len, struct proc *p)
+{
+	int nfd;
+	int i;
+	int nok;
+	int *fdv;
+
+	nfd = (m->m_len - CMSG_LEN(0)) / sizeof(int);
+	nok = (len < CMSG_LEN(0)) ? 0 : ((len - CMSG_LEN(0)) / sizeof(int));
+	fdv = (int *) CMSG_DATA(mtod(m,struct cmsghdr *));
+	for (i = nok; i < nfd; i++)
+		fdrelease(p,fdv[i]);
+}
+
 int
 recvit(struct proc *p, int s, struct msghdr *mp, caddr_t namelenp,
 	register_t *retsize)
@@ -743,24 +764,32 @@
 			len = 0;
 		else {
 			struct mbuf *m = control;
-			caddr_t p = (caddr_t)mp->msg_control;
+			caddr_t cp = (caddr_t)mp->msg_control;
 
 			do {
 				i = m->m_len;
 				if (len < i) {
 					mp->msg_flags |= MSG_CTRUNC;
 					i = len;
+					if (mtod(m,struct cmsghdr *)->cmsg_type == SCM_RIGHTS)
+						adjust_rights(m,len,p);
 				}
-				error = copyout(mtod(m, caddr_t), p,
+				error = copyout(mtod(m, caddr_t), cp,
 				    (unsigned)i);
-				if (m->m_next)
+				m = m->m_next;
+				if (m)
 					i = ALIGN(i);
-				p += i;
+				cp += i;
 				len -= i;
 				if (error != 0 || len <= 0)
 					break;
-			} while ((m = m->m_next) != NULL);
-			len = p - (caddr_t)mp->msg_control;
+			} while (m != NULL);
+			while (m) {
+				if (mtod(m,struct cmsghdr *)->cmsg_type == SCM_RIGHTS)
+					adjust_rights(m,0,p);
+				m = m->m_next;
+			}
+			len = cp - (caddr_t)mp->msg_control;
 		}
 		mp->msg_controllen = len;
 	}

/~\ The ASCII				der Mouse
\ / Ribbon Campaign
 X  Against HTML	       mouse@rodents.montreal.qc.ca
/ \ Email!	     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B