Messages in this thread | | | Date | Thu, 08 Oct 2009 22:55:04 +0200 | From | Jiri Slaby <> | Subject | Re: [PATCH v2 8/8] FS: proc, make limits writable |
| |
On 09/04/2009 04:26 PM, Oleg Nesterov wrote: > One small nit, just to suggest the further 9/8 cleanup, > >> +static const struct file_operations proc_pid_limits_operations = { >> + .read = proc_info_read, >> + .write = limits_write, >> +}; > > I think it makes sense to tweak proc_pid_limits() a little bit (and > rename it), so that we can do > > .read = limits_read, > .write = limits_write > > Then, > >> @@ -2501,7 +2571,9 @@ static const struct pid_entry tgid_base_stuff[] = { >> + NOD("limits", S_IFREG|S_IRUSR|S_IWUSR, NULL, >> + &proc_pid_limits_operations, >> + { .proc_read = proc_pid_limits }), > > We could use > > REG("limits", S_IRUSR|S_IWUSR, &proc_pid_limits_operations), > > instead, this looks a bit cleaner to me.
Hi again, nobody picked them up yet, I waited till the end of the merge window and now I'll repost.
Did you mean here to do the proc_info_read work (get/put task, alloc buf, simple_read) directly in limits_read?
> And another minor nit (just in case you will re-submit this series for > some reason). Perhaps the changelog in 6/8 should mention that we do > not do any security checks when tsk != current (without selinux). We > assume that either the caller is sys_setrlimit(), or the caller should > verify it has rights to change the limits: in case of limits_write() > we rely on ->mode = S_IRUSR|S_IWUSR.
I did it as a comment by the setrlimit. I think nobody would care about a changelog note ;).
| |