Why is RLIMIT_NPROC apllied to root(uid 0) processes? It's not kernel job to
prevent admin from shooting him/her self in the foot.
root should be able to do fork() regardless of any limits,
and IMHO the following patch is the right thing.
--- linux/kernel/fork.c~ Tue Sep 5 23:48:59 2000
+++ linux/kernel/fork.c Sun Nov 26 20:22:20 2000
@@ -560,7 +560,8 @@
*p = *current;
retval = -EAGAIN;
- if (atomic_read(&p->user->processes) >= p->rlim[RLIMIT_NPROC].rlim_cur)
+ if (p->user->uid &&
+ (atomic_read(&p->user->processes) >= p->rlim[RLIMIT_NPROC].rlim_cur))
goto bad_fork_free;
atomic_inc(&p->user->__count);
atomic_inc(&p->user->processes);
Jan
--
Jan R?korajski | ALL SUSPECTS ARE GUILTY. PERIOD!
baggins<at>mimuw.edu.pl | OTHERWISE THEY WOULDN'T BE SUSPECTS, WOULD THEY?
BOFH, type MANIAC | -- TROOPS by Kevin Rubio
On Tue, 28 Nov 2000, Jan Rekorajski wrote:
> --- linux/kernel/fork.c~ Tue Sep 5 23:48:59 2000
> +++ linux/kernel/fork.c Sun Nov 26 20:22:20 2000
> @@ -560,7 +560,8 @@
> *p = *current;
>
> retval = -EAGAIN;
> - if (atomic_read(&p->user->processes) >= p->rlim[RLIMIT_NPROC].rlim_cur)
> + if (p->user->uid &&
> + (atomic_read(&p->user->processes) >= p->rlim[RLIMIT_NPROC].rlim_cur))
Jan,
Hardcoding things signifying special treatment of uid=0 is almost always a
bad idea. If you _really_ think that superuser (whatever entity that might
be) should be exempt from RLIMIT_NPROC and can prove that (SuSv2 seems to
be silent so you may be right), then you should use capable() to do proper
capability test and not that horrible explicit uid test as in your patch
above.
Regards,
Tigran
> On Tue, 28 Nov 2000, Jan Rekorajski wrote:
> > --- linux/kernel/fork.c~ Tue Sep 5 23:48:59 2000
> > +++ linux/kernel/fork.c Sun Nov 26 20:22:20 2000
> > @@ -560,7 +560,8 @@
> > *p = *current;
> >
> > retval = -EAGAIN;
> > - if (atomic_read(&p->user->processes) >= p->rlim[RLIMIT_NPROC].rlim_cur)
> > + if (p->user->uid &&
> > + (atomic_read(&p->user->processes) >= p->rlim[RLIMIT_NPROC].rlim_cur))
>
On a totally unrelated to the resourcesnote, for your benefit, Jan:
the ->user->uid is maintained for a reason completely different from your
usage above (see kernel/user.c to learn why -- it is to easily find out
given user_struct which real uid it belongs to in uid_hash_find()). If you
wanted this process' uid you should have used p->uid. If you wanted this
process' effective uid (more likely) then you should have used p->euid.
Regards,
Tigran
Jan R_korajski writes:
> Why is RLIMIT_NPROC apllied to root(uid 0) processes? It's not kernel job to
> prevent admin from shooting him/her self in the foot.
> - if (atomic_read(&p->user->processes) >= p->rlim[RLIMIT_NPROC].rlim_cur)
By default, root has no real process limits anyways, so this test should
always succeed. However, it would be nice to be _able_ to set process
limits on root for one reason or another. Also, as we move towards more
secure systems, it is bad (IMHO) to special case root (uid=0) cases.
It just makes more to fix to get a system where root != god.
> root should be able to do fork() regardless of any limits,
> and IMHO the following patch is the right thing.
Then set the rlim_cur to unlimited, and blow your system up as you like.
Cheers, Andreas
--
Andreas Dilger \ "If a man ate a pound of pasta and a pound of antipasto,
\ would they cancel out, leaving him still hungry?"
http://www-mddsp.enel.ucalgary.ca/People/adilger/ -- Dogbert
On Tue, 28 Nov 2000, Tigran Aivazian wrote:
> On Tue, 28 Nov 2000, Jan Rekorajski wrote:
> > --- linux/kernel/fork.c~ Tue Sep 5 23:48:59 2000
> > +++ linux/kernel/fork.c Sun Nov 26 20:22:20 2000
> > @@ -560,7 +560,8 @@
> > *p = *current;
> >
> > retval = -EAGAIN;
> > - if (atomic_read(&p->user->processes) >= p->rlim[RLIMIT_NPROC].rlim_cur)
> > + if (p->user->uid &&
> > + (atomic_read(&p->user->processes) >= p->rlim[RLIMIT_NPROC].rlim_cur))
>
> Jan,
>
> Hardcoding things signifying special treatment of uid=0 is almost always a
> bad idea. If you _really_ think that superuser (whatever entity that might
> be) should be exempt from RLIMIT_NPROC and can prove that (SuSv2 seems to
> be silent so you may be right), then you should use capable() to do proper
> capability test and not that horrible explicit uid test as in your patch
> above.
Ok, how about setting limits on login? When this looks like:
--- uid = 0 here
setrlimit(RLIMIT_NPROC, n)
fork() <- this will fail if root has >n processes
setuid(user)
and it is hard to change this sequence, all PAM enabled apps depend on it :(
The other scenario may be when root has already n processes,
call them system stuff, and cannot login or even kill anything
because of limits.
Is this patch better? I put CAP_SYS_RESOURCE, but maybe it should be
CAP_SYS_ADMIN.
--- linux/kernel/fork.c~ Fri Nov 17 15:09:26 2000
+++ linux/kernel/fork.c Tue Nov 28 22:02:17 2000
@@ -562,7 +562,8 @@
*p = *current;
retval = -EAGAIN;
- if (atomic_read(&p->user->processes) >= p->rlim[RLIMIT_NPROC].rlim_cur)
+ if ((atomic_read(&p->user->processes) >= p->rlim[RLIMIT_NPROC].rlim_cur) &&
+ !capable(CAP_SYS_RESOURCE))
goto bad_fork_free;
atomic_inc(&p->user->__count);
atomic_inc(&p->user->processes);
Jan
--
Jan R?korajski | ALL SUSPECTS ARE GUILTY. PERIOD!
baggins<at>mimuw.edu.pl | OTHERWISE THEY WOULDN'T BE SUSPECTS, WOULD THEY?
BOFH, type MANIAC | -- TROOPS by Kevin Rubio
Jan Rekorajski <[email protected]> writes:
|> Why is RLIMIT_NPROC apllied to root(uid 0) processes? It's not kernel job to
|> prevent admin from shooting him/her self in the foot.
|>
|> root should be able to do fork() regardless of any limits,
|> and IMHO the following patch is the right thing.
AFAICS, _all_ resource limits are equally applied to root processes. Why
should NPROC be different?
Andreas.
--
Andreas Schwab "And now for something
SuSE Labs completely different."
[email protected]
SuSE GmbH, Schanz?ckerstr. 10, D-90443 N?rnberg
On Tue, 28 Nov 2000, Andreas Schwab wrote:
> Jan Rekorajski <[email protected]> writes:
>
> |> Why is RLIMIT_NPROC apllied to root(uid 0) processes? It's not kernel job to
> |> prevent admin from shooting him/her self in the foot.
> |>
> |> root should be able to do fork() regardless of any limits,
> |> and IMHO the following patch is the right thing.
>
> AFAICS, _all_ resource limits are equally applied to root processes. Why
> should NPROC be different?
Because you want to be able to `kill <pid>`?
And if you are over-limits you can't?
Jan
--
Jan R?korajski | ALL SUSPECTS ARE GUILTY. PERIOD!
baggins<at>mimuw.edu.pl | OTHERWISE THEY WOULDN'T BE SUSPECTS, WOULD THEY?
BOFH, type MANIAC | -- TROOPS by Kevin Rubio
> Why is RLIMIT_NPROC apllied to root(uid 0) processes? It's not kernel j=
> ob to
> prevent admin from shooting him/her self in the foot.
>
> root should be able to do fork() regardless of any limits,
> and IMHO the following patch is the right thing.
This patch is bogus. root can always raise their limit. But having root
tasks by default not take out the box is good
> > AFAICS, _all_ resource limits are equally applied to root processes. =
> Why
> > should NPROC be different?
>
> Because you want to be able to `kill <pid>`?
> And if you are over-limits you can't?
Wrong. limit is a shell built in
On Tue, Nov 28, 2000 at 09:58:14PM +0000, Alan Cox wrote:
> > Because you want to be able to `kill <pid>`?
> > And if you are over-limits you can't?
> Wrong. limit is a shell built in
I assume you mean kill is a shell builtin. Depending on your shell. :-).
It's still a real pain when you want to get the pid of the offending
proces(ses). You could of course do something like
for a in /proc/*; do echo -en "$a "; cat $a/cmdline; echo; done (it'll
barf a lot, but give a reasonable picture)...
Anyways, this is all not relevant, imho the whole point is moot.
"I don't like root having rlimits."
"So don't setrlimit root."
No reason to ditch functionality.
--
Frank v Waveren
fvw@[var.cx|stack.nl|chello.nl|dse.nl]
ICQ# 10074100
In article <[email protected]>,
Frank v Waveren <[email protected]> wrote:
>On Tue, Nov 28, 2000 at 09:58:14PM +0000, Alan Cox wrote:
>> > Because you want to be able to `kill <pid>`?
>> > And if you are over-limits you can't?
>> Wrong. limit is a shell built in
>
>I assume you mean kill is a shell builtin. Depending on your shell. :-).
No. Think about it.
Mike.
On Tue, 28 Nov 2000, Alan Cox wrote:
> > Why is RLIMIT_NPROC apllied to root(uid 0) processes? It's not kernel j=
> > ob to
> > prevent admin from shooting him/her self in the foot.
> >
> > root should be able to do fork() regardless of any limits,
> > and IMHO the following patch is the right thing.
>
> This patch is bogus. root can always raise their limit. But having root
> tasks by default not take out the box is good
OK, I just fix applications that assume root = no limits ;)
Jan
--
Jan R?korajski | ALL SUSPECTS ARE GUILTY. PERIOD!
baggins<at>mimuw.edu.pl | OTHERWISE THEY WOULDN'T BE SUSPECTS, WOULD THEY?
BOFH, type MANIAC | -- TROOPS by Kevin Rubio
Hi!
> > On Tue, 28 Nov 2000, Jan Rekorajski wrote:
> > > --- linux/kernel/fork.c~ Tue Sep 5 23:48:59 2000
> > > +++ linux/kernel/fork.c Sun Nov 26 20:22:20 2000
> > > @@ -560,7 +560,8 @@
> > > *p = *current;
> > >
> > > retval = -EAGAIN;
> > > - if (atomic_read(&p->user->processes) >= p->rlim[RLIMIT_NPROC].rlim_cur)
> > > + if (p->user->uid &&
> > > + (atomic_read(&p->user->processes) >= p->rlim[RLIMIT_NPROC].rlim_cur))
> >
> > Jan,
> >
> > Hardcoding things signifying special treatment of uid=0 is almost always a
> > bad idea. If you _really_ think that superuser (whatever entity that might
> > be) should be exempt from RLIMIT_NPROC and can prove that (SuSv2 seems to
> > be silent so you may be right), then you should use capable() to do proper
> > capability test and not that horrible explicit uid test as in your patch
> > above.
>
> Ok, how about setting limits on login? When this looks like:
>
> --- uid = 0 here
> setrlimit(RLIMIT_NPROC, n)
> fork() <- this will fail if root has >n processes
> setuid(user)
>
> and it is hard to change this sequence, all PAM enabled apps depend
> on it :(
So PAM dictates kernel changes? Fix pam, do not break kernel.
Pavel
--
I'm [email protected]. "In my country we have almost anarchy and I don't care."
Panos Katsaloulis describing me w.r.t. patents at [email protected]
On Thu, 30 Nov 2000, Pavel Machek wrote:
> Hi!
>
> > > On Tue, 28 Nov 2000, Jan Rekorajski wrote:
> > > > --- linux/kernel/fork.c~ Tue Sep 5 23:48:59 2000
> > > > +++ linux/kernel/fork.c Sun Nov 26 20:22:20 2000
> > > > @@ -560,7 +560,8 @@
> > > > *p = *current;
> > > >
> > > > retval = -EAGAIN;
> > > > - if (atomic_read(&p->user->processes) >= p->rlim[RLIMIT_NPROC].rlim_cur)
> > > > + if (p->user->uid &&
> > > > + (atomic_read(&p->user->processes) >= p->rlim[RLIMIT_NPROC].rlim_cur))
> > >
> > > Jan,
> > >
> > > Hardcoding things signifying special treatment of uid=0 is almost always a
> > > bad idea. If you _really_ think that superuser (whatever entity that might
> > > be) should be exempt from RLIMIT_NPROC and can prove that (SuSv2 seems to
> > > be silent so you may be right), then you should use capable() to do proper
> > > capability test and not that horrible explicit uid test as in your patch
> > > above.
> >
> > Ok, how about setting limits on login? When this looks like:
> >
> > --- uid = 0 here
> > setrlimit(RLIMIT_NPROC, n)
> > fork() <- this will fail if root has >n processes
> > setuid(user)
> >
> > and it is hard to change this sequence, all PAM enabled apps depend
> > on it :(
>
> So PAM dictates kernel changes? Fix pam, do not break kernel.
Fixed :)
Jan
--
Jan R?korajski | ALL SUSPECTS ARE GUILTY. PERIOD!
baggins<at>mimuw.edu.pl | OTHERWISE THEY WOULDN'T BE SUSPECTS, WOULD THEY?
BOFH, type MANIAC | -- TROOPS by Kevin Rubio
> > > Hardcoding things signifying special treatment of uid=0 is almost always a
> > > bad idea. If you _really_ think that superuser (whatever entity that might
> > > be) should be exempt from RLIMIT_NPROC and can prove that (SuSv2 seems to
> > > be silent so you may be right), then you should use capable() to do proper
> > > capability test and not that horrible explicit uid test as in your patch
> > > above.
I totally agree with you, Pavel. But while we are on this subject --
shouldn't the explicit check like this:
/*
* Use a reserved one if we're the superuser
*/
if (files_stat.nr_free_files && !current->euid)
goto used_one;
in fs/file_table.c:get_empty_filp() be switched to capabilities? I.e. is
the hardcoded euid=0 value intentional there or is it an omission?
Regards,
Tigran