[TRIVIAL] Re: setrlimit incorrectly allows hard limits to exceed soft limits

[TRIVIAL] Re: setrlimit incorrectly allows hard limits to exceed soft limits

[Rusty Trivial Russell]

From:  Kingsley Cheung <kingsley@aurema.com>

  Oops, should be after the copy :-(
  

> In 2.4.19 (also 2.5.46) setrlimit code only ever makes a comparison to
> check the old soft limit with the new soft limit and the new hard
> limit with the old hard limit.  There is never a check to ensure the
> new soft limit never exceeds the new hard limit. 
> 
> Just try "ulimit -H -m 10000" for memory limits that were not
> previously set.  You end up with (hard limit = 10000) < (soft limit =
> unlimited).
> 
> Fix is trivial.


--- trivial-2.5-bk/kernel/sys.c.orig	2002-12-06 13:56:43.000000000 +1100
+++ trivial-2.5-bk/kernel/sys.c	2002-12-06 13:56:43.000000000 +1100
@@ -1233,6 +1233,8 @@
 		return -EINVAL;
 	if(copy_from_user(&new_rlim, rlim, sizeof(*rlim)))
 		return -EFAULT;
+       if (new_rlim.rlim_cur > new_rlim.rlim_max)
+               return -EINVAL;
 	old_rlim = current->rlim + resource;
 	if (((new_rlim.rlim_cur > old_rlim->rlim_max) ||
 	     (new_rlim.rlim_max > old_rlim->rlim_max)) &&
-- 
  Don't blame me: the Monkey is driving
  File: Kingsley Cheung <kingsley@aurema.com>: Re: [PATCH] setrlimit incorrectly allows hard limits to exceed soft limits

Re: [TRIVIAL] Re: setrlimit incorrectly allows hard limits to exceed soft limits

[Rik van Riel]

On Fri, 6 Dec 2002, Rusty Trivial Russell wrote:

> > Just try "ulimit -H -m 10000" for memory limits that were not
> > previously set.  You end up with (hard limit = 10000) < (soft limit =
> > unlimited).

> +       if (new_rlim.rlim_cur > new_rlim.rlim_max)
> +               return -EINVAL;

Wouldn't it be better to simply take the soft limit down
to min(new_rlim.rlim_cur, new_rlim.rlim_max) ?

regards,

Rik
-- 
A: No.
Q: Should I include quotations after my reply?
http://www.surriel.com/		http://guru.conectiva.com/

Re: [TRIVIAL] Re: setrlimit incorrectly allows hard limits to exceed soft limits

[Kingsley Cheung]

On Fri, Dec 06, 2002 at 11:07:00AM -0200, Rik van Riel wrote:
> On Fri, 6 Dec 2002, Rusty Trivial Russell wrote:
> 
> > > Just try "ulimit -H -m 10000" for memory limits that were not
> > > previously set.  You end up with (hard limit = 10000) < (soft limit =
> > > unlimited).
> 
> > +       if (new_rlim.rlim_cur > new_rlim.rlim_max)
> > +               return -EINVAL;
> 
> Wouldn't it be better to simply take the soft limit down
> to min(new_rlim.rlim_cur, new_rlim.rlim_max) ?

Another way to do it would be to adjust the max limit up to the soft
limit only if the process had CAP_SYS_RESOURCE, just like in the
function svr4_ulimit in abi/svr4/ulimit.c.

In both cases, however, changing it will mean changing a limit on
behalf of the process.  But IMHO wouldn't most applications would
expect rlimits modified with setrlimit successfully to be set to the
rlimits that they specified as the argument?  Any child processes they
spawn would expect the same set of modified rlimits to be inherited as
well.  If we fix an incorrect pair of rlimits on their behalf, they
would then see different rlimit values with getrlimit later.

In light of that wouldn't it be better just to return an error and
insist that broken applications get fixed?  Unless of course there are
just too many broken apps out there : ) 

--
		Kingsley

Re: [TRIVIAL] Re: setrlimit incorrectly allows hard limits to exceed soft limits

[Rusty Russell]

In message <Pine.LNX.4.50L.0212061106050.22252-100000@duckman.distro.conectiva>
 you write:
> On Fri, 6 Dec 2002, Rusty Trivial Russell wrote:
> 
> > > Just try "ulimit -H -m 10000" for memory limits that were not
> > > previously set.  You end up with (hard limit = 10000) < (soft limit =
> > > unlimited).
> 
> > +       if (new_rlim.rlim_cur > new_rlim.rlim_max)
> > +               return -EINVAL;
> 
> Wouldn't it be better to simply take the soft limit down
> to min(new_rlim.rlim_cur, new_rlim.rlim_max) ?

POSIX says -EINVAL, and since it's a programmer fuckup, I'd agree.

Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [TRIVIAL] Re: setrlimit incorrectly allows hard limits to exceed soft limits

[Peter Chubb]

>>>>> "Rik" == Rik van Riel <riel@conectiva.com.br> writes:

Rik> On Fri, 6 Dec 2002, Rusty Trivial Russell wrote:
>> > Just try "ulimit -H -m 10000" for memory limits that were not >
>> previously set.  You end up with (hard limit = 10000) < (soft limit
>> = > unlimited).

>> + if (new_rlim.rlim_cur > new_rlim.rlim_max) + return -EINVAL;

Rik> Wouldn't it be better to simply take the soft limit down to
Rik> min(new_rlim.rlim_cur, new_rlim.rlim_max) ?
 
Single unix spec says to return EINVAL in this case.

[EINVAL]
An invalid resource was specified; or in a setrlimit() call, the new
rlim_cur exceeds the new rlim_max.

 

Re: [TRIVIAL] Re: setrlimit incorrectly allows hard limits to exceed soft limits

[Rik van Riel]

On Mon, 9 Dec 2002, Peter Chubb wrote:

> Rik> Wouldn't it be better to simply take the soft limit down to
> Rik> min(new_rlim.rlim_cur, new_rlim.rlim_max) ?
>
> Single unix spec says to return EINVAL in this case.
>
> [EINVAL]
> An invalid resource was specified; or in a setrlimit() call, the new
> rlim_cur exceeds the new rlim_max.

So how about "the old rlim_cur exceeds the new rlim_max" ? ;)

