On 2012-Aug-15 15:56:21 +0000, Konstantin Belousov <k...@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: pgpjSBXtQZhiY.pgp
Description: PGP signature

Reply via email to