Re: [PATCH] c/r: [signal 2/3] checkpoint/restart of rlimit

["Serge E. Hallyn" <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>]

Quoting Oren Laadan (orenl-RdfvBDnrOixBDgjK7y7TUQ@public.gmane.org):
> This patch adds checkpoint and restart of rlimit information
> that is part of shared signal_struct.

...

>  static int restore_signal(struct ckpt_ctx *ctx)
>  {
>  	struct ckpt_hdr_signal *h;
> +	struct rlimit rlim;
> +	int i, ret;
> 
>  	h = ckpt_read_obj_type(ctx, sizeof(*h), CKPT_HDR_SIGNAL);
>  	if (IS_ERR(h))
>  		return PTR_ERR(h);
> 
> -	/* fill in later */
> -
> +	/* rlimit */
> +	for (i = 0; i < RLIM_NLIMITS; i++) {
> +		rlim.rlim_cur = h->rlim[i].rlim_cur;
> +		rlim.rlim_max = h->rlim[i].rlim_max;
> +		ret = do_setrlimit(i, &rlim);

...
> +int do_setrlimit(unsigned int resource, struct rlimit *new_rlim)
>  {
> -	struct rlimit new_rlim, *old_rlim;
> +	struct rlimit *old_rlim;
>  	int retval;
> 
> -	if (resource >= RLIM_NLIMITS)
> -		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->signal->rlim + resource;
> -	if ((new_rlim.rlim_max > old_rlim->rlim_max) &&
> +	if ((new_rlim->rlim_max > old_rlim->rlim_max) &&
>  	    !capable(CAP_SYS_RESOURCE))
>  		return -EPERM;
> -	if (resource == RLIMIT_NOFILE && new_rlim.rlim_max > sysctl_nr_open)
> +	if (resource == RLIMIT_NOFILE && new_rlim->rlim_max > sysctl_nr_open)
>  		return -EPERM;
> 
> -	retval = security_task_setrlimit(resource, &new_rlim);
> +	retval = security_task_setrlimit(resource, new_rlim);
>  	if (retval)
>  		return retval;
> 
> -	if (resource == RLIMIT_CPU && new_rlim.rlim_cur == 0) {
> +	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;
> +		new_rlim->rlim_cur = 1;
>  	}
> 
>  	task_lock(current->group_leader);
> -	*old_rlim = new_rlim;
> +	*old_rlim = *new_rlim;
>  	task_unlock(current->group_leader);
> 
>  	if (resource != RLIMIT_CPU)
> @@ -1189,14 +1183,27 @@ SYSCALL_DEFINE2(setrlimit, unsigned int, resource, struct rlimit __user *, rlim)
>  	 * very long-standing error, and fixing it now risks breakage of
>  	 * applications, so we live with it
>  	 */
> -	if (new_rlim.rlim_cur == RLIM_INFINITY)
> +	if (new_rlim->rlim_cur == RLIM_INFINITY)
>  		goto out;
> 
> -	update_rlimit_cpu(new_rlim.rlim_cur);
> +	update_rlimit_cpu(new_rlim->rlim_cur);
>  out:
>  	return 0;
>  }
> 
> +SYSCALL_DEFINE2(setrlimit, unsigned int, resource, struct rlimit __user *, rlim)
> +{
> +	struct rlimit new_rlim;
> +
> +	if (resource >= RLIM_NLIMITS)
> +		return -EINVAL;
> +	if (copy_from_user(&new_rlim, rlim, sizeof(*rlim)))
> +		return -EFAULT;
> +	if (new_rlim.rlim_cur > new_rlim.rlim_max)
> +		return -EINVAL;

Should the above check go into do_setrlimit()?  No sense trusting
the data sent to sys_checkpoint() any more than the data sent to
sys_setrlimit().

> +	return do_setrlimit(resource, &new_rlim);
> +}
> +
>  /*
>   * It would make sense to put struct rusage in the task_struct,
>   * except that would make the task_struct be *really big*.  After
> -- 
> 1.6.0.4