Subject: Re: MSG_CTRUNC vs SCM_RIGHTS
To: None <tech-net@netbsd.org>
From: der Mouse <mouse@Rodents.Montreal.QC.CA>
List: tech-net
Date: 04/12/2004 13:17:12
>> Most basically, the problem is that once unp_externalize has been
>> called for a message, the process has the descriptors - but if the
>> control buffer is too small, the process won't find out the file
>> descriptor numbers of all of them.  [...]

> I think this is a bug.  I am rather perplexed as to what a simple,
> clean way to fix it would be -- if there is one.

Me too.  Lacking a simple and clean fix, I implemented a simple and
slightly less clean fix: in uipc_syscalls.c, where MSG_CTRUNC is set, I
close any file descriptors that are getting truncated away.  This calls
for a slight reorganization of the control structure, to correctly
handle mbufs that get completely truncated, but it's not all that hard.

Here's the patch I'm using.  Note this is before the SCM_RIGHTS ABI
change, but updating it for after the change wouldn't be hard; I've
looked at the relevant code in -current, and except for the ABI change
it looks identical.  This patch works perfectly for me: if I send 8 fds
to a process expecting to receive 5, cmsg_len shows that 8 were sent,
MSG_CTRUNC is set, and the 3 whose numbers weren't received aren't in
the process's fd table after the recvmsg (without the patch, they are).

Any intellectual property rights I may have in this patch I hereby
release into the public domain.  You are welcome to pick it up and sue
it in any way you care to.

--- .OLD/sys/kern/uipc_syscalls.c	Sat Feb 19 19:56:27 2000
+++ .NEW/sys/kern/uipc_syscalls.c	Mon Apr 12 04:32:55 2004
@@ -671,6 +671,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 - sizeof(struct cmsghdr)) / sizeof(int);
+ if (len < sizeof(struct cmsghdr)) len = sizeof(struct cmsghdr);
+ nok = (len - sizeof(struct cmsghdr)) / sizeof(int);
+ fdv = (int *)(mtod(m,struct cmsghdr *)+1);
+ for (i=nok;i<nfd;i++) fdrelease(p,fdv[i]);
+}
+
 int
 recvit(p, s, mp, namelenp, retsize)
 	register struct proc *p;
@@ -803,24 +824,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