[PATCH 2.6.13-rc6 1/2] New Syscall: get rlimits of any process (update)

[PATCH 2.6.13-rc6 1/2] New Syscall: get rlimits of any process (update)

[Wieland Gmeiner]

Hi!

I incorporated the changes suggested by Greg KH and Kyle Moffett and
isolated some duplicated code (between setrlimit/setprlimit resp.
getprlimit/setprlimit) into helper functions. This is the first of
two patches, it implements the getprlimit() call.

Rationale: Currently usage limits (rlimits) can only be set inside a
process space, or inherited from the parent process. It would be
useful to allow adjusting resource limits for running processes, e.g.
tuning the resource usage of daemon processes under changing workloads
without restarting them.

Implementation: This patch provides a new syscall getprlimit() for
reading a given process resource limits for i386. Its implementation
follows closely the getrlimit syscall. It is given a pid as an
additional argument. If the given pid equals zero the current process
rlimits are read and the behaviour resembles the behaviour of
getrlimit. Otherwise some checking on the validity of the given pid is
done and if the given process is found access is granted if
- the calling process holds the CAP_SYS_RESOURCE capability or
- the calling process uid equals the uid, euid, suid of the target
  process and the calling process gid equals the gid, egid, sgid of
  the target process.
(This resembles the behaviour of the ptrace system call.)


See the followup for the writing syscall.

Simple programs for testing the syscalls can be found on
http://stud4.tuwien.ac.at/~e8607062/studies/soc/patches/


Signed-off-by: Wieland Gmeiner <e8607062@student.tuwien.ac.at>


diff -uprN -X linux-2.6.13-rc6-vanilla/Documentation/dontdiff linux-2.6.13-rc6-vanilla/arch/i386/kernel/syscall_table.S linux-2.6.13-rc6-getprlimit/arch/i386/kernel/syscall_table.S
--- linux-2.6.13-rc6-vanilla/arch/i386/kernel/syscall_table.S	2005-08-09 16:03:08.000000000 +0200
+++ linux-2.6.13-rc6-getprlimit/arch/i386/kernel/syscall_table.S	2005-08-17 03:20:30.000000000 +0200
@@ -294,3 +294,4 @@ ENTRY(sys_call_table)
 	.long sys_inotify_init
 	.long sys_inotify_add_watch
 	.long sys_inotify_rm_watch
+	.long sys_getprlimit
diff -uprN -X linux-2.6.13-rc6-vanilla/Documentation/dontdiff linux-2.6.13-rc6-vanilla/include/asm-i386/unistd.h linux-2.6.13-rc6-getprlimit/include/asm-i386/unistd.h
--- linux-2.6.13-rc6-vanilla/include/asm-i386/unistd.h	2005-08-09 16:03:19.000000000 +0200
+++ linux-2.6.13-rc6-getprlimit/include/asm-i386/unistd.h	2005-08-17 03:20:29.000000000 +0200
@@ -299,8 +299,9 @@
 #define __NR_inotify_init	291
 #define __NR_inotify_add_watch	292
 #define __NR_inotify_rm_watch	293
+#define __NR_getprlimit		294
 
