Re: [RFC][PATCH 0/3] fork: Add the ability to create tasks with given pids

[Pavel Emelyanov <xemul@parallels.com>]

>> +static int pid_ns_ctl_permissions(struct nsproxy *namespaces,
>> +			struct ctl_table *table, int op)
>> +{
>> +	int mode = 0644;
>> +
>> +	if ((op & MAY_OPEN) &&
>> +			current != namespaces->pid_ns->child_reaper)
>> +		/*
>> +		 * Writing to this sysctl is allowed only for init
>> +		 * and to whoever it grands the open file
>> +		 */
>> +		mode &= ~0222;
>> +
>> +	return sysctl_test_perm(mode, op);
>> +}
>> +
>> +static struct ctl_table_root pid_ns_root = {
>> +	.permissions = pid_ns_ctl_permissions,
>> +};
> 
> Hmmm... I hope this could be prettier.  I'm having trouble following
> where the MAY_OPEN comes from.  Can you please explain?

>From this calltrace:

 pid_ns_ctl_permissions
 sysctl_perm
 proc_sys_permission
 inode_permission
 do_last <<<<< MAY_OPEN appears here
 path_openat
 do_filp_open
 do_sys_open
 sys_open


> Can't we for now allow this for root and then later allow CAP_CHECKPOINT 
> that Cyrill suggested?  Or do we want to allow setting pids even w/o CR 
> for NS creator?

I think that systemd guys can play with it. E.g. respawning daemons with predefined
pids sounds like an interesting thing to play with.

>> +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;
>> +	tmp.data = &current->nsproxy->pid_ns->last_pid;
>> +	return proc_dointvec(&tmp, write, buffer, lenp, ppos);
>> +}
> 
> Probably better to call set_last_pid() on write path instead?

Why? The usage of this sysctl is going to be synchronized  by external locks,
so why should we care?

>> Well, after a bit more thinking I found one more pros for this
>> sysctl - when restoring a container we'll have the possibility to
>> set the last_pid to what we want to prevent the pids reuse after the
>> restore.
> 
> Hmmm... I personally like this one better.  Restoring multilevel pids
> would be more tedious but should still be possible and I really like
> that it's staying out of clone path and all modifications are to ns
> and pid code.  Oleg, what do you think?
> 
> Thank you.
>