Limits broken in 2.4.x kernel.

Limits broken in 2.4.x kernel.

[war]

Problem: Per-user process limits to not work correctly with a 2.4.x
kernel.

Say I want to limit a user to [5] processes.

Example: Edit [/etc/security/limits.conf]
              user            hard         nproc 5
              -or-
              @group       hard         nproc 5

The result: The user cannot login.

How to fix?

Say the total number of user processes is always 100.
This is never the case, however I am showing it only for example.
Currently, you would have to set user or @group to 103, so the user
would be limited to 3 processes.

I was curious if this fix would ever be merged into the Linux Kernel so
limits would actually work properly?


Discussion:

<war> 1] set nproc to 4
<war> 2] try to login -> you cant login if there are more than 4 procs
running on the system
<riel> war: that bug is known
<riel> war: I fixed it a while ago in -ac
<riel> war: the problem is as follows
<riel> 1) login applies the rlimit to itself
<riel> 2) login forks and changes UID
<riel> 3) login exec()s the shell
<riel> of course, step 2) will fail because it set the maximum number of
processes on itself
<war> riel, so in the fixed version, if I set a user/group limit to 3
proc, ten the user will only be able to launch 3 proc, right?
<riel> war: exactly
<war> riel, is this fixed in 2.4.17?
<war> s/is/will
<riel> war: dunno if the fix got carried over from -ac
<war> riel, I would have never been able to say 'damn thats a bug' -
thanks for the information btw.
<riel> war: I tracked it down some time ago and fixed it for Conectiva
7.0's release ;)
<riel> also sent it to Linus and Alan the same day, but of course only
Alan applied it
<war> riel, can you post that information to LKML since you know the
exact problem and see what linus says?
<riel> war: Linus said nothing and won't say anything
<riel> war: as far as I'm concerned the problem is all yours
<riel> feel free to use my info to try to convince Linus
<war> riel, but isn't it considered broken then?
<war> I mean it doesn't work right, heh.
<riel> yes, I consider it broken
<riel> war: but ... if you want to get the fix integrated, you'll have
to do it yourself
<riel> war: I think http://surriel.com/patches/ has the patch somewhere
<riel> war: then you can try to get the thing merged with Linus
<war> riel, btw if you use a group with limits.conf, it should effect
each person in that group individually and not the group as a whole
right?
<riel> war: indeed

Re: Limits broken in 2.4.x kernel.

[Trond Myklebust]

>>>>> " " == war  <war@starband.net> writes:

     > Problem: Per-user process limits to not work correctly with a
     > 2.4.x kernel.

     > Say I want to limit a user to [5] processes.

     > Example: Edit [/etc/security/limits.conf]
     >               user hard nproc 5 -or- @group hard nproc 5

     > The result: The user cannot login.

     > How to fix?

One thing I noticed when doing the BSD cred patch for 2.5.x is that
somebody broke the process accounting in 2.[45].x at least for the
case of reparent_to_init():
If you just charge current->user without moving over the process from
the old uid to the new uid (such as is done in kernel/sys.c with the
set_user() routine) then you risk seriously corrupting the counters.

I'm not sure really what the point was of setting the user in
reparent_to_init() in the first place, since it doesn't setreuid().

Cheers,
  Trond

Re: Limits broken in 2.4.x kernel.

[Andrew Morton]

Trond Myklebust wrote:
> 
> >>>>> " " == war  <war@starband.net> writes:
> 
>      > Problem: Per-user process limits to not work correctly with a
>      > 2.4.x kernel.
> 
>      > Say I want to limit a user to [5] processes.
> 
>      > Example: Edit [/etc/security/limits.conf]
>      >               user hard nproc 5 -or- @group hard nproc 5
> 
>      > The result: The user cannot login.
> 
>      > How to fix?
> 
> One thing I noticed when doing the BSD cred patch for 2.5.x is that
> somebody broke the process accounting in 2.[45].x at least for the
> case of reparent_to_init():

That would be me.

> If you just charge current->user without moving over the process from
> the old uid to the new uid (such as is done in kernel/sys.c with the
> set_user() routine) then you risk seriously corrupting the counters.
>
> I'm not sure really what the point was of setting the user in
> reparent_to_init() in the first place, since it doesn't setreuid().

reparent_to_init() is there to cope with various strange things
which occur when a kernel thread is parented by a userspace process.
It's called after daemonize(), so the thread can no longer participate
in filesystem related things.

I think what you've pointed out here is yet another problem with
the idea of having kernel threads parented by user processes: they
articificially increase the user's process count.

I didn't have a clear reason for moving the UID to root's - it just
didn't seem a good idea to have kernel threads running with non-root
UIDs.   But we have a reason now - process accounting.

reparent_to_init() needs to decrement current->user's processes count,
and increment root's.  I'll do a patch.

-

Re: Limits broken in 2.4.x kernel.

[Trond Myklebust]

>>>>> " " == Andrew Morton <akpm@zip.com.au> writes:

     > reparent_to_init() needs to decrement current->user's processes
     > count, and increment root's.  I'll do a patch.

Please just convert 'set_user()' into a non-static routine. Calling
set_user(0, 1) would do precisely what you want, and the same thing
could then be used for kmod.
There's no real reason for having several different local hacks that
all do the same thing kicking around the place.

Cheers,
  Trond

Re: Limits broken in 2.4.x kernel.

[Alan Cox]

> would be limited to 3 processes.
> 
> I was curious if this fix would ever be merged into the Linux Kernel so
> limits would actually work properly?

Linus kept rejecting it. Now we have Marcelo as 2.4.x maintainer I'll
look at submitting it. 2.5 will no doubt stay broken for a while.

Re: Limits broken in 2.4.x kernel.

[Rik van Riel]

On Tue, 18 Dec 2001, Alan Cox wrote:

> > would be limited to 3 processes.
> >
> > I was curious if this fix would ever be merged into the Linux Kernel so
> > limits would actually work properly?
>
> Linus kept rejecting it. Now we have Marcelo as 2.4.x maintainer I'll
> look at submitting it. 2.5 will no doubt stay broken for a while.

One of the things to remember for when marcelo takes over
2.6, I guess ;)

Rik
-- 
DMCA, SSSCA, W3C?  Who cares?  http://thefreeworld.net/

http://www.surriel.com/		http://distro.conectiva.com/

Re: Limits broken in 2.4.x kernel.

[Alan Cox]

> > Linus kept rejecting it. Now we have Marcelo as 2.4.x maintainer I'll
> > look at submitting it. 2.5 will no doubt stay broken for a while.
> 
> One of the things to remember for when marcelo takes over
> 2.6, I guess ;)

Not what I meant - process counting is not block I/O stuff

Re: Limits broken in 2.4.x kernel.

[Andrew Morton]

Trond Myklebust wrote:
> 
> >>>>> " " == Andrew Morton <akpm@zip.com.au> writes:
> 
>      > reparent_to_init() needs to decrement current->user's processes
>      > count, and increment root's.  I'll do a patch.
> 
> Please just convert 'set_user()' into a non-static routine. Calling
> set_user(0, 1) would do precisely what you want, and the same thing
> could then be used for kmod.
> There's no real reason for having several different local hacks that
> all do the same thing kicking around the place.
> 

I bet you thought I'd forgotten.

- Make set_user() non-static

- Convert set_user() to use cached copy of `current'

- Fix world's tiniest SMP race in set_user() - we should
  increment usage count on the old struct before decrementing the
  count on the new one - they may be the same.

- change exec_usermodehelper() to use set_user()

- change reparent_to_init() to use set_user() - fixes possible
  error in user process accounting.

It is all tested.



--- linux-2.4.18-pre7/include/linux/sched.h	Fri Dec 21 11:19:23 2001
+++ linux-akpm/include/linux/sched.h	Tue Jan 29 00:04:58 2002
@@ -150,6 +150,7 @@ extern void trap_init(void);
 extern void update_process_times(int user);
 extern void update_one_process(struct task_struct *p, unsigned long user,
 			       unsigned long system, int cpu);
+extern int set_user(uid_t new_ruid, int dumpclear);
 
 #define	MAX_SCHEDULE_TIMEOUT	LONG_MAX
 extern signed long FASTCALL(schedule_timeout(signed long timeout));
--- linux-2.4.18-pre7/kernel/sys.c	Wed Jan 23 15:11:35 2002
+++ linux-akpm/kernel/sys.c	Tue Jan 29 00:07:02 2002
@@ -490,9 +490,10 @@ static inline void cap_emulate_setxuid(i
 	}
 }
 
-static int set_user(uid_t new_ruid, int dumpclear)
+int set_user(uid_t new_ruid, int dumpclear)
 {
 	struct user_struct *new_user, *old_user;
+	struct task_struct *this_task = current;
 
 	/* What if a process setreuid()'s and this brings the
 	 * new uid over his NPROC rlimit?  We can check this now
@@ -502,17 +503,16 @@ static int set_user(uid_t new_ruid, int 
 	new_user = alloc_uid(new_ruid);
 	if (!new_user)
 		return -EAGAIN;
-	old_user = current->user;
-	atomic_dec(&old_user->processes);
+	old_user = this_task->user;
 	atomic_inc(&new_user->processes);
+	atomic_dec(&old_user->processes);
 
-	if(dumpclear)
-	{
-		current->mm->dumpable = 0;
+	if (dumpclear && this_task->mm) {
+		this_task->mm->dumpable = 0;
 		wmb();
 	}
-	current->uid = new_ruid;
-	current->user = new_user;
+	this_task->uid = new_ruid;
+	this_task->user = new_user;
 	free_uid(old_user);
 	return 0;
 }
--- linux-2.4.18-pre7/kernel/sched.c	Fri Dec 21 11:19:23 2001
+++ linux-akpm/kernel/sched.c	Tue Jan 29 00:04:58 2002
@@ -1259,7 +1259,8 @@ void reparent_to_init(void)
 	this_task->cap_permitted = CAP_FULL_SET;
 	this_task->keep_capabilities = 0;
 	memcpy(this_task->rlim, init_task.rlim, sizeof(*(this_task->rlim)));
-	this_task->user = INIT_USER;
+	/* Become root */
+	set_user(0, 0);
 
 	spin_unlock(&runqueue_lock);
 	write_unlock_irq(&tasklist_lock);
--- linux-2.4.18-pre7/kernel/kmod.c	Tue Jul 17 18:23:50 2001
+++ linux-akpm/kernel/kmod.c	Tue Jan 29 00:04:58 2002
@@ -111,15 +111,8 @@ int exec_usermodehelper(char *program_pa
 		if (curtask->files->fd[i]) close(i);
 	}
 
-	/* Drop the "current user" thing */
-	{
-		struct user_struct *user = curtask->user;
-		curtask->user = INIT_USER;
-		atomic_inc(&INIT_USER->__count);
-		atomic_inc(&INIT_USER->processes);
-		atomic_dec(&user->processes);
-		free_uid(user);
-	}
+	/* Become root */
+	set_user(0, 1);
 
 	/* Give kmod all effective privileges.. */
 	curtask->euid = curtask->fsuid = 0;