Date: Thu, 16 Aug 2012 17:29:16 +1000 From: Peter Jeremy <peter@rulingia.com> To: Konstantin Belousov <kib@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r239301 - in head/sys: kern nlm sys Message-ID: <20120816072916.GA12294@server.rulingia.com> In-Reply-To: <201208151556.q7FFuLnM076015@svn.freebsd.org> References: <201208151556.q7FFuLnM076015@svn.freebsd.org>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --]
On 2012-Aug-15 15:56:21 +0000, Konstantin Belousov <kib@freebsd.org> wrote:
> Add a sysctl kern.pid_max, which limits the maximum pid the system is
> allowed to allocate, and corresponding tunable with the same
> name. Note that existing processes with higher pids are left intact.
Sorry for not picking this up when you first posted the patch but I
think you need to place a lower bound on max_pid to prevent the system
being rendered unusable.
>Modified: head/sys/kern/kern_fork.c
>==============================================================================
>--- head/sys/kern/kern_fork.c Wed Aug 15 15:53:27 2012 (r239300)
>+++ head/sys/kern/kern_fork.c Wed Aug 15 15:56:21 2012 (r239301)
>@@ -209,8 +209,8 @@ sysctl_kern_randompid(SYSCTL_HANDLER_ARG
> pid = randompid;
> error = sysctl_handle_int(oidp, &pid, 0, req);
> if (error == 0 && req->newptr != NULL) {
>- if (pid < 0 || pid > PID_MAX - 100) /* out of range */
>- pid = PID_MAX - 100;
>+ if (pid < 0 || pid > pid_max - 100) /* out of range */
>+ pid = pid_max - 100;
Setting max_pid to a value less than 100 will have an undesirable effect here.
>+static int
>+sysctl_kern_pid_max(SYSCTL_HANDLER_ARGS)
>+{
>+ int error, pm;
>+
>+ pm = pid_max;
>+ error = sysctl_handle_int(oidp, &pm, 0, req);
>+ if (error || !req->newptr)
>+ return (error);
>+ sx_xlock(&proctree_lock);
>+ sx_xlock(&allproc_lock);
>+ /* Only permit the values less then PID_MAX. */
>+ if (pm > PID_MAX)
>+ error = EINVAL;
>+ else
>+ pid_max = pm;
>+ sx_xunlock(&allproc_lock);
>+ sx_xunlock(&proctree_lock);
>+ return (error);
>+}
>+SYSCTL_PROC(_kern, OID_AUTO, pid_max, CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_TUN |
>+ CTLFLAG_MPSAFE, 0, 0, sysctl_kern_pid_max, "I",
>+ "Maximum allowed pid");
I don't see anything in this code that would prevent setting max_pid
to an unusably low (as in making the system unusable) or even negative
value
>+ TUNABLE_INT_FETCH("kern.pid_max", &pid_max);
>+ if (pid_max > PID_MAX)
>+ pid_max = PID_MAX;
> }
Likewise, this needs a lower bounds check.
--
Peter Jeremy
[-- Attachment #2 --]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (FreeBSD)
iEYEARECAAYFAlAsoUwACgkQ/opHv/APuIecLgCfb6vRgonlR0UnIf6f6pMioKiK
ki8AnjX7t4aEvoCr+EChC6MRSHF31fQy
=e8AG
-----END PGP SIGNATURE-----
help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120816072916.GA12294>