sysctl: Add the kernel.ns_last_pid control
Subsystems (2)
PATCH 1/1 sysctl: Add the kernel.ns_last_pid control
The sysctl works on the current task's pid namespace, getting and setting its last_pid field.
Writing is allowed for CAP_SYS_ADMIN-capable tasks thus making it possible to create a task with desired pid value. This ability is required badly for the checkpoint/restore in userspace.
This approach suits all the parties for now.
--- Documentation/sysctl/kernel.txt | 8 ++++++++ kernel/pid.c | 4 +++- kernel/pid_namespace.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletions(-)
Documentation/sysctl/kernel.txt +8 -0
@@ -401,6 +401,14 @@ PIDs of value pid_max or larger are not allocated. ============================================================== +ns_last_pid: + +The last pid allocated in the current (the one task using this sysctl +lives in) pid namespace. When selecting a pid for a next task on fork +kernel tries to allocate a number starting from this one. + +============================================================== + powersave-nap: (PPC only) If set, Linux-PPC will use the 'nap' mode of powersaving,
kernel/pid.c +3 -1
@@ -137,7 +137,9 @@ static int pid_before(int base, int a, int b) }/* -* We might be racing with someone else trying to set pid_ns->last_pid. +* We might be racing with someone else trying to set pid_ns->last_pid +* at the pid allocation time (there's also a sysctl for this, but racing +* with this one is OK, see comment in kernel/pid_namespace.c about it). * We want the winner to have the "later" value, because if the * "earlier" value prevails, then a pid may get reused immediately. *
kernel/pid_namespace.c +31 -0
@@ -191,9 +191,40 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)return ; } +static int pid_ns_ctl_handler (struct ctl_table *table,int write, +void __user *buffer,size_t *lenp,loff_t *ppos) +{ +struct ctl_table tmp = *table; + +if (write && !capable (CAP_SYS_ADMIN )) +return -EPERM ; + +/* +* Writing directly to ns' last_pid field is OK, since this field +* is volatile in a living namespace anyway and a code writing to +* it should synchronize its usage with external means. +*/ + + tmp.data = ¤t->nsproxy->pid_ns->last_pid; +return proc_dointvec (&tmp, write, buffer, lenp, ppos); +} + +static struct ctl_table pid_ns_ctl_table[] = { + { + .procname ="ns_last_pid" , + .maxlen =sizeof (int ), + .mode =0666 ,/* permissions are checked in the handler */ + .proc_handler = pid_ns_ctl_handler, + }, + { } +}; + +static struct ctl_path kern_path[] = { { .procname ="kernel" , }, { } }; +static __init int pid_namespaces_init (void ) { pid_ns_cachep =KMEM_CACHE (pid_namespace,SLAB_PANIC ); +register_sysctl_paths (kern_path, pid_ns_ctl_table);return 0 ; }
Signature · 1 line
1.5.5.6
kernel/pid_namespace.c void zap_pid_ns_processes(struct pid_namespace *pid_ns) · line 206
Quoted 13 lines · Pavel Emelyanov
kernel/pid_namespace.c +13
-0+static int pid_ns_ctl_handler (struct ctl_table *table,int write, +void __user *buffer,size_t *lenp,loff_t *ppos) +{ +struct ctl_table tmp = *table; + +if (write && !capable (CAP_SYS_ADMIN )) +return -EPERM ; + +/* +* Writing directly to ns' last_pid field is OK, since this field +* is volatile in a living namespace anyway and a code writing to +* it should synchronize its usage with external means. +*/
I would still prefer using set_last_pid() but if you insist to update last_pid directly, please note the direct update in the comment on top of set_last_pid() too.
Other than that,
Thanks.
Signature · 1 line
tejun
Quoted 10 lines · Tejun Heo
Quoted 9 lines · Pavel Emelyanov
kernel/pid_namespace.c +9
-0+static int pid_ns_ctl_handler (struct ctl_table *table,int write, +void __user *buffer,size_t *lenp,loff_t *ppos) +{ +struct ctl_table tmp = *table; + +if (write && !capable (CAP_SYS_ADMIN )) +return -EPERM ; + +/*
Quoted 8 lines
kernel/pid_namespace.c void zap_pid_ns_processes(struct pid_namespace *pid_ns) · line 206
Quoted 4 lines
kernel/pid_namespace.c +4
-0+* Writing directly to ns' last_pid field is OK, since this field +* is volatile in a living namespace anyway and a code writing to +* it should synchronize its usage with external means. +*/ I would still prefer using
set_last_pid()but if you insist to updatelast_piddirectly, please note the direct update in the comment on top ofset_last_pid()too.
It's already there in this patch.
Quoted 3 lines
Other than that,
Acked-by Tejun Heo <[email protected]>
Thanks!
Quoted 2 lines
Thanks.
Quoted 5 lines · Pavel Emelyanov
Quoted 3 lines
I would still prefer using
set_last_pid()but if you insist to updatelast_piddirectly, please note the direct update in the comment on top ofset_last_pid()too.It's already there in this patch.
Heh, indeed. My eyes just skipped over them. :)
Thanks.
Signature · 1 line
tejun
Quoted 33 lines · Pavel Emelyanov
kernel/pid_namespace.c +26
-0@@ -191,9 +191,40 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)return ; } +static int pid_ns_ctl_handler (struct ctl_table *table,int write, +void __user *buffer,size_t *lenp,loff_t *ppos) +{ +struct ctl_table tmp = *table; + +if (write && !capable (CAP_SYS_ADMIN )) +return -EPERM ; + +/* +* Writing directly to ns' last_pid field is OK, since this field +* is volatile in a living namespace anyway and a code writing to +* it should synchronize its usage with external means. +*/ + + tmp.data = ¤t->nsproxy->pid_ns->last_pid; +return proc_dointvec (&tmp, write, buffer, lenp, ppos); +} + +static struct ctl_table pid_ns_ctl_table[] = { + { + .procname ="ns_last_pid" , + .maxlen =sizeof (int ), + .mode =0666 ,/* permissions are checked in the handler */ + .proc_handler = pid_ns_ctl_handler, + }, + { }
kernel/pid_namespace.c void zap_pid_ns_processes(struct pid_namespace *pid_ns) · line 229
Quoted 10 lines
kernel/pid_namespace.c +5
-0+}; + +static struct ctl_path kern_path[] = { { .procname ="kernel" , }, { } }; +static __init int pid_namespaces_init (void ) { pid_ns_cachep =KMEM_CACHE (pid_namespace,SLAB_PANIC ); +register_sysctl_paths (kern_path, pid_ns_ctl_table);return 0 ; }
Hmm. This way it depends on CONFIG_PID_NS.
Can't we simply add an entry into kern_table[] ? And without ns_, just /proc/sys/kernel/last_pid.
Oleg.
Quoted 38 lines · Oleg Nesterov
Quoted 37 lines · Pavel Emelyanov
kernel/pid_namespace.c +30
-0@@ -191,9 +191,40 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)return ; } +static int pid_ns_ctl_handler (struct ctl_table *table,int write, +void __user *buffer,size_t *lenp,loff_t *ppos) +{ +struct ctl_table tmp = *table; + +if (write && !capable (CAP_SYS_ADMIN )) +return -EPERM ; + +/* +* Writing directly to ns' last_pid field is OK, since this field +* is volatile in a living namespace anyway and a code writing to +* it should synchronize its usage with external means. +*/ + + tmp.data = ¤t->nsproxy->pid_ns->last_pid; +return proc_dointvec (&tmp, write, buffer, lenp, ppos); +} + +static struct ctl_table pid_ns_ctl_table[] = { + { + .procname ="ns_last_pid" , + .maxlen =sizeof (int ), + .mode =0666 ,/* permissions are checked in the handler */ + .proc_handler = pid_ns_ctl_handler, + }, + { } +}; + +static struct ctl_path kern_path[] = { { .procname ="kernel" , }, { } }; +
Quoted 8 lines
kernel/pid_namespace.c void zap_pid_ns_processes(struct pid_namespace *pid_ns) · line 229
Quoted 6 lines
kernel/pid_namespace.c +1
-0static __init int pid_namespaces_init (void ) { pid_ns_cachep =KMEM_CACHE (pid_namespace,SLAB_PANIC ); +register_sysctl_paths (kern_path, pid_ns_ctl_table);return 0 ; }Hmm. This way it depends on
CONFIG_PID_NS.
Yes, since this _is_ for namespaces. As we've found out this is close to completely unusable in the initial namespace in which tasks are just forking without caring much about what CAP_SYS_ADMIN-s think about this.
Quoted 1 line
Can't we simply add an entry into kern_table[] ?
And store the .proc_handler function dealing with somewhat which is pid namespace specific in the same generic file?
Quoted 1 line
And without
ns_, just/proc/sys/kernel/last_pid.
But that's the namespace's last pid, not just some system-wide last pid.
Quoted 4 lines
Oleg.
.
Quoted 14 lines · Pavel Emelyanov
Quoted 11 lines · Oleg Nesterov
kernel/pid_namespace.c void zap_pid_ns_processes(struct pid_namespace *pid_ns) · line 229
Quoted 9 lines
kernel/pid_namespace.c +4
-0+ +static struct ctl_path kern_path[] = { { .procname ="kernel" , }, { } }; +static __init int pid_namespaces_init (void ) { pid_ns_cachep =KMEM_CACHE (pid_namespace,SLAB_PANIC ); +register_sysctl_paths (kern_path, pid_ns_ctl_table);return 0 ; }Hmm. This way it depends on
CONFIG_PID_NS.
Quoted 3 lines
Yes, since this _is_ for namespaces. As we've found out this is close to completely unusable in the initial namespace in which tasks are just forking without caring much about what CAP_SYS_ADMIN-s think about this.
I agree, it is not very much usable. Still I think it can be used. Say, init can write RESERVED_PIDS to this file. Or you can use it to test the pid-reuse problems.
Quoted 4 lines
Quoted 1 line
Can't we simply add an entry into kern_table[] ?
And store the .proc_handler function dealing with somewhat which is pid namespace specific in the same generic file?
Why not? In fact I think that, say, /proc/sys/kernel/pid_max should act per-namespace too.
Quoted 3 lines
Quoted 1 line
And without
ns_, just/proc/sys/kernel/last_pid.But that's the namespace's last pid, not just some system-wide last pid.
Sure, it is not system wide. Unless you use it from the root ns.
OK. I do not really care. I think the patch is correct, lets do it this way.
Oleg.
Quoted 8 lines · On Mon, 28 Nov 2011 19:21:25 +0400 Pavel Emelyanov <[email protected]> wrote:
The sysctl works on the current task's pid namespace, getting and setting its
last_pidfield.Writing is allowed for CAP_SYS_ADMIN-capable tasks thus making it possible to create a task with desired pid value. This ability is required badly for the checkpoint/restore in userspace.
This approach suits all the parties for now.
I'm checking this November patch prior to sending it to Linus...
Quoted 68 lines
Documentation/sysctl/kernel.txt +8
-0@@ -401,6 +401,14 @@ PIDs of value pid_max or larger are not allocated. ============================================================== +ns_last_pid: + +The last pid allocated in the current (the one task using this sysctl +lives in) pid namespace. When selecting a pid for a next task on fork +kernel tries to allocate a number starting from this one. + +============================================================== + powersave-nap: (PPC only) If set, Linux-PPC will use the 'nap' mode of powersaving,kernel/pid.c +3
-1@@ -137,7 +137,9 @@ static int pid_before(int base, int a, int b) }/* -* We might be racing with someone else trying to set pid_ns->last_pid. +* We might be racing with someone else trying to set pid_ns->last_pid +* at the pid allocation time (there's also a sysctl for this, but racing +* with this one is OK, see comment in kernel/pid_namespace.c about it). * We want the winner to have the "later" value, because if the * "earlier" value prevails, then a pid may get reused immediately. * kernel/pid_namespace.c +26
-0@@ -191,9 +191,40 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)return ; } +static int pid_ns_ctl_handler (struct ctl_table *table,int write, +void __user *buffer,size_t *lenp,loff_t *ppos) +{ +struct ctl_table tmp = *table; + +if (write && !capable (CAP_SYS_ADMIN )) +return -EPERM ; + +/* +* Writing directly to ns' last_pid field is OK, since this field +* is volatile in a living namespace anyway and a code writing to +* it should synchronize its usage with external means. +*/ + + tmp.data = ¤t->nsproxy->pid_ns->last_pid; +return proc_dointvec (&tmp, write, buffer, lenp, ppos); +} + +static struct ctl_table pid_ns_ctl_table[] = { + { + .procname ="ns_last_pid" , + .maxlen =sizeof (int ), + .mode =0666 ,/* permissions are checked in the handler */ + .proc_handler = pid_ns_ctl_handler, + }, + { }
kernel/pid_namespace.c void zap_pid_ns_processes(struct pid_namespace *pid_ns) · line 229
Quoted 10 lines
kernel/pid_namespace.c +5
-0+}; + +static struct ctl_path kern_path[] = { { .procname ="kernel" , }, { } }; +static __init int pid_namespaces_init (void ) { pid_ns_cachep =KMEM_CACHE (pid_namespace,SLAB_PANIC ); +register_sysctl_paths (kern_path, pid_ns_ctl_table);return 0 ; }
I think we should now make this code conditional on the new CONFIG_CHECKPOINT_RESTORE. I'll merge the patch as-is and will ask you or Cyrill to send a followup patch doing this, please?
I'll confess that part of my motivation for wrapping c/r-specific code inside CONFIG_CHECKPOINT_RESTORE is to make it easy for us to later delete it all if your c/r project end up being unsuccessful. Sorry :)