-#define NR_syscalls 294
+#define NR_syscalls 295
 
 /*
  * user-visible error numbers are in the range -1 - -128: see
diff -uprN -X linux-2.6.13-rc6-vanilla/Documentation/dontdiff linux-2.6.13-rc6-vanilla/kernel/sys.c linux-2.6.13-rc6-getprlimit/kernel/sys.c
--- linux-2.6.13-rc6-vanilla/kernel/sys.c	2005-08-09 16:03:21.000000000 +0200
+++ linux-2.6.13-rc6-getprlimit/kernel/sys.c	2005-08-17 23:56:40.000000000 +0200
@@ -1604,6 +1604,63 @@ asmlinkage long sys_setrlimit(unsigned i
 }
 
 /*
+ * As ptrace implies the ability to execute arbitrary code in the given
+ * process, which means that the calling process could obtain and set
+ * rlimits for that process without getprlimit/setprlimit anyways,
+ * we use the same permission checks as ptrace.
+ */
+
+static inline int prlim_check_perm(task_t *task)
+{
+	return ((current->uid == task->euid) &&
+		(current->uid == task->suid) &&
+		(current->uid == task->uid) &&
+		(current->gid == task->egid) &&
+		(current->gid == task->sgid) &&
+		(current->gid == task->gid)) || capable(CAP_SYS_RESOURCE);
+}
+
+asmlinkage long sys_getprlimit(pid_t pid, unsigned int resource,
+			       struct rlimit __user *rlim)
+{
+	struct rlimit value;
+	task_t *p;
+	int retval = -EINVAL;
+
+	if (resource >= RLIM_NLIMITS)
+		goto out_nounlock;
+
+	if (pid < 0)
+		goto out_nounlock;
+
+	retval = -ESRCH;
+	if (pid == 0) {
+		p = current;
+	} else {
+		read_lock(&tasklist_lock);
+		p = find_task_by_pid(pid);
+	}
+	if (p) {
+		retval = -EPERM;
+		if (!prlim_check_perm(p))
+			goto out_unlock;
+
+		task_lock(p->group_leader);
+		value = p->signal->rlim[resource];
+		task_unlock(p->group_leader);
+		retval = copy_to_user(rlim, &value, sizeof(*rlim)) ? -EFAULT : 0;
+	}
+	if (pid == 0)
+		goto out_nounlock;
+
+out_unlock:
+	read_unlock(&tasklist_lock);
+
+out_nounlock:
+	return retval;
+}
+
+/*
  * It would make sense to put struct rusage in the task_struct,
  * except that would make the task_struct be *really big*.  After
  * task_struct gets moved into malloc'ed memory, it would

[PATCH 2.6.13-rc6 2/2] New Syscall: set rlimits of any process (update)

[Wieland Gmeiner]

This is the second of two patches, it implements the setprlimit()
syscall.

Implementation: This patch provides a new syscall setprlimit() for
writing a given process resource limits for i386. Its implementation
follows closely the setrlimit syscall. It is given a pid as an
additional argument. If the given pid equals zero the current process
rlimits are written and the behaviour resembles the behaviour of
setrlimit. Otherwise some checking on the validity of the given pid is
done and if the given process is found access is granted if
- the calling process holds the CAP_SYS_RESOURCE capability or
- the calling process uid equals the uid, euid, suid of the target
  process and the calling process gid equals the gid, egid, sgid of
  the target process.
(This resembles the behaviour of the ptrace system call.)

Simple programs for testing the syscalls can be found on
http://stud4.tuwien.ac.at/~e8607062/studies/soc/patches/


Signed-off-by: Wieland Gmeiner <e8607062@student.tuwien.ac.at>


diff -uprN -X linux-2.6.13-rc6-vanilla/Documentation/dontdiff linux-2.6.13-rc6-getprlimit/arch/i386/kernel/syscall_table.S linux-2.6.13-rc6-setprlimit/arch/i386/kernel/syscall_table.S
--- linux-2.6.13-rc6-getprlimit/arch/i386/kernel/syscall_table.S	2005-08-17 03:20:30.000000000 +0200
+++ linux-2.6.13-rc6-setprlimit/arch/i386/kernel/syscall_table.S	2005-08-18 01:28:13.000000000 +0200
@@ -295,3 +295,4 @@ ENTRY(sys_call_table)
 	.long sys_inotify_add_watch
 	.long sys_inotify_rm_watch
 	.long sys_getprlimit
+	.long sys_setprlimit		/* 295 */
diff -uprN -X linux-2.6.13-rc6-vanilla/Documentation/dontdiff linux-2.6.13-rc6-getprlimit/include/asm-i386/unistd.h linux-2.6.13-rc6-setprlimit/include/asm-i386/unistd.h
--- linux-2.6.13-rc6-getprlimit/include/asm-i386/unistd.h	2005-08-17 03:20:29.000000000 +0200
+++ linux-2.6.13-rc6-setprlimit/include/asm-i386/unistd.h	2005-08-18 01:28:12.000000000 +0200
@@ -300,8 +300,9 @@
 #define __NR_inotify_add_watch	292
 #define __NR_inotify_rm_watch	293
 #define __NR_getprlimit		294
+#define __NR_setprlimit		295
 
