Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 4 Oct 2014 14:21:54 +0000
From:      "Bjoern A. Zeeb" <bz@FreeBSD.org>
To:        Mateusz Guzik <mjg@FreeBSD.org>, Konstantin Belousov <kib@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r272505 - in head/sys: kern sys
Message-ID:  <42180557-0119-4597-9492-662E1671A840@FreeBSD.org>
In-Reply-To: <201410040808.s9488uAI099166@svn.freebsd.org>
References:  <201410040808.s9488uAI099166@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

On 04 Oct 2014, at 08:08 , Mateusz Guzik <mjg@FreeBSD.org> wrote:

> Author: mjg
> Date: Sat Oct  4 08:08:56 2014
> New Revision: 272505
> URL: https://svnweb.freebsd.org/changeset/base/272505
>=20
> Log:
>  Plug capability races.
>=20
>  fp and appropriate capability lookups were not atomic, which could =
result in
>  improper capabilities being checked.
>=20
>  This could result either in protection bypass or in a spurious =
ENOTCAPABLE.
>=20
>  Make fp + capability check atomic with the help of sequence counters.
>=20
>  Reviewed by:	kib
>  MFC after:	3 weeks
>=20
> Modified:
>  head/sys/kern/kern_descrip.c
>  head/sys/sys/filedesc.h
> =85


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
> =
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D
> --- 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_
>=20
> +#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>
>=20
> #include <machine/_limits.h>
> @@ -50,6 +53,9 @@ struct filecaps {
> };
>=20
> 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
>=20
> /*
>  * 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
>=20
> /*
>  * Structure to keep track of (process leader, struct fildedesc) =
tuples.
>=20

=97=20
Bjoern A. Zeeb             "Come on. Learn, goddamn it.", WarGames, 1983




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?42180557-0119-4597-9492-662E1671A840>