[PATCH] making /proc/<pid>/limits writable

[PATCH] making /proc/<pid>/limits writable

[Anders Johansson]

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

(For any replies, please cc. me, because I'm not subscribed)

Attached is a patch making the limits for a process writable, so it is 
possible to change the limits of a task other than current.

There are many reasons why this is desirable. The core limit is to me the most 
important. If a process hangs, because of some sort of race condition that 
happens once in a blue moon, and ulimit -c is set to 0, it might take forever 
to reproduce. It would be nice to be able to change that limit dynamically, to 
be able to get a core dump.

Other limits should also be settable, but for now I've only done core size.

The patch is against 2.6.27-rc1

btw, in case it isn't painfully obvious: this is my very first kernel patch 
that isn't a simple one-or-two-line bug fix

Anders

[-- Attachment #2: pid_limits.diff --]
[-- Type: text/x-patch, Size: 3600 bytes --]

diff -ur a/fs/proc/base.c b/fs/proc/base.c
--- a/fs/proc/base.c	2008-07-30 21:44:44.000000000 +0200
+++ b/fs/proc/base.c	2008-07-30 21:53:07.000000000 +0200
@@ -423,6 +423,76 @@
 
 #endif
 
+static ssize_t pid_limits_write(struct file *file, const char __user *buf,
+				size_t count, loff_t *offs)
+{
+	int ret = 0;
+	struct task_struct *task;
+
+	if (!count)
+		goto out_no_task;
+	char c;
+	char *s = buf, *tmp;
+	char buffer[256];
+	unsigned long softlim, hardlim;
+	char hardlim_set = 0;
+
+	task = get_proc_task(file->f_dentry->d_inode);
+	if (!task) {
+		ret = -ESRCH;
+		goto out_no_task;
+	}
+	if (get_user(c, s)) {
+		ret = -EFAULT;
+		goto out;
+	}
+	s += 2;
+	switch (c) {
+		case 'c': {
+			if (copy_from_user(buffer, s, 255)) {
+				ret = -EFAULT;
+				goto out;
+			}
+			if (strncmp(buffer, "unlimited", 9) == 0)
+				softlim = RLIM_INFINITY;
+			else {
+				softlim = strict_strtoul(buffer, &tmp, 10);
+				if (tmp == buffer) {
+					ret = -EFAULT;
+					goto out;
+				}
+				softlim *= 1024;
+			}
+			if ((tmp - buffer) < count) {
+				s = tmp+1;
+				if (strncmp(s, "unlimited", 9) == 0) {
+					hardlim = RLIM_INFINITY;
+					hardlim_set = 1;
+				} else {
+					hardlim = strict_strtoul(s, &tmp, 10);
+					if (s != tmp) {
+						hardlim *= 1024;
+						hardlim_set = 1;
+					}
+				}
+			}
+			task_lock(task->group_leader);
+			task->signal->rlim[RLIMIT_CORE].rlim_cur = softlim;
+			if (hardlim_set) {
+				if ((hardlim <= task->signal->rlim[RLIMIT_CORE].rlim_max) ||
+					capable(CAP_SYS_ADMIN))
+					task->signal->rlim[RLIMIT_CORE].rlim_max = hardlim;
+			}
+			task_unlock(task->group_leader);
+		}
+	}
+	ret = count;
+out:
+	put_task_struct(task);
+out_no_task:
+	return ret;
+}
+
 /* The badness from the OOM killer */
 unsigned long badness(struct task_struct *p, unsigned long uptime);
 static int proc_oom_score(struct task_struct *task, char *buffer)
@@ -462,19 +532,28 @@
 };
 
 /* Display limits for a process */
-static int proc_pid_limits(struct task_struct *task, char *buffer)
+static int pid_limits_read(struct file *file, char __user *buf,
+				size_t cnt, loff_t *ppos)
 {
 	unsigned int i;
 	int count = 0;
 	unsigned long flags;
-	char *bufptr = buffer;
+	char *bufptr = buf;
+	int ret = -ESRCH;
 
 	struct rlimit rlim[RLIM_NLIMITS];
+	struct task_struct *task;
 
+	if (*ppos > 0)
+		return 0;
+
+	task = get_proc_task(file->f_dentry->d_inode);
+	if (!task)
+		goto out_no_task;
 	rcu_read_lock();
 	if (!lock_task_sighand(task,&flags)) {
 		rcu_read_unlock();
-		return 0;
+		goto out;
 	}
 	memcpy(rlim, task->signal->rlim, sizeof(struct rlimit) * RLIM_NLIMITS);
 	unlock_task_sighand(task, &flags);
@@ -506,8 +585,13 @@
 		else
 			count += sprintf(&bufptr[count], "\n");
 	}
-
-	return count;
+	
+	ret = count;
+	*ppos = count;
+out:
+	put_task_struct(task);
+out_no_task:
+	return ret;
 }
 
 #ifdef CONFIG_HAVE_ARCH_TRACEHOOK
