Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 8 Oct 2015 11:07:09 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r289026 - head/sys/kern
Message-ID:  <201510081107.t98B791W017144@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kib
Date: Thu Oct  8 11:07:09 2015
New Revision: 289026
URL: https://svnweb.freebsd.org/changeset/base/289026

Log:
  Enforce the maxproc limitation before allocating struct proc, initial
  struct thread and kernel stack for the thread.  Otherwise, a load
  similar to a fork bomb would exhaust KVA and possibly kmem, mostly due
  to the struct proc being type-stable.
  
  The nprocs counter is changed from being protected by allproc_lock sx
  to be an atomic variable.  Note that ddb/db_ps.c:db_ps() use of nprocs
  was unsafe before, and is still unsafe, but it seems that the only
  possible undesired consequence is the harmless warning printed when
  allproc linked list length does not match nprocs.
  
  Diagnosed by:	Svatopluk Kraus <onwahe@gmail.com>
  Tested by:	pho
  Sponsored by:	The FreeBSD Foundation
  MFC after:	1 week

Modified:
  head/sys/kern/kern_exit.c
  head/sys/kern/kern_fork.c

Modified: head/sys/kern/kern_exit.c
==============================================================================
--- head/sys/kern/kern_exit.c	Thu Oct  8 10:00:41 2015	(r289025)
+++ head/sys/kern/kern_exit.c	Thu Oct  8 11:07:09 2015	(r289026)
@@ -963,9 +963,7 @@ proc_reap(struct thread *td, struct proc
 	KASSERT(FIRST_THREAD_IN_PROC(p),
 	    ("proc_reap: no residual thread!"));
 	uma_zfree(proc_zone, p);
-	sx_xlock(&allproc_lock);
-	nprocs--;
-	sx_xunlock(&allproc_lock);
+	atomic_add_int(&nprocs, -1);
 }
 
 static int

Modified: head/sys/kern/kern_fork.c
==============================================================================
--- head/sys/kern/kern_fork.c	Thu Oct  8 10:00:41 2015	(r289025)
+++ head/sys/kern/kern_fork.c	Thu Oct  8 11:07:09 2015	(r289026)
@@ -382,12 +382,6 @@ do_fork(struct thread *td, int flags, st
 	p2_held = 0;
 	p1 = td->td_proc;
 
-	/*
-	 * Increment the nprocs resource before blocking can occur.  There
-	 * are hard-limits as to the number of processes that can run.
-	 */
-	nprocs++;
-
 	trypid = fork_findpid(flags);
 
 	sx_sunlock(&proctree_lock);
@@ -771,16 +765,14 @@ int
 fork1(struct thread *td, int flags, int pages, struct proc **procp,
     int *procdescp, int pdflags, struct filecaps *fcaps)
 {
-	struct proc *p1;
-	struct proc *newproc;
-	int ok;
+	struct proc *p1, *newproc;
 	struct thread *td2;
 	struct vmspace *vm2;
+	struct file *fp_procdesc;
 	vm_ooffset_t mem_charged;
-	int error;
+	int error, nprocs_new, ok;
 	static int curfail;
 	static struct timeval lastfail;
-	struct file *fp_procdesc = NULL;
 
 	/* Check for the undefined or unimplemented flags. */
 	if ((flags & ~(RFFLAGS | RFTSIGFLAGS(RFTSIGMASK))) != 0)
@@ -819,6 +811,35 @@ fork1(struct thread *td, int flags, int 
 		return (fork_norfproc(td, flags));
 	}
 
+	fp_procdesc = NULL;
+	newproc = NULL;
+	vm2 = NULL;
+
+	/*
+	 * Increment the nprocs resource before allocations occur.
+	 * Although process entries are dynamically created, we still
+	 * keep a global limit on the maximum number we will
+	 * create. There are hard-limits as to the number of processes
+	 * that can run, established by the KVA and memory usage for
+	 * the process data.
+	 *
+	 * Don't allow a nonprivileged user to use the last ten
+	 * processes; don't let root exceed the limit.
+	 */
+	nprocs_new = atomic_fetchadd_int(&nprocs, 1) + 1;
+	if ((nprocs_new >= maxproc - 10 && priv_check_cred(td->td_ucred,
+	    PRIV_MAXPROC, 0) != 0) || nprocs_new >= maxproc) {
+		error = EAGAIN;
+		sx_xlock(&allproc_lock);
+		if (ppsratecheck(&lastfail, &curfail, 1)) {
+			printf("maxproc limit exceeded by uid %u (pid %d); "
+			    "see tuning(7) and login.conf(5)\n",
+			    td->td_ucred->cr_ruid, p1->p_pid);
+		}
+		sx_xunlock(&allproc_lock);
+		goto fail2;
+	}
+
 	/*
 	 * If required, create a process descriptor in the parent first; we
 	 * will abandon it if something goes wrong. We don't finit() until
@@ -831,7 +852,6 @@ fork1(struct thread *td, int flags, int 
 	}
 
 	mem_charged = 0;
-	vm2 = NULL;
 	if (pages == 0)
 		pages = kstack_pages;
 	/* Allocate new proc. */
@@ -898,20 +918,7 @@ fork1(struct thread *td, int flags, int 
 
 	/* We have to lock the process tree while we look for a pid. */
 	sx_slock(&proctree_lock);
-
-	/*
-	 * Although process entries are dynamically created, we still keep
-	 * a global limit on the maximum number we will create.  Don't allow
-	 * a nonprivileged user to use the last ten processes; don't let root
-	 * exceed the limit. The variable nprocs is the current number of
-	 * processes, maxproc is the limit.
-	 */
 	sx_xlock(&allproc_lock);
-	if ((nprocs >= maxproc - 10 && priv_check_cred(td->td_ucred,
-	    PRIV_MAXPROC, 0) != 0) || nprocs >= maxproc) {
-		error = EAGAIN;
-		goto fail;
-	}
 
 	/*
 	 * Increment the count of procs running with this uid. Don't allow
@@ -942,11 +949,7 @@ fork1(struct thread *td, int flags, int 
 	}
 
 	error = EAGAIN;
-fail:
 	sx_sunlock(&proctree_lock);
-	if (ppsratecheck(&lastfail, &curfail, 1))
-		printf("maxproc limit exceeded by uid %u (pid %d); see tuning(7) and login.conf(5)\n",
-		    td->td_ucred->cr_ruid, p1->p_pid);
 	sx_xunlock(&allproc_lock);
 #ifdef MAC
 	mac_proc_destroy(newproc);
@@ -963,6 +966,7 @@ fail2:
 		fdclose(td, fp_procdesc, *procdescp);
 		fdrop(fp_procdesc, td);
 	}
+	atomic_add_int(&nprocs, -1);
 	pause("fork", hz / 2);
 	return (error);
 }



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