Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 9 May 2013 11:25:38 +0300
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        John Baldwin <jhb@freebsd.org>
Cc:        arch@freebsd.org
Subject:   Re: Extending MADV_PROTECT
Message-ID:  <20130509082538.GQ3047@kib.kiev.ua>
In-Reply-To: <201305081209.49429.jhb@freebsd.org>
References:  <201305071433.27993.jhb@freebsd.org> <20130508095827.GK3047@kib.kiev.ua> <201305081209.49429.jhb@freebsd.org>

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

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

On Wed, May 08, 2013 at 12:09:49PM -0400, John Baldwin wrote:
> On Wednesday, May 08, 2013 5:58:27 am Konstantin Belousov wrote:
> > On Tue, May 07, 2013 at 02:33:27PM -0400, John Baldwin wrote:
> > > One of the issues I have with our current MADV_PROTECT is that it
> > > isn't very administrative-friendly. That is, as a sysadmin I can't
> > > easily protect arbitrary processes from the OOM killer. Instead, the
> > > binary has to be changed to invoke madvise(). Furthermore, once the
> > > protection is granted it can't be revoked. Also, any binaries that
> > > want this have to be run as root. Instead, I would like to be able
> > > to both set and revoke this for existing processes and possibly even
> > > allow it to be inherited (so I can tag a top-level daemon that forks
> > > and have all its future children be protected for example). To that
> > > end I've whipped up a simple patch (against 8, but should port to
> > > HEAD easily if folks think it is a good idea) to add a new pprotect()
> > > system call and userland program (protect) that can be used similar to
> > > ktrace(1) either as a modifier when running a new program or as a tool
> > > for setting or clearing protection for existing processes.
> > >
> > > The inherit feature isn't implemented yet, but it should be simple
> > > to do. One would simply need a new flag that PPROT_INHERIT sets that
> > > is checked on fork and propagates P_PROTECTED if it is set. Also,
> > > one other thought I had is that at some point we might want to make
> > > P_PROTECTED more fine-grained, e.g. by allowing for OOM "priorities".
> > > To that end, it may make sense to add a new argument to protect,
> > > though you could also reserve part of the 'op' parameter to encode a
> > > priority.
> >=20
> > Wouldn't the pprot_setchildren() miss a child for which the new pid and
> > struct proc are already allocated in the do_fork(), but which is not yet
> > linked into the process tree ?  If true, I think this does not
> > fulfill the promise of the PPROT_DESCEND.
>=20
> ktrace has the same issue, and really, this is just a race.  If the user
> had run the command a few nanoseconds earlier the proc wouldn't be alloca=
ted
> at all, and I doubt a user would notice the difference in those two cases.
> If you are doing this programmatically then that is a race that the progr=
am
> can handle.  It isn't any different from having a new process begin its
> fork() a few nanoseconds after this returns either.  This is why if you
> want that behavior you would use -di (and applies equally to ktrace).
So to get this correct, a person first should enable inheritance, and only
then turn on the protection on the subtree ? This sounds somewhat sloppy,
but fine.

>=20
> > It seems that the patch posted missed the chunk for sys/proc.h.
> > For HEAD, you probably need e.g. p_flag2 and P2_PROTECTED instead.
>=20
> P_PROTECTED already exists on both HEAD and 8.x (the MADV_PROTECT it is
> extending is quite old).  What we would need a p_flag2 for would be to
> add a new P_PROTECTED_INHERIT if we decided we wanted to support that.
>=20
> > Since the syscall is mean to be extended in the future, would it make
> > more sense to add a multiplexer, e.g. procctl(2), one operation of which
> > would be PROCCTL_PROTECT ?
>=20
> Do we expect it to do more than adjust protection?  We already have a few
> other process-control system calls (e.g. ptrace()).  It's hard to ensure
> it is sufficiently generic when only abstracting from one use case.

You mentioned a priority, and I think ability to pass a structure to the
sub-function of the syscall is better then carving bits in the int argument,
or introducing a new syscall.

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

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (FreeBSD)

iQIcBAEBAgAGBQJRi12BAAoJEJDCuSvBvK1B2tsQAKgeRYX3QS2qPKjCNSmXpHJS
ZueorZhQus7sbHU5gj4ur3Q0dC2KxH6/Zg7HeVBYnIZ7Re02RNifTENoUOfY3h6D
WGrdCubKBh0ME/QlLfdQ4GGX86JmzG7UOAvgAAH267h9T8Ly7tQexEPTIZwxRwcj
MYJdSPWS6D4EIfHxdT/Ao8ZMZ5eF3MLYwpTPBiALEj/Dm8xA7xLEGshJdKCVhDtY
jbTh61LeGuNKyZXoVuR2S25vwxac5RSLrZhlxiKNRsy6XuAJLzM7EIDZHfQ50n+K
xPSTIXZ9bMVMn8CYuGv11Xi6lLX2ipb5cGh7aGXtvRjF7ii3x0CrwuWL2FPKmJof
W7Hro4U2hJaOFtks566RPzcfNPfEfjonuhJGwJSCXV1zSESHCYulVAKGJb8KJ7wj
K5kj2DeBfm+bp23iCEFUEnabfXCJibYrbFh9mRuMYbQTJIK4XM7k4Rmkh7Iu2QqM
XhPvFFSU0wBWoBbzc26Ngkr/OLjQgTktj6oS/nqKN47fc9RLOORCszefJwGJ2JXd
VczjAQSP7jJB4qkeHrgNK0ZVUzt6HDArRnNK8DIltZDLZFiVwoxLkg9UvRpHsJma
Gp0OkqpIj8sOquSoIdZ6GA4tsQzE4d4//A3444JtX6hpKdkjBRedGO2ai+Gk1MR5
wXJgyd5AwstbbgGNJ2X5
=pKFU
-----END PGP SIGNATURE-----

--cw81t83UHTonwpSu--



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