lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 28 Sep 2009 21:14:55 -0400 From: Neil Horman <nhorman@...driver.com> To: Andrew Morton <akpm@...ux-foundation.org> Cc: linux-kernel@...r.kernel.org Subject: Re: [PATCH] proc: augment /proc/pid/limits to allow setting of process limits. On Mon, Sep 28, 2009 at 03:44:03PM -0700, Andrew Morton wrote: > On Mon, 28 Sep 2009 16:06:00 -0400 > Neil Horman <nhorman@...driver.com> wrote: > > > Augment /proc/<pid>/limits file to support limit setting > > > > It was suggested to me recently that we support a mechanism by which we can set > > various process limits from points external to the process. The reasoning being > > that some processes are very long lived, and it would be beneficial to these > > long lived processes if we could modify their various limits without needing to > > kill them, adjust the limits for the user and restarting them. While individual > > application can certainly export this control on their own, it would be nice if > > such functionality were available to a sysadmin, without needing to have each > > application re-invent the wheel. > > > > As such, I've implemented the below patch, which makes /proc/pid/limits writable > > for each process. By writing the following format: > > <limit> <current value> <max value> > > to the limits file, an administrator can now dynamically change the limits for > > the respective process. Tested by myself with good results. > > > > Confused. This appears to allow processes to cheerily exceed their > inherited limits, without bound. See sys_setrliit()'s > > if (new_rlim.rlim_cur > new_rlim.rlim_max) > return -EINVAL; > Gaahh! You're right, in my worry to get all the string parsing right, I didn't even consider the semantics of setrlimit. > It might allow user A to diddle user B's limit too, I didn't check? > No, it can't do that. file permissions only allow the process owner and root to modify the limits. > And it cheerily avoids security_task_setrlimit() too. > Yeah, it completely breaks that. Sorry. > Apart from those somewhat fatal problems, it's all a bit unpleasing that > we now have two ways of setting rlimits, one of which is a superset of > the other. Perhaps a better way would be a new sys_setrlimit2() which > takes a pid (in the current pid namespace, one assumes). Then deprecate > sys_setrlimit(). > Do you think its worth adding a syscall just for this? I think theres merit in this feature (I wrote it :)), but I'm not sure if syscall is really warranted. you're above notes are obviously a problem, but I think they can be fixed. Its easy to make sure that if the writing user is the process owner and restrict the max value raising, and the selinux check can be added. clearly I rescind this patch (sorry for the noise). I'll see if I can add the checks needed above and repost. Neil > > > > ... > > > > +static ssize_t proc_pid_limit_write(struct file *file, const char __user *buf, > > + size_t count, loff_t *ppos) > > +{ > > + char *buffer; > > + char *element, *vmc, *vmm; > > + unsigned long long valuec, valuem; > > + unsigned long flags; > > + int i; > > + int index = -1; > > + size_t wcount = 0; > > + struct task_struct *task = get_proc_task(file->f_path.dentry->d_inode); > > + > > + > > + if (*ppos != 0) > > + goto out; > > + > > + if (count > 128) > > + goto out; > > + buffer = kzalloc(128, GFP_KERNEL); > > + > > + if (!buffer) > > + goto out; > > + > > + element = kzalloc(sizeof(buffer), GFP_KERNEL); > > + vmc = kzalloc(sizeof(buffer), GFP_KERNEL); > > + vmm = kzalloc(sizeof(buffer), GFP_KERNEL); > > + > > + if (!element || !vmm || !vmc) > > + goto out_free; > > + > > + wcount = count - copy_from_user(buffer, buf, count); > > + if (wcount < count) > > + goto out_free; > > + > > + i = sscanf(buffer, "%s %s %s", element, vmc, vmm); > > + > > + if (i < 3) > > + goto out_free; > > + > > + for (i = 0; i <= strlen(element); i++) > > + element[i] = tolower(element[i]); > > + > > + if (!strncmp(vmc, "unlimited", 9)) > > + valuec = RLIM_INFINITY; > > + else > > + valuec = simple_strtoull(vmc, NULL, 10); > > + > > + if (!strncmp(vmm, "unlimited", 9)) > > + valuem = RLIM_INFINITY; > > + else > > + valuem = simple_strtoull(vmm, NULL, 10); > > + > > + for (i = 0; i < RLIM_NLIMITS; i++) { > > + if ((lnames[i].match) && > > + !strncmp(element, lnames[i].match, > > + strlen(lnames[i].match))) { > > + index = i; > > + break; > > + } > > + } > > + > > + if (!lock_task_sighand(task, &flags)) > > + goto out_free; > > The function silently does nothing if lock_task_sighand() fails. > > > + if (index >= 0) { > > + task->signal->rlim[index].rlim_cur = valuec; > > + task->signal->rlim[index].rlim_max = valuem; > > + } > > + > > + unlock_task_sighand(task, &flags); > > + > > +out_free: > > + kfree(element); > > + kfree(vmc); > > + kfree(vmm); > > + kfree(buffer); > > +out: > > + *ppos += count; > > + put_task_struct(task); > > return count; > > } > > > > + > > +static const struct file_operations proc_limit_operations = { > > + .read = proc_pid_limit_read, > > + .write = proc_pid_limit_write, > > whitespace got munged. > > > +}; > > + > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists