mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* CPU time limit patch / setrlimit(RLIMIT_CPU, 0) cheat fix
@ 2007-04-17 13:57 Tom Alsberg
  2007-04-20  7:45 ` Andrew Morton
  0 siblings, 1 reply; 2+ messages in thread
From: Tom Alsberg @ 2007-04-17 13:57 UTC (permalink / raw)
  To: Linux-Kernel Mailing List

[-- Attachment #1: Type: text/plain, Size: 2092 bytes --]

Hi there.

As discovered here today, the change in Kernel 2.6.17 intended to
inhibit users from setting RLIMIT_CPU to 0 (as that is equivalent to
unlimited) by "cheating" and setting it to 1 in such a case, does not
make a difference, as the check is done in the wrong place (too late),
and only applies to the profiling code.

On all systems I checked running kernels above 2.6.17, no matter what
the hard and soft CPU time limits were before, a user could escape
them by issuing in the shell (sh/bash/zsh) "ulimit -t 0", and then the
user's process was not ever killed.

Attached is a trivial patch to fix that.  Simply moving the check to a
slightly earlier location (specifically, before the line that actually
assigns the limit - *old_rlim = new_rlim), does the trick.

Do note that at least the zsh (but not ash, dash, or bash) shell has
the problem of "caching" the limits set by the ulimit command, so when
running zsh the fix will not immediately be evident - after entering
"ulimit -t 0", "ulimit -a" will show "-t: cpu time (seconds) 0", even
though the actual limit as returned by getrlimit(...) will be 1.  It
can be verified by opening a subshell (which will not have the values
of the parent shell in cache) and checking in it, or just by running a
CPU intensive command like "echo '65536^1048576' | bc" and verifying
that it dumps core after one second.

Regardless of whether that is a misfeature in the shell, perhaps it
would be better to return -EINVAL from setrlimit in such a case
instead of cheating and setting to 1, as that does not really reflect
the actual state of the process anymore.  I do not however know what
the ground for that decision was in the original 2.6.17 change, and
whether there would be any "backward" compatibility issues, so I
preferred not to touch that right now.

  Cheers,
  -- Tom

-- 
  Tom Alsberg - hacker (being the best description fitting this space)
  Web page:	http://www.cs.huji.ac.il/~alsbergt/
DISCLAIMER:  The above message does not even necessarily represent what
my fingers have typed on the keyboard, save anything further.

[-- Attachment #2: linux-2.6.20.3-cpulimit.diff --]
[-- Type: text/x-diff, Size: 1186 bytes --]

Follows a trivial patch to check for RLIMIT_CPU to 0 in the right place.

diff -urN linux-2.6.20.3.orig/kernel/sys.c linux-2.6.20.3/kernel/sys.c
--- linux-2.6.20.3.orig/kernel/sys.c	2007-03-13 20:27:08.000000000 +0200
+++ linux-2.6.20.3/kernel/sys.c	2007-04-17 16:38:51.651236000 +0300
@@ -1916,6 +1916,16 @@
 	if (retval)
 		return retval;
 
+	if (resource == RLIMIT_CPU && new_rlim.rlim_cur == 0) {
+		/*
+		 * The caller is asking for an immediate RLIMIT_CPU
+		 * expiry.  But we use the zero value to mean "it was
+		 * never set".  So let's cheat and make it one second
+		 * instead
+		 */
+		new_rlim.rlim_cur = 1;
+	}
+
 	task_lock(current->group_leader);
 	*old_rlim = new_rlim;
 	task_unlock(current->group_leader);
@@ -1937,15 +1947,6 @@
 		unsigned long rlim_cur = new_rlim.rlim_cur;
 		cputime_t cputime;
 
-		if (rlim_cur == 0) {
-			/*
-			 * The caller is asking for an immediate RLIMIT_CPU
-			 * expiry.  But we use the zero value to mean "it was
-			 * never set".  So let's cheat and make it one second
-			 * instead
-			 */
-			rlim_cur = 1;
-		}
 		cputime = secs_to_cputime(rlim_cur);
 		read_lock(&tasklist_lock);
 		spin_lock_irq(&current->sighand->siglock);

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: CPU time limit patch / setrlimit(RLIMIT_CPU, 0) cheat fix
  2007-04-17 13:57 CPU time limit patch / setrlimit(RLIMIT_CPU, 0) cheat fix Tom Alsberg
@ 2007-04-20  7:45 ` Andrew Morton
  0 siblings, 0 replies; 2+ messages in thread
From: Andrew Morton @ 2007-04-20  7:45 UTC (permalink / raw)
  To: Tom Alsberg; +Cc: Linux-Kernel Mailing List

On Tue, 17 Apr 2007 16:57:55 +0300 Tom Alsberg <alsbergt@cs.huji.ac.il> wrote:

> Hi there.
> 
> As discovered here today, the change in Kernel 2.6.17 intended to
> inhibit users from setting RLIMIT_CPU to 0 (as that is equivalent to
> unlimited) by "cheating" and setting it to 1 in such a case, does not
> make a difference, as the check is done in the wrong place (too late),
> and only applies to the profiling code.

hm, I tested that.  I guess I must have only tested the profiling limit.

> On all systems I checked running kernels above 2.6.17, no matter what
> the hard and soft CPU time limits were before, a user could escape
> them by issuing in the shell (sh/bash/zsh) "ulimit -t 0", and then the
> user's process was not ever killed.
> 
> Attached is a trivial patch to fix that.  Simply moving the check to a
> slightly earlier location (specifically, before the line that actually
> assigns the limit - *old_rlim = new_rlim), does the trick.
> 
> Do note that at least the zsh (but not ash, dash, or bash) shell has
> the problem of "caching" the limits set by the ulimit command, so when
> running zsh the fix will not immediately be evident - after entering
> "ulimit -t 0", "ulimit -a" will show "-t: cpu time (seconds) 0", even
> though the actual limit as returned by getrlimit(...) will be 1.  It
> can be verified by opening a subshell (which will not have the values
> of the parent shell in cache) and checking in it, or just by running a
> CPU intensive command like "echo '65536^1048576' | bc" and verifying
> that it dumps core after one second.
> 
> Regardless of whether that is a misfeature in the shell, perhaps it
> would be better to return -EINVAL from setrlimit in such a case
> instead of cheating and setting to 1, as that does not really reflect
> the actual state of the process anymore.  I do not however know what
> the ground for that decision was in the original 2.6.17 change, and
> whether there would be any "backward" compatibility issues, so I
> preferred not to touch that right now.

The 2.6.17 changes was done that way allegedly for 2.4 compatibility.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2007-04-20  7:46 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-04-17 13:57 CPU time limit patch / setrlimit(RLIMIT_CPU, 0) cheat fix Tom Alsberg
2007-04-20  7:45 ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome