--ELM715089176-380-0_
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=US-ASCII

Hi,
I'd like to bump the f_count and f_msgcount members of struct file
to 'u_long', so that they can't be overflowed easily (it's possible
now, since they are just 'short' and overflow is not checked anywhere).

I wonder if any DIAGNOSTIC/DEBUG checks need to be added too
for f_count/f_msgcount. I believe it's not necessary, see below.

Since 'maxfiles' is an 'int', number of descriptors (and thus
references to appropriate struct file) could not be bigger than 2
** (sizeof(int) * 8 - 1). Theoretically, the amount of all of
references might be temporarily higher than maxfiles if descriptor
passing is used. In that case, number of references might be as
high as (maxfiles + unp_rights), where both 'maxfiles' and 'unp_rights'
are 'int'.

On 32bit archs, the overflow can't actually happen - 2 * 2 **
(sizeof(int) * 8 - 1) references means there exist that number of
pointers to the struct, which would consume 4GB of physical &
virtual kernel memory, which is unfeasible even with Intel's PSE36
extension support.  On LP64 archs, it is theoretically possible
for kernel to have 4GB memory just for the the file descriptor
pointers.  However, (2 * 2 ** (sizeof(int) * 8 - 1)) still fits
into 'long' on LP64, so we are covered. Of course, unp_internalize()
should be changed to check for 'unp_rights' overflow and return
error if that would happen.

I'm appending the patch to fix this issue.

Opinions?

Jarda
-- 
Jaromir Dolecek <jaromir.dolecek@artisys.cz>
ARTISYS, s.r.o., Stursova 71, 61600 Brno, Czech Republic
phone: +420-5-41224836 / fax: +420-5-41224870 / http://www.artisys.cz

--ELM715089176-380-0_
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=US-ASCII
Content-Disposition: attachment; filename=frefs.diff

Index: conf/param.c
===================================================================
RCS file: /cvsroot/syssrc/sys/conf/param.c,v
retrieving revision 1.39
diff -u -p -r1.39 param.c
--- param.c	2001/11/08 05:59:31	1.39
+++ param.c	2001/12/14 09:29:23
@@ -107,10 +107,12 @@ int	tickadj = 240000 / (60 * HZ);		/* ca
 int	rtc_offset = RTC_OFFSET;
 int	maxproc = NPROC;
 int	desiredvnodes = NVNODE;
-int	maxfiles = MAXFILES;
 int	ncallout = 16 + NPROC;	/* size of callwheel (rounded to ^2) */
 u_long	sb_max = SB_MAX;	/* maximum socket buffer size */
 int	fscale = FSCALE;	/* kernel uses `FSCALE', user uses `fscale' */
+
+/* (maxfiles + unp_rights) MUST fit into struct file's f_count/f_msgcount */
+int	maxfiles = MAXFILES;
 
 /*
  * Various mbuf-related parameters.  These can also be changed at run-time
Index: kern/uipc_usrreq.c
===================================================================
RCS file: /cvsroot/syssrc/sys/kern/uipc_usrreq.c,v
retrieving revision 1.53
diff -u -p -r1.53 uipc_usrreq.c
--- uipc_usrreq.c	2001/11/12 15:25:34	1.53
+++ uipc_usrreq.c	2001/12/14 09:29:53
@@ -481,7 +481,9 @@ u_long	unpst_recvspace = PIPSIZ;
 u_long	unpdg_sendspace = 2*1024;	/* really max datagram size */
 u_long	unpdg_recvspace = 4*1024;
 
+/* (maxfiles + unp_rights) MUST fit into struct file's f_count/f_msgcount */
 int	unp_rights;			/* file descriptors in flight */
+#define UNPRIGHTS_SZ	INT_MAX
 
 int
 unp_attach(so)
@@ -939,6 +941,10 @@ unp_internalize(control, p)
 			return (EBADF);
 	}
 
+	/* Verify that unp_rights would not overflow */
+	if (__predict_false(unp_rights > INT_MAX - nfds))
+		return (EMFILE);
+
 	/* Make sure we have room for the struct file pointers */
  morespace:
 	neededspace = CMSG_SPACE(nfds * sizeof(struct file *)) -
@@ -957,6 +963,14 @@ unp_internalize(control, p)
 		/* copy the data to the cluster */
 		memcpy(mtod(control, char *), cm, cm->cmsg_len);
 		cm = mtod(control, struct cmsghdr *);
+
+		/*
+		 * Verify that unp_rights would not overflow again, we might
+		 * have been blocked.
+		 */
+		if (__predict_false(unp_rights > INT_MAX - nfds))
+			return (EMFILE);
+
 		goto morespace;
 	}
 
Index: sys/file.h
===================================================================
RCS file: /cvsroot/syssrc/sys/sys/file.h,v
retrieving revision 1.30
diff -u -p -r1.30 file.h
--- file.h	2001/12/07 07:09:30	1.30
+++ file.h	2001/12/14 09:29:53
@@ -59,10 +59,9 @@ struct file {
 #define	DTYPE_VNODE	1		/* file */
 #define	DTYPE_SOCKET	2		/* communications endpoint */
 #define	DTYPE_PIPE	3		/* pipe */
-	short		f_type;		/* descriptor type */
-	short		f_count;	/* reference count */
-	short		f_msgcount;	/* references from message queue */
-	short		f_pad0;		/* spare */
+	int		f_type;		/* descriptor type */
+	long		f_count;	/* reference count */
+	long		f_msgcount;	/* references from message queue */
 	struct ucred	*f_cred;	/* creds associated with descriptor */
 	struct fileops {
 		int	(*fo_read)	(struct file *fp, off_t *offset,

--ELM715089176-380-0_--