Subject: Re: setrlimit(RLIMIT_STACK,..) check for limit < stack usage
To: Jaromir Dolecek <jdolecek@NetBSD.org>
From: Giles Lean <giles@nemeton.com.au>
List: tech-kern
Date: 11/18/2001 11:18:04
Jaromir Dolecek writes:

> I've found ancient kern/3045, which basically points out that it's
> quite pointless to allow setrlimit(RLIMIT_STACK, ...) to set the
> stack size to value below current usage.  ...  I think that it would
> be more sensible to return EINVAL in this case.

The Single Unix Standard version 2 aka UNIX98 agrees with you:

  The setrlimit() function may fail if:

     [EINVAL] The limit specified cannot be lowered because
	      current usage is already higher than the limit.

>  		/*
> +		 * Return EINVAL if the new limit is lower than current
> +		 * usage. Otherwise, the process would get SIGSEGV the moment
> +		 * it would try to access anything on it's current stack.
> +		 * This is not what the caller intended, unless on crack
> +		 * or mistaken.
> +		 */
> +		if (limp->rlim_cur < p->p_vmspace->vm_dsize * PAGE_SIZE
> +		    || limp->rlim_max < p->p_vmspace->vm_dsize * PAGE_SIZE)
> +			return (EINVAL);
> +

I'd change the last sentence of the comment to reference the
standard. :-) The actual code change looks good but I'm not familiar
with the VM code to be certain.

While you're tweaking the function I'd suggest another change as SUSv2
also requires EINVAL when rlim_cur exceeds rlim_max:

   [EINVAL] ... in a setrlimit() call, the new rlim_cur exceeds the new
            rlim_max.

If this is thought desirable (and worthwhile :-), then the following code:

	if (limp->rlim_cur > alimp->rlim_max || 
	    limp->rlim_max > alimp->rlim_max)
		if ((error = suser(cred->pc_ucred, &p->p_acflag)) != 0)
			return (error);
	if (limp->rlim_cur > limp->rlim_max)
		limp->rlim_cur = limp->rlim_max;

could perhaps become:

	if (limp->rlim_cur > limp->rlim_max)
		return (EINVAL);
	if (limp->rlim_max > alimp->rlim_max &&
	    suser(cred->pc_ucred, &p->p_acflag) != 0)
	        return (EPERM);

Regards,

Giles