[PATCH] no RLIMIT_NPROC for root, please

[PATCH] no RLIMIT_NPROC for root, please

[Jan Rekorajski]

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] no RLIMIT_NPROC for root, please

[Tigran Aivazian]

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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] no RLIMIT_NPROC for root, please

[Tigran Aivazian]

> 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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] no RLIMIT_NPROC for root, please

[Andreas Dilger]

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] no RLIMIT_NPROC for root, please

[Andreas Schwab]

Jan Rekorajski <baggins@sith.mimuw.edu.pl> 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."
Andreas.Schwab@suse.de
SuSE GmbH, Schanzäckerstr. 10, D-90443 Nürnberg
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] no RLIMIT_NPROC for root, please

[Jan Rekorajski]

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] no RLIMIT_NPROC for root, please

[Jan Rekorajski]

On Tue, 28 Nov 2000, Andreas Schwab wrote:

> Jan Rekorajski <baggins@sith.mimuw.edu.pl> 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] no RLIMIT_NPROC for root, please

[Alan Cox]

> 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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] no RLIMIT_NPROC for root, please

[Alan Cox]

> > 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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] no RLIMIT_NPROC for root, please

[Frank v Waveren]

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] no RLIMIT_NPROC for root, please

[Miquel van Smoorenburg]

In article <20001128231334.A438@var.cx>,
Frank v Waveren  <fvw@var.cx> 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.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] no RLIMIT_NPROC for root, please

[Jan Rekorajski]

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] no RLIMIT_NPROC for root, please

[Pavel Machek]

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 pavel@ucw.cz. "In my country we have almost anarchy and I don't care."
Panos Katsaloulis describing me w.r.t. patents at discuss@linmodems.org
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] no RLIMIT_NPROC for root, please

[Jan Rekorajski]

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] no RLIMIT_NPROC for root, please

[Tigran Aivazian]

> > > 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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/