[PATCH 2/2] Use sequence counters to make sure fget_unlocked gets a file pointers in sync with its capabilities.

[Mateusz Guzik]

Otherwise there are exploitable races (e.g. with dup2) which can be used
to circumvent protection.
---
 sys/kern/kern_descrip.c | 55 ++++++++++++++++++++++++++++++++++++++++++-------
 sys/sys/filedesc.h      | 16 ++++++++++++++
 2 files changed, 64 insertions(+), 7 deletions(-)

diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 4aeb380..1033b2f 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -309,11 +309,18 @@ _fdfree(struct filedesc *fdp, int fd, int last)
 	struct filedescent *fde;
 
 	fde = &fdp->fd_ofiles[fd];
+#ifdef CAPABILITIES
+	if (!last)
+		seq_write_begin(&fde->fde_seq);
+#endif
 	filecaps_free(&fde->fde_caps);
 	if (last)
 		return;
-	bzero(fde, sizeof(*fde));
+	bzero(fde_change(fde), fde_change_size);
 	fdunused(fdp, fd);
+#ifdef CAPABILITIES
+	seq_write_end(&fde->fde_seq);
+#endif
 }
 
 static inline void
@@ -902,13 +909,19 @@ do_dup(struct thread *td, int flags, int old, int new,
 	/*
 	 * Duplicate the source descriptor.
 	 */
+#ifdef CAPABILITIES
+	seq_write_begin(&newfde->fde_seq);
+#endif
 	filecaps_free(&newfde->fde_caps);
-	*newfde = *oldfde;
+	memcpy(fde_change(newfde), fde_change(oldfde), fde_change_size);
 	filecaps_copy(&oldfde->fde_caps, &newfde->fde_caps);
 	if ((flags & DUP_CLOEXEC) != 0)
 		newfde->fde_flags = oldfde->fde_flags | UF_EXCLOSE;
 	else
 		newfde->fde_flags = oldfde->fde_flags & ~UF_EXCLOSE;
+#ifdef CAPABILITIES
+	seq_write_end(&newfde->fde_seq);
+#endif
 	*retval = new;
 
 	if (delfp != NULL) {
@@ -1776,6 +1789,9 @@ finstall(struct thread *td, struct file *fp, int *fd, int flags,
 	}
 	fhold(fp);
 	fde = &fdp->fd_ofiles[*fd];
+#ifdef CAPABILITIES
+	seq_write_begin(&fde->fde_seq);
+#endif
 	fde->fde_file = fp;
 	if ((flags & O_CLOEXEC) != 0)
 		fde->fde_flags |= UF_EXCLOSE;
@@ -1783,6 +1799,9 @@ finstall(struct thread *td, struct file *fp, int *fd, int flags,
 		filecaps_move(fcaps, &fde->fde_caps);
 	else
 		filecaps_fill(&fde->fde_caps);
+#ifdef CAPABILITIES
+	seq_write_end(&fde->fde_seq);
+#endif
 	FILEDESC_XUNLOCK(fdp);
 	return (0);
 }
@@ -2315,6 +2334,7 @@ fget_unlocked(struct filedesc *fdp, int fd, cap_rights_t *needrightsp,
 	struct file *fp;
 	u_int count;
 #ifdef CAPABILITIES
+	seq_t seq;
 	cap_rights_t haverights;
 	int error;
 #endif
@@ -2338,7 +2358,12 @@ fget_unlocked(struct filedesc *fdp, int fd, cap_rights_t *needrightsp,
 	 */
 	for (;;) {
 #ifdef CAPABILITIES
+		seq = seq_read(fd_seq(fdp, fd));
 		fde = fdt->fdt_ofiles[fd];
+		if (!seq_consistent(fd_seq(fdp, fd), seq)) {
+			cpu_spinwait();
+			continue;
+		}
 		fp = fde.fde_file;
 #else
 		fp = fdt->fdt_ofiles[fd].fde_file;
@@ -2724,6 +2749,7 @@ int
 dupfdopen(struct thread *td, struct filedesc *fdp, int dfd, int mode,
     int openerror, int *indxp)
 {
+	struct filedescent *newfde, *oldfde;
 	struct file *fp;
 	int error, indx;
 
@@ -2767,17 +2793,32 @@ dupfdopen(struct thread *td, struct filedesc *fdp, int dfd, int mode,
 			return (EACCES);
 		}
 		fhold(fp);
-		fdp->fd_ofiles[indx] = fdp->fd_ofiles[dfd];
-		filecaps_copy(&fdp->fd_ofiles[dfd].fde_caps,
-		    &fdp->fd_ofiles[indx].fde_caps);
+		newfde = &fdp->fd_ofiles[indx];
+		oldfde = &fdp->fd_ofiles[dfd];
+#ifdef CAPABILITIES
+		seq_write_begin(&newfde->fde_seq);
+#endif
+		memcpy(fde_change(newfde), fde_change(oldfde), fde_change_size);
+		filecaps_copy(&oldfde->fde_caps, &newfde->fde_caps);
+#ifdef CAPABILITIES
+		seq_write_end(&newfde->fde_seq);
+#endif
 		break;
 	case ENXIO:
 		/*
 		 * Steal away the file pointer from dfd and stuff it into indx.
 		 */
-		fdp->fd_ofiles[indx] = fdp->fd_ofiles[dfd];
-		bzero(&fdp->fd_ofiles[dfd], sizeof(fdp->fd_ofiles[dfd]));
+		newfde = &fdp->fd_ofiles[indx];
+		oldfde = &fdp->fd_ofiles[dfd];
+#ifdef CAPABILITIES
+		seq_write_begin(&newfde->fde_seq);
+#endif
+		memcpy(fde_change(newfde), fde_change(oldfde), fde_change_size);
+		bzero(fde_change(oldfde), fde_change_size);
 		fdunused(fdp, dfd);
+#ifdef CAPABILITIES
+		seq_write_end(&newfde->fde_seq);
+#endif
 		break;
 	}
 	FILEDESC_XUNLOCK(fdp);
diff --git a/sys/sys/filedesc.h b/sys/sys/filedesc.h
index 29cb6ae..a7f7737 100644
--- a/sys/sys/filedesc.h
+++ b/sys/sys/filedesc.h
@@ -33,11 +33,14 @@
 #ifndef _SYS_FILEDESC_H_
 #define	_SYS_FILEDESC_H_
 
+#include "opt_capsicum.h"
+
 #include <sys/caprights.h>
 #include <sys/queue.h>
 #include <sys/event.h>
 #include <sys/lock.h>
 #include <sys/priority.h>
+#include <sys/seq.h>
 #include <sys/sx.h>
 
 #include <machine/_limits.h>
@@ -50,6 +53,9 @@ struct filecaps {
 };
 
 struct filedescent {
+#ifdef CAPABILITIES
+	seq_t		 fde_seq;		/* if you need fde_file and fde_caps in sync */
+#endif
 	struct file	*fde_file;		/* file structure for open file */
 	struct filecaps	 fde_caps;		/* per-descriptor rights */
 	uint8_t		 fde_flags;		/* per-process open file flags */
@@ -58,6 +64,13 @@ struct filedescent {
 #define	fde_fcntls	fde_caps.fc_fcntls
 #define	fde_ioctls	fde_caps.fc_ioctls
 #define	fde_nioctls	fde_caps.fc_nioctls
+#ifdef CAPABILITIES
+#define	fde_change(fde)	((char *)(fde) + sizeof(seq_t))
+#define	fde_change_size	(sizeof(struct filedescent) - sizeof(seq_t))
+#else
+#define	fde_change(fde)	((fde))
+#define	fde_change_size	(sizeof(struct filedescent))
+#endif
 
 struct fdescenttbl {
 	int	fdt_nfiles;		/* number of open files allocated */
@@ -88,6 +101,9 @@ struct filedesc {
 };
 #define	fd_nfiles	fd_files->fdt_nfiles
 #define	fd_ofiles	fd_files->fdt_ofiles
+#ifdef CAPABILITIES
+#define	fd_seq(fdp, fd)	(&(fdp)->fd_ofiles[(fd)].fde_seq)
+#endif
 
 /*
  * Structure to keep track of (process leader, struct fildedesc) tuples.
-- 
2.0.2