@@ -2456,7 +2540,7 @@
 	REG("environ",    S_IRUSR, environ),
 	INF("auxv",       S_IRUSR, pid_auxv),
 	ONE("status",     S_IRUGO, pid_status),
-	INF("limits",	  S_IRUSR, pid_limits),
+	REG("limits",	  S_IRUSR|S_IWUSR, pid_limits),
 #ifdef CONFIG_SCHED_DEBUG
 	REG("sched",      S_IRUGO|S_IWUSR, pid_sched),
 #endif
@@ -2791,7 +2875,7 @@
 	REG("environ",   S_IRUSR, environ),
 	INF("auxv",      S_IRUSR, pid_auxv),
 	ONE("status",    S_IRUGO, pid_status),
-	INF("limits",	 S_IRUSR, pid_limits),
+	REG("limits",	 S_IRUSR|S_IWUSR, pid_limits),
 #ifdef CONFIG_SCHED_DEBUG
 	REG("sched",     S_IRUGO|S_IWUSR, pid_sched),
 #endif
Only in b/fs/proc: base.c.orig
Only in b/fs/proc: base.c.rej
Only in b/fs/proc: .base.c.rej.swp

Re: [PATCH] making /proc/<pid>/limits writable

[KOSAKI Motohiro]

Hi Andres,

> Attached is a patch making the limits for a process writable, so it is
> possible to change the limits of a task other than current.
>
> There are many reasons why this is desirable. The core limit is to me the most
> important. If a process hangs, because of some sort of race condition that
> happens once in a blue moon, and ulimit -c is set to 0, it might take forever
> to reproduce. It would be nice to be able to change that limit dynamically, to
> be able to get a core dump.
>
> Other limits should also be settable, but for now I've only done core size.

Hmm..
I agree to your motivation.

but I have two objection.
 - this interface is slightly ugly. end user can't lean easily.
 - you don't write any documentation nor patch description about how to use.
   So, I needed to read source code for know about how to use.

Re: [PATCH] making /proc/<pid>/limits writable

[Jiri Slaby]

On 07/30/2008 09:57 PM, Anders Johansson wrote:
> (For any replies, please cc. me, because I'm not subscribed)
> 
> Attached is a patch making the limits for a process writable, so it is 
> possible to change the limits of a task other than current.
> 
> There are many reasons why this is desirable. The core limit is to me the most 
> important. If a process hangs, because of some sort of race condition that 
> happens once in a blue moon, and ulimit -c is set to 0, it might take forever 
> to reproduce. It would be nice to be able to change that limit dynamically, to 
> be able to get a core dump.
> 
> Other limits should also be settable, but for now I've only done core size.
> 
> The patch is against 2.6.27-rc1
> 
> btw, in case it isn't painfully obvious: this is my very first kernel patch 
> that isn't a simple one-or-two-line bug fix


diff -ur a/fs/proc/base.c b/fs/proc/base.c
--- a/fs/proc/base.c	2008-07-30 21:44:44.000000000 +0200
+++ b/fs/proc/base.c	2008-07-30 21:53:07.000000000 +0200
@@ -423,6 +423,76 @@

  #endif

+static ssize_t pid_limits_write(struct file *file, const char __user *buf,
+				size_t count, loff_t *offs)
+{
+	int ret = 0;
+	struct task_struct *task;
+
+	if (!count)
+		goto out_no_task;

Mixing decls with code causes warnings.

+	char c;
+	char *s = buf, *tmp;

This is sparse unfriendly. Try to build with C=1. This must have generate a 
compiler warning too (about the const).

+	char buffer[256];
+	unsigned long softlim, hardlim;
+	char hardlim_set = 0;
+
+	task = get_proc_task(file->f_dentry->d_inode);
+	if (!task) {
+		ret = -ESRCH;
+		goto out_no_task;
+	}
+	if (get_user(c, s)) {
+		ret = -EFAULT;
+		goto out;
+	}
+	s += 2;
+	switch (c) {
+		case 'c': {

I think this indent is not scripts/checkpatch.pl compliant as suggested by some 
people already.

+			if (copy_from_user(buffer, s, 255)) {

Hmm, does this work? It should return unable-to-copy bytes which would mean to 
return 255-count-2. Some kind of min(count-2, 255U); is needed here, I think.

+				ret = -EFAULT;
+				goto out;
+			}
+			if (strncmp(buffer, "unlimited", 9) == 0)
+				softlim = RLIM_INFINITY;
+			else {
+				softlim = strict_strtoul(buffer, &tmp, 10);

This doesn't compile, I suppose? Anyway you need buffer[255] = '\0'; somewhere 
before this.

+				if (tmp == buffer) {
+					ret = -EFAULT;
+					goto out;
+				}
+				softlim *= 1024;
+			}
+			if ((tmp - buffer) < count) {

"Use of unitialized value" here? tmp & unlimited path.