Date: Sat, 4 Oct 2014 08:08:56 +0000 (UTC) From: Mateusz Guzik <mjg@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r272505 - in head/sys: kern sys Message-ID: <201410040808.s9488uAI099166@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: mjg Date: Sat Oct 4 08:08:56 2014 New Revision: 272505 URL: https://svnweb.freebsd.org/changeset/base/272505 Log: Plug capability races. fp and appropriate capability lookups were not atomic, which could result in improper capabilities being checked. This could result either in protection bypass or in a spurious ENOTCAPABLE. Make fp + capability check atomic with the help of sequence counters. Reviewed by: kib MFC after: 3 weeks Modified: head/sys/kern/kern_descrip.c head/sys/sys/filedesc.h Modified: head/sys/kern/kern_descrip.c ============================================================================== --- head/sys/kern/kern_descrip.c Sat Oct 4 08:05:39 2014 (r272504) +++ head/sys/kern/kern_descrip.c Sat Oct 4 08:08:56 2014 (r272505) @@ -288,11 +288,18 @@ _fdfree(struct filedesc *fdp, int fd, in 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 @@ -883,13 +890,19 @@ do_dup(struct thread *td, int flags, int /* * 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) { @@ -1756,6 +1769,9 @@ finstall(struct thread *td, struct file } 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; @@ -1763,6 +1779,9 @@ finstall(struct thread *td, struct file 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); } @@ -2294,6 +2313,7 @@ fget_unlocked(struct filedesc *fdp, int struct file *fp; u_int count; #ifdef CAPABILITIES + seq_t seq; cap_rights_t haverights; int error; #endif @@ -2314,7 +2334,12 @@ fget_unlocked(struct filedesc *fdp, int */ for (;;) { #ifdef CAPABILITIES + seq = seq_read(fd_seq(fdp, fd)); fde = fdp->fd_ofiles[fd]; + if (!seq_consistent(fd_seq(fdp, fd), seq)) { + cpu_spinwait(); + continue; + } fp = fde.fde_file; #else fp = fdp->fd_ofiles[fd].fde_file; @@ -2343,7 +2368,11 @@ fget_unlocked(struct filedesc *fdp, int */ if (atomic_cmpset_acq_int(&fp->f_count, count, count + 1) != 1) continue; +#ifdef CAPABILITIES + if (seq_consistent_nomb(fd_seq(fdp, fd), seq)) +#else if (fp == fdp->fd_ofiles[fd].fde_file) +#endif break; fdrop(fp, curthread); } @@ -2700,6 +2729,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; @@ -2743,17 +2773,32 @@ dupfdopen(struct thread *td, struct file 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); Modified: head/sys/sys/filedesc.h ============================================================================== --- head/sys/sys/filedesc.h Sat Oct 4 08:05:39 2014 (r272504) +++ head/sys/sys/filedesc.h Sat Oct 4 08:08:56 2014 (r272505) @@ -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 /* * This structure is used for the management of descriptors. It may be @@ -82,6 +95,9 @@ struct filedesc { int fd_holdleaderscount; /* block fdfree() for shared close() */ int fd_holdleaderswakeup; /* fdfree() needs wakeup */ }; +#ifdef CAPABILITIES +#define fd_seq(fdp, fd) (&(fdp)->fd_ofiles[(fd)].fde_seq) +#endif /* * Structure to keep track of (process leader, struct fildedesc) tuples.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201410040808.s9488uAI099166>