From 92a8e90db1bfdbb3f31f91a381bb62550c66236a Mon Sep 17 00:00:00 2001 From: Mateusz Guzik Date: Wed, 13 Aug 2014 04:48:11 +0200 Subject: [PATCH 2/2] Use sequence counters to make sure fget_unlocked gets a file pointers in sync with its capabilities. 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 #include #include #include #include +#include #include #include @@ -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