Rik
-- 
Bravely reimplemented by the knights who say "NIH".
http://www.surriel.com/		http://guru.conectiva.com/
Current spamtrap:  <a href=mailto:"october@surriel.com">october@surriel.com</a>

Re: [TRIVIAL] Re: setrlimit incorrectly allows hard limits to exceed soft limits

[Peter Chubb]

>>>>> "Rik" == Rik van Riel <riel@conectiva.com.br> writes:

Rik> On Mon, 9 Dec 2002, Peter Chubb wrote: Wouldn't it be better to
Rik> simply take the soft limit down to min(new_rlim.rlim_cur,
Rik> new_rlim.rlim_max) ?
>> Single unix spec says to return EINVAL in this case.
>> 
>> [EINVAL] An invalid resource was specified; or in a setrlimit()
>> call, the new rlim_cur exceeds the new rlim_max.

Rik> So how about "the old rlim_cur exceeds the new rlim_max" ? ;)

You always have to set both, so the old value is irrelevant, except in
so far as rlim_max may not be increased except by a privileged
process.

setrlimit may return EINVAL if the actual usage is above the new
rlim_cur.

--
Dr Peter Chubb				    peterc@gelato.unsw.edu.au
You are lost in a maze of BitKeeper repositories, all almost the same.

Re: [TRIVIAL] Re: setrlimit incorrectly allows hard limits to exceed soft limits

[Rik van Riel]

On Tue, 10 Dec 2002, Peter Chubb wrote:

> You always have to set both,

Duh, forgot about that.  Nevermind.

Rik
-- 
Bravely reimplemented by the knights who say "NIH".
http://www.surriel.com/		http://guru.conectiva.com/
Current spamtrap:  <a href=mailto:"october@surriel.com">october@surriel.com</a>