-#define NR_syscalls 295
+#define NR_syscalls 296
 
 /*
  * user-visible error numbers are in the range -1 - -128: see
diff -uprN -X linux-2.6.13-rc6-vanilla/Documentation/dontdiff linux-2.6.13-rc6-getprlimit/include/linux/security.h linux-2.6.13-rc6-setprlimit/include/linux/security.h
--- linux-2.6.13-rc6-getprlimit/include/linux/security.h	2005-08-17 03:11:32.000000000 +0200
+++ linux-2.6.13-rc6-setprlimit/include/linux/security.h	2005-08-18 01:28:05.000000000 +0200
@@ -591,6 +591,15 @@ struct swap_info_struct;
  *	@resource contains the resource whose limit is being set.
  *	@new_rlim contains the new limits for @resource.
  *	Return 0 if permission is granted.
+ * @task_setprlimit:
+ *	Check permission before setting the resource limits of process @p
+ *	for @resource to @new_rlim.  The old resource limit values can
+ *	be examined by dereferencing (p->signal->rlim + resource).
+ *	@p contains the task struct of the process whose resource limit is
+ *	being set.
+ *	@resource contains the resource whose limit is being set.
+ *	@new_rlim contains the new limits for @resource.
+ *	Return 0 if permission is granted.
  * @task_setscheduler:
  *	Check permission before setting scheduling policy and/or parameters of
  *	process @p based on @policy and @lp.
@@ -1157,6 +1166,8 @@ struct security_operations {
 	int (*task_setgroups) (struct group_info *group_info);
 	int (*task_setnice) (struct task_struct * p, int nice);
 	int (*task_setrlimit) (unsigned int resource, struct rlimit * new_rlim);
+	int (*task_setprlimit) (struct task_struct * p, unsigned int resource,
+				struct rlimit * new_rlim);
 	int (*task_setscheduler) (struct task_struct * p, int policy,
 				  struct sched_param * lp);
 	int (*task_getscheduler) (struct task_struct * p);
@@ -1804,6 +1815,13 @@ static inline int security_task_setrlimi
 	return security_ops->task_setrlimit (resource, new_rlim);
 }
 
+static inline int security_task_setprlimit (struct task_struct *p,
+					    unsigned int resource,
+					    struct rlimit *new_rlim)
+{
+	return security_ops->task_setprlimit (p, resource, new_rlim);
+}
+
 static inline int security_task_setscheduler (struct task_struct *p,
 					      int policy,
 					      struct sched_param *lp)
diff -uprN -X linux-2.6.13-rc6-vanilla/Documentation/dontdiff linux-2.6.13-rc6-getprlimit/kernel/sys.c linux-2.6.13-rc6-setprlimit/kernel/sys.c
--- linux-2.6.13-rc6-getprlimit/kernel/sys.c	2005-08-17 23:56:40.000000000 +0200
+++ linux-2.6.13-rc6-setprlimit/kernel/sys.c	2005-08-18 01:44:25.000000000 +0200
@@ -1561,6 +1561,27 @@ asmlinkage long sys_old_getrlimit(unsign
 
 #endif
 
+static inline void check_process_timer(struct task_struct *task,
+				       unsigned int resource,
+				       struct rlimit *new_rlim,
+				       int holds_lock)
+{
+	if (resource == RLIMIT_CPU && new_rlim->rlim_cur != RLIM_INFINITY &&
+	    (cputime_eq(task->signal->it_prof_expires, cputime_zero) ||
+	     new_rlim->rlim_cur <= cputime_to_secs(
+		     task->signal->it_prof_expires))) {
+		cputime_t cputime = secs_to_cputime(new_rlim->rlim_cur);
+		if (!holds_lock)
+			read_lock(&tasklist_lock);
+		spin_lock_irq(&task->sighand->siglock);
+		set_process_cpu_timer(task, CPUCLOCK_PROF,
+				      &cputime, NULL);
+		spin_unlock_irq(&task->sighand->siglock);
+		if (!holds_lock)
+			read_unlock(&tasklist_lock);
+	}
+}
+
 asmlinkage long sys_setrlimit(unsigned int resource, struct rlimit __user *rlim)
 {
 	struct rlimit new_rlim, *old_rlim;
@@ -1570,8 +1591,8 @@ asmlinkage long sys_setrlimit(unsigned i
 		return -EINVAL;
 	if(copy_from_user(&new_rlim, rlim, sizeof(*rlim)))
 		return -EFAULT;
-       if (new_rlim.rlim_cur > new_rlim.rlim_max)
-               return -EINVAL;
+	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) &&
 	    !capable(CAP_SYS_RESOURCE))
@@ -1587,18 +1608,7 @@ asmlinkage long sys_setrlimit(unsigned i
 	*old_rlim = new_rlim;
 	task_unlock(current->group_leader);
 
-	if (resource == RLIMIT_CPU && new_rlim.rlim_cur != RLIM_INFINITY &&
-	    (cputime_eq(current->signal->it_prof_expires, cputime_zero) ||
-	     new_rlim.rlim_cur <= cputime_to_secs(
-		     current->signal->it_prof_expires))) {
-		cputime_t cputime = secs_to_cputime(new_rlim.rlim_cur);
-		read_lock(&tasklist_lock);
-		spin_lock_irq(&current->sighand->siglock);
-		set_process_cpu_timer(current, CPUCLOCK_PROF,
-				      &cputime, NULL);
-		spin_unlock_irq(&current->sighand->siglock);
-		read_unlock(&tasklist_lock);
-	}
+	check_process_timer(current, resource, &new_rlim, 0);
 
 	return 0;
 }
@@ -1660,6 +1670,62 @@ out_nounlock:
 	return retval;
 }
 
+asmlinkage long sys_setprlimit(pid_t pid, unsigned int resource,
+			       struct rlimit __user *rlim)
+{
+	struct rlimit new_rlim, *old_rlim;
+	int retval;
+	task_t *p;
+
+	if (resource >= RLIM_NLIMITS)
+		return -EINVAL;
+	if (pid < 0)
+		return -EINVAL;
+	if (copy_from_user(&new_rlim, rlim, sizeof(*rlim)))
+		return -EFAULT;
+	if (new_rlim.rlim_cur > new_rlim.rlim_max)
+		return -EINVAL;
+
+	retval = -ESRCH;
+	if (pid == 0) {
+		p = current;
+	} else {
+		read_lock(&tasklist_lock);
+		p = find_task_by_pid(pid);
+	}
+	if (p) {
+		retval = -EPERM;
+		if (!prlim_check_perm(p))
+			goto out_unlock;
+
+		old_rlim = p->signal->rlim + resource;
+		if ((new_rlim.rlim_max > old_rlim->rlim_max) &&
+		    !capable(CAP_SYS_RESOURCE))
+			goto out_unlock;
+		if (resource == RLIMIT_NOFILE && new_rlim.rlim_max > NR_OPEN)
+			goto out_unlock;
+
+		retval = security_task_setprlimit(p, resource, &new_rlim);
+		if (retval)
+			goto out_unlock;
+
+		task_lock(p->group_leader);
+		*old_rlim = new_rlim;
+		task_unlock(p->group_leader);
+		retval = 0;
+
+		check_process_timer(p, resource, &new_rlim, 1);
+	}
+	if (pid == 0)
+		goto out_nounlock;
+
+out_unlock:
+	read_unlock(&tasklist_lock);
+
+out_nounlock:
+	return retval;
+}
+
 /*
  * It would make sense to put struct rusage in the task_struct,
  * except that would make the task_struct be *really big*.  After
diff -uprN -X linux-2.6.13-rc6-vanilla/Documentation/dontdiff linux-2.6.13-rc6-getprlimit/security/dummy.c linux-2.6.13-rc6-setprlimit/security/dummy.c
--- linux-2.6.13-rc6-getprlimit/security/dummy.c	2005-08-17 03:11:33.000000000 +0200
+++ linux-2.6.13-rc6-setprlimit/security/dummy.c	2005-08-18 01:28:06.000000000 +0200
@@ -548,6 +548,12 @@ static int dummy_task_setrlimit (unsigne
 	return 0;
 }
 
+static int dummy_task_setprlimit (struct task_struct *p, unsigned int resource,
+				  struct rlimit *new_rlim)
+{
+	return 0;
+}
+
 static int dummy_task_setscheduler (struct task_struct *p, int policy,
 				    struct sched_param *lp)
 {
@@ -937,6 +943,7 @@ void security_fixup_ops (struct security
 	set_to_dummy_if_null(ops, task_setgroups);
 	set_to_dummy_if_null(ops, task_setnice);
 	set_to_dummy_if_null(ops, task_setrlimit);
+	set_to_dummy_if_null(ops, task_setprlimit);
 	set_to_dummy_if_null(ops, task_setscheduler);
 	set_to_dummy_if_null(ops, task_getscheduler);
 	set_to_dummy_if_null(ops, task_wait);
diff -uprN -X linux-2.6.13-rc6-vanilla/Documentation/dontdiff linux-2.6.13-rc6-getprlimit/security/selinux/hooks.c linux-2.6.13-rc6-setprlimit/security/selinux/hooks.c
--- linux-2.6.13-rc6-getprlimit/security/selinux/hooks.c	2005-08-17 03:11:33.000000000 +0200
+++ linux-2.6.13-rc6-setprlimit/security/selinux/hooks.c	2005-08-18 01:28:11.000000000 +0200
@@ -2703,12 +2703,16 @@ static int selinux_task_setnice(struct t
 	return task_has_perm(current,p, PROCESS__SETSCHED);
 }
 
-static int selinux_task_setrlimit(unsigned int resource, struct rlimit *new_rlim)
+static inline int check_rlim_perm(struct task_struct *task, unsigned int resource,
+				  struct rlimit *new_rlim)
 {
-	struct rlimit *old_rlim = current->signal->rlim + resource;
+	struct rlimit *old_rlim = task->signal->rlim + resource;
 	int rc;
 
-	rc = secondary_ops->task_setrlimit(resource, new_rlim);
+	if (task == current)
+		rc = secondary_ops->task_setrlimit(resource, new_rlim);
+	else
+		rc = secondary_ops->task_setprlimit(task, resource, new_rlim);
 	if (rc)
 		return rc;
 
@@ -2717,11 +2721,22 @@ static int selinux_task_setrlimit(unsign
 	   later be used as a safe reset point for the soft limit
 	   upon context transitions. See selinux_bprm_apply_creds. */
 	if (old_rlim->rlim_max != new_rlim->rlim_max)
