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

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4ECB8346.8040806@parallels.com>
Date:	Tue, 22 Nov 2011 15:11:02 +0400
From:	Pavel Emelyanov <xemul@...allels.com>
To:	Tejun Heo <tj@...nel.org>
CC:	Oleg Nesterov <oleg@...hat.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Alan Cox <alan@...ux.intel.com>,
	Roland McGrath <roland@...k.frob.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Cyrill Gorcunov <gorcunov@...nvz.org>,
	James Bottomley <jbottomley@...allels.com>
Subject: Re: [RFC][PATCH 0/3] fork: Add the ability to create tasks with given
 pids

>> +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.
> 

--
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/