Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 23 Jun 2014 19:35:23 +0300
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Mateusz Guzik <mjguzik@gmail.com>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Mateusz Guzik <mjg@FreeBSD.org>
Subject:   Re: svn commit: r267760 - head/sys/kern
Message-ID:  <20140623163523.GK93733@kib.kiev.ua>
In-Reply-To: <20140623131653.GC27040@dft-labs.eu>
References:  <201406230128.s5N1SIYK097224@svn.freebsd.org> <20140623064044.GD93733@kib.kiev.ua> <20140623070652.GA27040@dft-labs.eu> <20140623072519.GE93733@kib.kiev.ua> <20140623080501.GB27040@dft-labs.eu> <20140623081823.GG93733@kib.kiev.ua> <20140623131653.GC27040@dft-labs.eu>

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

--d6iqOn7HZPWKXx18
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Jun 23, 2014 at 03:16:53PM +0200, Mateusz Guzik wrote:
> On Mon, Jun 23, 2014 at 11:18:23AM +0300, Konstantin Belousov wrote:
> > On Mon, Jun 23, 2014 at 10:05:01AM +0200, Mateusz Guzik wrote:
> > > On Mon, Jun 23, 2014 at 10:25:19AM +0300, Konstantin Belousov wrote:
> > > > On Mon, Jun 23, 2014 at 09:06:52AM +0200, Mateusz Guzik wrote:
> > > > > The table is modified in these functions and is reachable from th=
e rest
> > > > > of the kernel (can be found by e.g. sysctl_kern_proc_filedesc), t=
hus
> > > > > XLOCK is needed to ensure consistency for readers. It can also be
> > > > > altered by mountcheckdirs, although not in a way which disrupts a=
ny of
> > > > > these functions.
> > > > I would think that such cases should be avoided by testing for P_IN=
EXEC,
> > > > but I do not insist on this.
> > > >=20
> > >=20
> > > proc lock has to be dropped before filedesc lock is taken and I don't
> > > see any way to prevent the proc from transitioning to P_INEXEC afterw=
ards.
> > > Since sysctl_kern_proc_filedesc et al can take a long time it does not
> > > seem feasible to pursue this.
> > >=20
>=20
> After a second look this problem has to be dealt with.
>=20
> If traversal while transition to P_INEXEC is allowed, execve dealing
> with a setuid binary is problematic. This is more of hypothetical nature,
> but with sufficienly long delay it could finish the syscall and start
> opening some files, which paths would now be visible for an unprivileged
> reader.
>=20
> That said, I propose adding a counter to struct proc which would which
> would block execve. It would be quite similar to p_lock.
I thought about this too.  In fact, I considered using PHOLD for this.

>=20
> iow execve would:
>=20
>         PROC_LOCK(p);
> 	p->p_flag |=3D P_INEXEC;=20
>         while (p->p_execlock > 0)
>                 msleep(&p->p_execlock, &p->p_mtx, PWAIT, "execlock", 0);
> 	PROC_UNLOCK(p);
>=20
> And it would be mandatory for external fdp consumers to grab the counter.
>=20
> I'm tempted to add P_GETPIN which would both increase p_lock and p_execlo=
ck,
> that way the process is guaranteed not to exit and not to execve even
> after proc lock is dropped.
See above about PHOLD.

>=20
> There is a separate question if p_execlock should be renamed and
> extended to also block any kind of credential changes.
>=20
> Then the guarantee is even stronger since we know that credentials we
> checked against are not going to change for the duration of our
> operations, but it is unclear if we need this.

If doing separate execlock/p_lock, I think that it could be possible
to use per-process sx lock instead of hand-rolling the counter.  The
accessors would lock sx shared, while kern_execve would take it in
exclusive mode.

--d6iqOn7HZPWKXx18
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJTqFdKAAoJEJDCuSvBvK1ByesP/3O3VMR+fSO9C4laRxens4do
NOBAA1n7HiR6RlLNcynFk9POCcXycDJGTXezIRkDbEOmLa/OJSV0xU584s6QsJgO
IcpbNf+kXTo5MPh5nuHHpaqc1OOGUATb0VvYg4+bOF/Oa/KfLgFe/oNW4oE+/FtQ
pltr1NeFqRUcnx0M78vpVb1QraMZ/Rm2YxaRvTb/ZRn4es2pcARqa4rN3QHvgCjf
zzkuyV1xYNqrBifD4j9K+zvjzy4q6x6NXWLHaqzjNGCYqmde1STvlhUwchToP2bl
dx6VvY2awJpYZ1sMdhAmDAlOfd8LxWJGCygUfeQj8iLopAVaBZObDOKvtTl8jv9a
PmYqzkI2/Z7Wo31z1TTEqSVMu9X940lBiX8J14YeNHIaF8mVG/xHDbqGaR8L3kr2
e5E2Xvk9Xx3s1Y8lCEkuurLip2cav2MN4jPEokEXyNASvz2HLlTm1OlBRf/Z0Alg
AEssNhUkr8aZ6nKQr/cNSiptIAnMYxRWOM+mkiSrgXKVK1G5o4HRPHhgMSHe4fU3
KZ0LLa9WTx+m334/WdR0cDS7v4Dx8LMFuB2wumCwDoFmBU/md/c25nTXL2jgRZ5e
7VQgSkdb2ngmzmuakaSqYRQBDHMNbJVt/8p54KNMY/ooKLByx0ICE+6ttM2fQDCg
Ro1HyH80rKXWB+7UtRrB
=QEZ4
-----END PGP SIGNATURE-----

--d6iqOn7HZPWKXx18--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140623163523.GK93733>