svn commit: r239301 - in head/sys: kern nlm sys

[Peter Jeremy]

On 2012-Aug-15 15:56:21 +0000, Konstantin Belousov <kib at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-head/attachments/20120816/7358a46d/attachment.pgp