It is possible on large memory systems that the default process limits
can exceed PID_MAX. This will allow a non-root user to consume all pids
resulting in the kernel to basically hang in get_pid().
The following patch to fork_init() will prevent max_threads from exceeding
PID_MAX.
Pat
--- linux-2.4.19-pre2-x/kernel/fork.c Mon Feb 25 14:38:13 2002
+++ linux-2.4.19-pre2/kernel/fork.c Wed Mar 6 09:15:17 2002
@@ -74,6 +74,11 @@
*/
max_threads = mempages / (THREAD_SIZE/PAGE_SIZE) / 8;
+ /* don't let threads go beyond PID_MAX */
+ if (max_threads > PID_MAX) {
+ max_threads = PID_MAX;
+ }
+
init_task.rlim[RLIMIT_NPROC].rlim_cur = max_threads/2;
init_task.rlim[RLIMIT_NPROC].rlim_max = max_threads/2;
}
On Thu, 2002-03-07 at 14:45, Patrick O'Rourke wrote:
> It is possible on large memory systems that the default process limits
> can exceed PID_MAX. This will allow a non-root user to consume all pids
> resulting in the kernel to basically hang in get_pid().
>
> + /* don't let threads go beyond PID_MAX */
> + if (max_threads > PID_MAX) {
> + max_threads = PID_MAX;
> + }
> +
The problem with this approach is that it doesn't take into account
pgrp, tgids, etc... I submitted the following patch a couple of weeks
ago that fixes the problem a better way.
Thanks,
Paul Larson
diff -Naur linux-2.4.18-rc2/kernel/fork.c linux-getpid/kernel/fork.c
--- linux-2.4.18-rc2/kernel/fork.c Wed Feb 20 09:54:39 2002
+++ linux-getpid/kernel/fork.c Fri Feb 22 15:52:52 2002
@@ -20,6 +20,7 @@
#include <linux/vmalloc.h>
#include <linux/completion.h>
#include <linux/personality.h>
+#include <linux/compiler.h>
#include <asm/pgtable.h>
#include <asm/pgalloc.h>
@@ -85,12 +86,13 @@
{
static int next_safe = PID_MAX;
struct task_struct *p;
- int pid;
+ int pid, beginpid;
if (flags & CLONE_PID)
return current->pid;
spin_lock(&lastpid_lock);
+ beginpid = last_pid;
if((++last_pid) & 0xffff8000) {
last_pid = 300; /* Skip daemons etc. */
goto inside;
@@ -110,12 +112,16 @@
last_pid = 300;
next_safe = PID_MAX;
}
+ if(unlikely(last_pid == beginpid))
+ goto nomorepids;
goto repeat;
}
if(p->pid > last_pid && next_safe > p->pid)
next_safe = p->pid;
if(p->pgrp > last_pid && next_safe > p->pgrp)
next_safe = p->pgrp;
+ if(p->tgid > last_pid && next_safe > p->tgid)
+ next_safe = p->tgid;
if(p->session > last_pid && next_safe > p->session)
next_safe = p->session;
}
@@ -125,6 +131,11 @@
spin_unlock(&lastpid_lock);
return pid;
+
+nomorepids:
+ read_unlock(&tasklist_lock);
+ spin_unlock(&lastpid_lock);
+ return 0;
}
static inline int dup_mmap(struct mm_struct * mm)
@@ -620,6 +631,8 @@
copy_flags(clone_flags, p);
p->pid = get_pid(clone_flags);
+ if (p->pid == 0 && current->pid != 0)
+ goto bad_fork_cleanup;
p->run_list.next = NULL;
p->run_list.prev = NULL;
On Thu, 2002-03-07 at 16:11, Paul Larson wrote:
> On Thu, 2002-03-07 at 14:45, Patrick O'Rourke wrote:
> > It is possible on large memory systems that the default process limits
> > can exceed PID_MAX. This will allow a non-root user to consume all pids
> > resulting in the kernel to basically hang in get_pid().
> >
>
> > + /* don't let threads go beyond PID_MAX */
> > + if (max_threads > PID_MAX) {
> > + max_threads = PID_MAX;
> > + }
> > +
> The problem with this approach is that it doesn't take into account
> pgrp, tgids, etc... I submitted the following patch a couple of weeks
> ago that fixes the problem a better way.
>
> Thanks,
> Paul Larson
Marcelo, any chance of getting this accepted into the next 2.4.19-pre?
It is obviously a bug that is afecting several others as well. I think
the LSE team is looking at some performance enhancements to get_pid, but
from what I've seen so far they will be working on top of this bug fix.
I just verified that the patch still applies cleanly against
2.4.19-pre2.
Thanks,
Paul Larson
Ok, this time with the patch for real. :)
On Thu, 2002-03-07 at 16:11, Paul Larson wrote:
> On Thu, 2002-03-07 at 14:45, Patrick O'Rourke wrote:
> > It is possible on large memory systems that the default process limits
> > can exceed PID_MAX. This will allow a non-root user to consume all pids
> > resulting in the kernel to basically hang in get_pid().
> >
>
> > + /* don't let threads go beyond PID_MAX */
> > + if (max_threads > PID_MAX) {
> > + max_threads = PID_MAX;
> > + }
> > +
> The problem with this approach is that it doesn't take into account
> pgrp, tgids, etc... I submitted the following patch a couple of weeks
> ago that fixes the problem a better way.
>
> Thanks,
> Paul Larson
Marcelo, any chance of getting this accepted into the next 2.4.19-pre?
It is obviously a bug that is afecting several others as well. I think
the LSE team is looking at some performance enhancements to get_pid, but
from what I've seen so far they will be working on top of this bug fix.
I just verified that the patch still applies cleanly against
2.4.19-pre2.
Thanks,
Paul Larson
diff -Naur linux-2.4.18-rc2/kernel/fork.c linux-getpid/kernel/fork.c
--- linux-2.4.18-rc2/kernel/fork.c Wed Feb 20 09:54:39 2002
+++ linux-getpid/kernel/fork.c Fri Feb 22 15:52:52 2002
@@ -20,6 +20,7 @@
#include <linux/vmalloc.h>
#include <linux/completion.h>
#include <linux/personality.h>
+#include <linux/compiler.h>
#include <asm/pgtable.h>
#include <asm/pgalloc.h>
@@ -85,12 +86,13 @@
{
static int next_safe = PID_MAX;
struct task_struct *p;
- int pid;
+ int pid, beginpid;
if (flags & CLONE_PID)
return current->pid;
spin_lock(&lastpid_lock);
+ beginpid = last_pid;
if((++last_pid) & 0xffff8000) {
last_pid = 300; /* Skip daemons etc. */
goto inside;
@@ -110,12 +112,16 @@
last_pid = 300;
next_safe = PID_MAX;
}
+ if(unlikely(last_pid == beginpid))
+ goto nomorepids;
goto repeat;
}
if(p->pid > last_pid && next_safe > p->pid)
next_safe = p->pid;
if(p->pgrp > last_pid && next_safe > p->pgrp)
next_safe = p->pgrp;
+ if(p->tgid > last_pid && next_safe > p->tgid)
+ next_safe = p->tgid;
if(p->session > last_pid && next_safe > p->session)
next_safe = p->session;
}
@@ -125,6 +131,11 @@
spin_unlock(&lastpid_lock);
return pid;
+
+nomorepids:
+ read_unlock(&tasklist_lock);
+ spin_unlock(&lastpid_lock);
+ return 0;
}
static inline int dup_mmap(struct mm_struct * mm)
@@ -620,6 +631,8 @@
copy_flags(clone_flags, p);
p->pid = get_pid(clone_flags);
+ if (p->pid == 0 && current->pid != 0)
+ goto bad_fork_cleanup;
p->run_list.next = NULL;
p->run_list.prev = NULL;
Paul Larson wrote:
> Ok, this time with the patch for real. :)
While I agree with your patch, I still think something should be
done in fork_init() to restrict the initial value of max_threads.
We should not rely on the amount of memory alone.
For example, after a fresh reboot on an i386 system with 12gb
of ram we get:
[joeuser@simpsons-lisa joeuser]$ id
uid=500(joeuser) gid=500(joeuser) groups=500(joeuser)
[joeuser@simpsons-lisa joeuser]$ ulimit -a
core file size (blocks) 1000000
data seg size (kbytes) unlimited
file size (blocks) unlimited
max locked memory (kbytes) unlimited
max memory size (kbytes) unlimited
open files 1024
pipe size (512 bytes) 8
stack size (kbytes) 8192
cpu time (seconds) unlimited
max user processes 49152
virtual memory (kbytes) unlimited
[joeuser@simpsons-lisa joeuser]$
I believe it is wrong for max user processes to start off above PID_MAX.
Pat
--
Patrick O'Rourke
[email protected]