-		return task_has_perm(current, current, PROCESS__SETRLIMIT);
+		return task_has_perm(current, task, PROCESS__SETRLIMIT);
 
 	return 0;
 }
 
+static int selinux_task_setrlimit(unsigned int resource, struct rlimit *new_rlim)
+{
+	return check_rlim_perm(current, resource, new_rlim);
+}
+
+static int selinux_task_setprlimit(struct task_struct *p, unsigned int resource,
+				   struct rlimit *new_rlim)
+{
+	return check_rlim_perm(p, resource, new_rlim);
+}
+
 static int selinux_task_setscheduler(struct task_struct *p, int policy, struct sched_param *lp)
 {
 	return task_has_perm(current, p, PROCESS__SETSCHED);

Re: [PATCH 2.6.13-rc6 1/2] New Syscall: get rlimits of any process (update)

[Chris Wright]

* Wieland Gmeiner (e8607062@student.tuwien.ac.at) wrote:
> diff -uprN -X linux-2.6.13-rc6-vanilla/Documentation/dontdiff linux-2.6.13-rc6-vanilla/kernel/sys.c linux-2.6.13-rc6-getprlimit/kernel/sys.c
> --- linux-2.6.13-rc6-vanilla/kernel/sys.c	2005-08-09 16:03:21.000000000 +0200
> +++ linux-2.6.13-rc6-getprlimit/kernel/sys.c	2005-08-17 23:56:40.000000000 +0200
> @@ -1604,6 +1604,63 @@ asmlinkage long sys_setrlimit(unsigned i
>  }
>  
>  /*
> + * As ptrace implies the ability to execute arbitrary code in the given
> + * process, which means that the calling process could obtain and set
> + * rlimits for that process without getprlimit/setprlimit anyways,
> + * we use the same permission checks as ptrace.
> + */
> +
> +static inline int prlim_check_perm(task_t *task)
> +{
> +	return ((current->uid == task->euid) &&
> +		(current->uid == task->suid) &&
> +		(current->uid == task->uid) &&
> +		(current->gid == task->egid) &&
> +		(current->gid == task->sgid) &&
> +		(current->gid == task->gid)) || capable(CAP_SYS_RESOURCE);
> +}

This comment and the code aren't matching.  CAP_SYS_RESOUCE now means
effective on any other process, which it never did before.  That should
be given careful thought.  CAP_SYS_PTRACE indeed would let you call
get/setrlimit in traced task, perhaps that what you meant?

> +
> +asmlinkage long sys_getprlimit(pid_t pid, unsigned int resource,
> +			       struct rlimit __user *rlim)
> +{
> +	struct rlimit value;
> +	task_t *p;
> +	int retval = -EINVAL;
> +
> +	if (resource >= RLIM_NLIMITS)
> +		goto out_nounlock;
> +
> +	if (pid < 0)
> +		goto out_nounlock;
> +
> +	retval = -ESRCH;
> +	if (pid == 0) {
> +		p = current;
> +	} else {
> +		read_lock(&tasklist_lock);
> +		p = find_task_by_pid(pid);
> +	}
> +	if (p) {
> +		retval = -EPERM;
> +		if (!prlim_check_perm(p))
> +			goto out_unlock;
> +
> +		task_lock(p->group_leader);
> +		value = p->signal->rlim[resource];
> +		task_unlock(p->group_leader);
> +		retval = copy_to_user(rlim, &value, sizeof(*rlim)) ? -EFAULT : 0;

Do not call copy_to_user() with tasklist_lock held.  Also, this is the
same basic code as sys_getrlimit().  So they should share code. (IOW,
sys_getrlimit() is now really sys_getprlimit(0,...))

thanks,
-chris

Re: [PATCH 2.6.13-rc6 2/2] New Syscall: set rlimits of any process (update)

[Chris Wright]

* Wieland Gmeiner (e8607062@student.tuwien.ac.at) wrote:
> diff -uprN -X linux-2.6.13-rc6-vanilla/Documentation/dontdiff linux-2.6.13-rc6-getprlimit/include/linux/security.h linux-2.6.13-rc6-setprlimit/include/linux/security.h
> --- linux-2.6.13-rc6-getprlimit/include/linux/security.h	2005-08-17 03:11:32.000000000 +0200
> +++ linux-2.6.13-rc6-setprlimit/include/linux/security.h	2005-08-18 01:28:05.000000000 +0200
> @@ -591,6 +591,15 @@ struct swap_info_struct;
>   *	@resource contains the resource whose limit is being set.
>   *	@new_rlim contains the new limits for @resource.
>   *	Return 0 if permission is granted.
> + * @task_setprlimit:
> + *	Check permission before setting the resource limits of process @p
> + *	for @resource to @new_rlim.  The old resource limit values can
> + *	be examined by dereferencing (p->signal->rlim + resource).
> + *	@p contains the task struct of the process whose resource limit is
> + *	being set.
> + *	@resource contains the resource whose limit is being set.
> + *	@new_rlim contains the new limits for @resource.
> + *	Return 0 if permission is granted.
>   * @task_setscheduler:
>   *	Check permission before setting scheduling policy and/or parameters of
>   *	process @p based on @policy and @lp.
> @@ -1157,6 +1166,8 @@ struct security_operations {
>  	int (*task_setgroups) (struct group_info *group_info);
>  	int (*task_setnice) (struct task_struct * p, int nice);
>  	int (*task_setrlimit) (unsigned int resource, struct rlimit * new_rlim);
> +	int (*task_setprlimit) (struct task_struct * p, unsigned int resource,
> +				struct rlimit * new_rlim);

No need for a new hook.  They do the same thing.  Just collapse into one.

> diff -uprN -X linux-2.6.13-rc6-vanilla/Documentation/dontdiff linux-2.6.13-rc6-getprlimit/kernel/sys.c linux-2.6.13-rc6-setprlimit/kernel/sys.c
> --- linux-2.6.13-rc6-getprlimit/kernel/sys.c	2005-08-17 23:56:40.000000000 +0200
> +++ linux-2.6.13-rc6-setprlimit/kernel/sys.c	2005-08-18 01:44:25.000000000 +0200
> @@ -1561,6 +1561,27 @@ asmlinkage long sys_old_getrlimit(unsign
>  
>  #endif
>  
> +static inline void check_process_timer(struct task_struct *task,
> +				       unsigned int resource,
> +				       struct rlimit *new_rlim,
> +				       int holds_lock)
> +{
> +	if (resource == RLIMIT_CPU && new_rlim->rlim_cur != RLIM_INFINITY &&
> +	    (cputime_eq(task->signal->it_prof_expires, cputime_zero) ||
> +	     new_rlim->rlim_cur <= cputime_to_secs(
> +		     task->signal->it_prof_expires))) {
> +		cputime_t cputime = secs_to_cputime(new_rlim->rlim_cur);
> +		if (!holds_lock)
> +			read_lock(&tasklist_lock);

This is not a great idea.  Please rework so that locking isn't
conditional like that.

> +		spin_lock_irq(&task->sighand->siglock);
> +		set_process_cpu_timer(task, CPUCLOCK_PROF,
> +				      &cputime, NULL);
> +		spin_unlock_irq(&task->sighand->siglock);
> +		if (!holds_lock)
> +			read_unlock(&tasklist_lock);
> +	}
> +}
> +
>  asmlinkage long sys_setrlimit(unsigned int resource, struct rlimit __user *rlim)
>  {
>  	struct rlimit new_rlim, *old_rlim;
> @@ -1570,8 +1591,8 @@ asmlinkage long sys_setrlimit(unsigned i
>  		return -EINVAL;
>  	if(copy_from_user(&new_rlim, rlim, sizeof(*rlim)))
>  		return -EFAULT;
> -       if (new_rlim.rlim_cur > new_rlim.rlim_max)
> -               return -EINVAL;
> +	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) &&
>  	    !capable(CAP_SYS_RESOURCE))
> @@ -1587,18 +1608,7 @@ asmlinkage long sys_setrlimit(unsigned i
>  	*old_rlim = new_rlim;
>  	task_unlock(current->group_leader);
>  
> -	if (resource == RLIMIT_CPU && new_rlim.rlim_cur != RLIM_INFINITY &&
> -	    (cputime_eq(current->signal->it_prof_expires, cputime_zero) ||
> -	     new_rlim.rlim_cur <= cputime_to_secs(
> -		     current->signal->it_prof_expires))) {
> -		cputime_t cputime = secs_to_cputime(new_rlim.rlim_cur);
> -		read_lock(&tasklist_lock);
> -		spin_lock_irq(&current->sighand->siglock);
> -		set_process_cpu_timer(current, CPUCLOCK_PROF,
> -				      &cputime, NULL);
> -		spin_unlock_irq(&current->sighand->siglock);
> -		read_unlock(&tasklist_lock);
> -	}
> +	check_process_timer(current, resource, &new_rlim, 0);
>  
>  	return 0;
>  }
> @@ -1660,6 +1670,62 @@ out_nounlock:
>  	return retval;
>  }
>  
> +asmlinkage long sys_setprlimit(pid_t pid, unsigned int resource,
> +			       struct rlimit __user *rlim)
> +{
> +	struct rlimit new_rlim, *old_rlim;
> +	int retval;
> +	task_t *p;
> +
> +	if (resource >= RLIM_NLIMITS)
> +		return -EINVAL;
> +	if (pid < 0)
> +		return -EINVAL;
> +	if (copy_from_user(&new_rlim, rlim, sizeof(*rlim)))
> +		return -EFAULT;
> +	if (new_rlim.rlim_cur > new_rlim.rlim_max)
> +		return -EINVAL;
> +
> +	retval = -ESRCH;
> +	if (pid == 0) {
> +		p = current;
> +	} else {
> +		read_lock(&tasklist_lock);
> +		p = find_task_by_pid(pid);

Don't do this conditional locking.  Just hold lock in either case.
Makes cleanup on errors easier, and code simpler to read.

> +	}
> +	if (p) {
> +		retval = -EPERM;
> +		if (!prlim_check_perm(p))
> +			goto out_unlock;
> +
> +		old_rlim = p->signal->rlim + resource;
> +		if ((new_rlim.rlim_max > old_rlim->rlim_max) &&
> +		    !capable(CAP_SYS_RESOURCE))
> +			goto out_unlock;
> +		if (resource == RLIMIT_NOFILE && new_rlim.rlim_max > NR_OPEN)
> +			goto out_unlock;
> +
> +		retval = security_task_setprlimit(p, resource, &new_rlim);
> +		if (retval)
> +			goto out_unlock;
> +
> +		task_lock(p->group_leader);
> +		*old_rlim = new_rlim;
> +		task_unlock(p->group_leader);
> +		retval = 0;

Again, rather than duplicate this code, share functionality with the
old setrlimit.  Also, you missed the compat entries.

There have been some patches floating about to do that (at least
for getrlimit) in /proc.  Did you consider that approach?  Oh, btw,
I _thought_ {get,set}prlimit() already existed in BSD with slightly
different parameters.

> +
> +		check_process_timer(p, resource, &new_rlim, 1);
> +	}
> +	if (pid == 0)
> +		goto out_nounlock;
> +
> +out_unlock:
> +	read_unlock(&tasklist_lock);
> +
> +out_nounlock:
> +	return retval;
> +}
> +
>  /*
>   * It would make sense to put struct rusage in the task_struct,
>   * except that would make the task_struct be *really big*.  After

Re: [PATCH 2.6.13-rc6 1/2] New Syscall: get rlimits of any process (update)

[Andi Kleen]

Wieland Gmeiner <e8607062@student.tuwien.ac.at> writes:

Is there a realistic use case where this new system call is actually useful
and solves something that cannot be solved without it?

If not I think it's better not to merge this to avoid unnecessary bloat.

-Andi

Re: [PATCH 2.6.13-rc6 2/2] New Syscall: set rlimits of any process (update)

[Stephen Smalley]

On Thu, 2005-08-18 at 03:02 +0200, Wieland Gmeiner wrote:
> diff -uprN -X linux-2.6.13-rc6-vanilla/Documentation/dontdiff linux-2.6.13-rc6-getprlimit/security/selinux/hooks.c linux-2.6.13-rc6-setprlimit/security/selinux/hooks.c
> --- linux-2.6.13-rc6-getprlimit/security/selinux/hooks.c	2005-08-17 03:11:33.000000000 +0200
> +++ linux-2.6.13-rc6-setprlimit/security/selinux/hooks.c	2005-08-18 01:28:11.000000000 +0200
> @@ -2717,11 +2721,22 @@ static int selinux_task_setrlimit(unsign
>  	   later be used as a safe reset point for the soft limit
>  	   upon context transitions. See selinux_bprm_apply_creds. */
>  	if (old_rlim->rlim_max != new_rlim->rlim_max)
> -		return task_has_perm(current, current, PROCESS__SETRLIMIT);
> +		return task_has_perm(current, task, PROCESS__SETRLIMIT);
>  
>  	return 0;
>  }

This isn't sufficient for mandatory access control over setprlimit.  As
long as the setrlimit operation could only be performed by the process
on itself, we were only concerned with controlling attempts to modify
the hard limit (not the soft limit), as that is being used as a safe
reset point for the soft limit upon security context transitions.  But
for the case of setprlimit where the target process may differ, we need
to be able to control setting of any limit, soft or hard, in order to
control the ability of a process in one security context to modify the
state of a process in a different security context.  Further, we would
need a parallel check on the getprlimit side, to control the ability of
a process in one security context to observe the state of a process in a
different security context.

-- 
Stephen Smalley
National Security Agency

Re: [PATCH 2.6.13-rc6 1/2] New Syscall: get rlimits of any process (update)

[Wieland Gmeiner]

On Thu, 2005-08-18 at 04:05 +0200, Andi Kleen wrote:

> Is there a realistic use case where this new system call is actually useful
> and solves something that cannot be solved without it?

As an example: It seems to be a common problem with numerous services to
run out of available file descriptors. There are several workarounds to
this problem, the most common seems to be increasing the systemwide max
number of filedescriptors and restarting the service. If you google for
e.g. 'linux "too many open files"' you get a bunch of mailing list
support requests about that problem.

Also some documention for specific services show that there is a need to
adjust rlimits per process at runtime, e.g.:
http://www.squid-cache.org/Doc/FAQ/FAQ-11.html#ss11.4
http://slacksite.com/apache/logging.html
http://staff.in2.hr/denis/oracle/10g1install_fedora3_en.html#n2

Thanks,
Wieland

Re: [PATCH 2.6.13-rc6 1/2] New Syscall: get rlimits of any process (update)

[Andi Kleen]

Wieland Gmeiner <e8607062@student.tuwien.ac.at> writes:

> On Thu, 2005-08-18 at 04:05 +0200, Andi Kleen wrote:
> 
> > Is there a realistic use case where this new system call is actually useful
> > and solves something that cannot be solved without it?
> 
> As an example: It seems to be a common problem with numerous services to
> run out of available file descriptors. There are several workarounds to
> this problem, the most common seems to be increasing the systemwide max
> number of filedescriptors and restarting the service. If you google for
> e.g. 'linux "too many open files"' you get a bunch of mailing list
> support requests about that problem.

Consider a process that runs out of fds. If that happens it's already
too late - it lost some of its files and is probably in a inconsistent
state internally because usually error handling parts for such things
are not working very well. The best thing to do is to restart it.

The only way to handle it properly would be to constantly monitor the
process and then try to increase it shortly before it runs out of
fds. First there is no reliable race free way to do this, and if you
really do all this overhead then it's far easier to just increase the
max number of fds in the first place.

In short your new call doesn't solve the problem in a sufficient way.

The real fix probably would be to just increase the default limits
even further, possible coupled with more aggressive per user memory
management. The main reason these limits tends to too small
is to avoid a user to fill up kernel memory too much, and there
is no global "per user" accounting which accounts all memory
used by a user. If that was available the per process limit
could be raised much further.

And e.g.  on 64bit machines there tends to be more usable low mem for
these purposes anyways so even without additional accounting the 
limits could be higher there.

-Andi

Re: [PATCH 2.6.13-rc6 1/2] New Syscall: get rlimits of any process (update)

[James Morris]

On Thu, 18 Aug 2005, Wieland Gmeiner wrote:

> On Thu, 2005-08-18 at 04:05 +0200, Andi Kleen wrote:
> 
> > Is there a realistic use case where this new system call is actually useful
> > and solves something that cannot be solved without it?
> 
> As an example: It seems to be a common problem with numerous services to
> run out of available file descriptors. There are several workarounds to
> this problem, the most common seems to be increasing the systemwide max
> number of filedescriptors and restarting the service. If you google for
> e.g. 'linux "too many open files"' you get a bunch of mailing list
> support requests about that problem.

This is a system tuning issue, not justification for two new system calls.


- James
-- 
James Morris
<jmorris@namei.org>

Re: [PATCH 2.6.13-rc6 1/2] New Syscall: get rlimits of any process (update)

[Alan Cox]

> Also some documention for specific services show that there is a need to
> adjust rlimits per process at runtime, e.g.:
> http://www.squid-cache.org/Doc/FAQ/FAQ-11.html#ss11.4
> http://slacksite.com/apache/logging.html
> http://staff.in2.hr/denis/oracle/10g1install_fedora3_en.html#n2

Perhaps those application authors should provide a management interface
to do so within the soft limit range at least. Its not clear to me that
growing the fd array on a process is even safe. Some programs do size
arrays at startup after querying the rlimit data.

Re: [PATCH 2.6.13-rc6 1/2] New Syscall: get rlimits of any process (update)

[Lee Revell]

On Thu, 2005-08-18 at 18:19 +0200, Wieland Gmeiner wrote:
> As an example: It seems to be a common problem with numerous services
> to run out of available file descriptors. There are several
> workarounds to this problem, the most common seems to be increasing
> the systemwide max number of filedescriptors and restarting the
> service. If you google for e.g. 'linux "too many open files"' you get
> a bunch of mailing list support requests about that problem. 

Maybe the distros need to just increase the default FD limit to 1024.  I
hit this constantly with gtk-gnutella, if try to download a file that's
available on more than 1024 hosts it will open sockets until it hits
that limit then bomb out.

1024 seems way too low these days given the proliferation of "swarming"
P2P protocols like gnutella and bittorrent.

Lee

Re: [PATCH 2.6.13-rc6 1/2] New Syscall: get rlimits of any process (update)

[Alan Cox]

On Iau, 2005-08-18 at 14:17 -0400, Lee Revell wrote:
> Maybe the distros need to just increase the default FD limit to 1024.  I
> hit this constantly with gtk-gnutella, if try to download a file that's
> available on more than 1024 hosts it will open sockets until it hits
> that limit then bomb out.

Sounds like a remarkably badly designed application. The author should
perhaps look at the papers on tcp capture and efficiency unless they
have a truely remarkably huge network pipe.

Re: [PATCH 2.6.13-rc6 1/2] New Syscall: get rlimits of any process (update)

[Lee Revell]

On Fri, 2005-08-19 at 00:13 +0100, Alan Cox wrote:
> On Iau, 2005-08-18 at 14:17 -0400, Lee Revell wrote:
> > Maybe the distros need to just increase the default FD limit to 1024.  I
> > hit this constantly with gtk-gnutella, if try to download a file that's
> > available on more than 1024 hosts it will open sockets until it hits
> > that limit then bomb out.
> 
> Sounds like a remarkably badly designed application. The author should
> perhaps look at the papers on tcp capture and efficiency unless they
> have a truely remarkably huge network pipe.
> 

OK say your DSL can dl at 350KB/sec.

A search for a 200MB file tells you it's available on 2000 hosts, all of
whom are on dialup.  You break it into 100 chunks and request a chunk
from what should be the fastest 100 hosts.  You wait a bit for the
speeds to stabilize and find you're only getting 150KB/sec aggregate, so
you reduce the size of the chunks and connect to 100 more hosts.  Repeat
until the pipe is full or you run out of FDs.

What's inefficient about this?  It's remarkably fast even for high
demand files and does not suck up all available upstream BW like
bittorrent and is leech-friendly ;-)

Lee

Re: [PATCH 2.6.13-rc6 1/2] New Syscall: get rlimits of any process (update)

[Lee Revell]

On Fri, 2005-08-19 at 01:29 +0100, Alan Cox wrote:
> On Iau, 2005-08-18 at 19:16 -0400, Lee Revell wrote:
> > A search for a 200MB file tells you it's available on 2000 hosts, all of
> > whom are on dialup.  
> 
> What about the real world ?
> 

OK that was a poorly contrived example.  Anyway the specific numbers
don't matter as much; it's actually quite a common scenario to run out
of FDs before you can fill the pipe.  It happens to me about once a
week.

Modern P2P protocols all seem to use some type of "swarming".  IIRC the
inspiration was the DDOS attacks of the late 90s - thousands of hosts on
slow connections can fill even the fattest pipe.

Lee

Re: [PATCH 2.6.13-rc6 1/2] New Syscall: get rlimits of any process (update)

[Alan Cox]

On Iau, 2005-08-18 at 19:16 -0400, Lee Revell wrote:
> A search for a 200MB file tells you it's available on 2000 hosts, all of
> whom are on dialup.  

What about the real world ?

Re: [PATCH 2.6.13-rc6 1/2] New Syscall: get rlimits of any process (update)

[Elliot Lee]

On Thu, 18 Aug 2005, Alan Cox wrote:

> > Also some documention for specific services show that there is a need to
> > adjust rlimits per process at runtime, e.g.:
> > http://www.squid-cache.org/Doc/FAQ/FAQ-11.html#ss11.4
> > http://slacksite.com/apache/logging.html
> > http://staff.in2.hr/denis/oracle/10g1install_fedora3_en.html#n2
> 
> Perhaps those application authors should provide a management interface
> to do so within the soft limit range at least. Its not clear to me that
> growing the fd array on a process is even safe. Some programs do size
> arrays at startup after querying the rlimit data.

This is getting hung up on one particular example, while missing the
bigger picture.

Being able to set rlimits for other processes is useful in general, just
as things like nice() and sched_setscheduler() are. Maybe it is reducing
max RSS on certain processes and increasing it on other processes, so that
the memory tends to be used for higher-priority processes. Maybe it is 
setting max cpu time to T+5 minutes, so that if a seemingly stuck process 
has not exited in five minutes, it will die. Maybe it is limiting  the 
number of processes that a daemon can spawn.

In addition, it's not always practical to have application authors provide 
a management interface.

You're right that there can be potential problems with adjusting rlimits 
on the fly, but that doesn't seem like a sufficient reason to avoid 
including the feature.

Best,
-- Elliot
Pioneers get the Arrows. Settlers get the Land.

Re: [PATCH 2.6.13-rc6 1/2] New Syscall: get rlimits of any process (update)

[Eric W. Biederman]

Wieland Gmeiner <e8607062@student.tuwien.ac.at> writes:

> (This resembles the behaviour of the ptrace system call.)

ptrace does not solve this problem because?

If you can do this now without a syscall why do you need
a syscall to optimize this code path?

Eric

Re: [PATCH 2.6.13-rc6 1/2] New Syscall: get rlimits of any process (update)

[Ulrich Drepper]

On 8/18/05, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> Perhaps those application authors should provide a management interface
> to do so within the soft limit range at least. Its not clear to me that
> growing the fd array on a process is even safe. Some programs do size
> arrays at startup after querying the rlimit data.

That's very true.  Using such a remote-rlimit syscall would break all
kinds of code.  It's a basic assumption from Unix/POSIX that the
limits remain constant.  And as Alan hinted at: this is why there are
soft and hard limits.  If tey are set to the same value you obviously
don't get anything.  But this is the application programmer's fault. 
An application which is aware of resources and tries to limit them
should set the soft limits to a reasonable low value and the hard
limit to the absolute maximum (probably the system's maximum).  Then
you can have remote procedure calls into the application to adjust the
soft limits.  Having to change the hard limit means the capacity
planning for the app is completely wrong.  A restart is certainly
acceptable in that case since it should really never happen.