On 04 Oct 2014, at 08:08 , Mateusz Guzik <m...@freebsd.org> wrote:
> 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
> …
This file is included from user space. There is no opt_capsicum.h there.
Including an opt_* in the header file seems wrong in a lot of ways usually.
I tried to add a bandaid for the moment with r272523 which (to be honest) makes
it worse.
This needs a better fix.
I also wonder why the (conditional) fde_seq ended up at the beginning of the
structure rather than the end?
> 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.
>
—
Bjoern A. Zeeb "Come on. Learn, goddamn it.", WarGames, 1983
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
