lkml.org 
[lkml]   [2011]   [Nov]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC][PATCH 0/3] fork: Add the ability to create tasks with given pids
On 11/22/2011 07:23 PM, Tejun Heo wrote:
> Hello,
>
> On Tue, Nov 22, 2011 at 03:11:02PM +0400, Pavel Emelyanov wrote:
>>> 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
>
> Thanks a lot. :)
>
>>> 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.
>
> But wouldn't CAP_CHECKPOINT be enough for systemd?

It would, but what's the point in granting to a systemd (which can be a container's
init by the way) the ability to use the _whole_ checkpoint/restore engine?

Even more - protecting with the capability implies, that any task might want to play
with it. But what's the point for an arbitrary task, that just _lives_ in a pid namespace
to set the last_pid of its namespace?

>>>> +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?
>
> I think the question should usually be the other way around. Why
> deviate when the deviation doesn't earn any tangible benefit? If you
> think setting it explicitly is justified, explain why in the comment
> of the setter and places where those explicit settings are.

The set_last_pid() is the way to update the last_pid by two concurrent updaters. Since
setting the last_pid via sysctl is racy by its nature, using that race protection is
just pointless.

And yes, I agree, that writing this comment is a good idea :)

> Thanks.
>



\
 
 \ /
  Last update: 2011-11-22 17:33    [W:0.092 / U:0.428 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site