From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751409AbdEBJx3 (ORCPT ); Tue, 2 May 2017 05:53:29 -0400 Received: from mail-eopbgr10100.outbound.protection.outlook.com ([40.107.1.100]:1376 "EHLO EUR02-HE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751249AbdEBJxW (ORCPT ); Tue, 2 May 2017 05:53:22 -0400 Authentication-Results: chromium.org; dkim=none (message not signed) header.d=none;chromium.org; dmarc=none action=none header.from=virtuozzo.com; Subject: Re: [PATCH v3] pid_ns: Introduce ioctl to set vector of ns_last_pid's on ns hierarhy To: "Eric W. Biederman" References: <149329634856.21195.14196911999722279118.stgit@localhost.localdomain> <87mvb16fv7.fsf@xmission.com> <12a73543-79ea-4bac-7e96-6ab237534af2@virtuozzo.com> <877f254yx0.fsf@xmission.com> <8737crt4dz.fsf@xmission.com> CC: , , , , , , , , , , , , , , , From: Kirill Tkhai Message-ID: Date: Tue, 2 May 2017 12:53:15 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <8737crt4dz.fsf@xmission.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [195.214.232.6] X-ClientProxiedBy: AM4PR0501CA0015.eurprd05.prod.outlook.com (10.167.83.153) To DB6PR0802MB2280.eurprd08.prod.outlook.com (10.172.228.8) X-MS-Office365-Filtering-Correlation-Id: ebf2a7b7-cff6-4213-8dcc-08d491410d98 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(201703131423075)(201703031133081);SRVR:DB6PR0802MB2280; X-Microsoft-Exchange-Diagnostics: 1;DB6PR0802MB2280;3:DOzgnQM/vs6Cx6US0l+jSIL2wY1uAtXCU9STkh+GS1cir68tqc7wgl8bcb8G56jRBDxuS/61VLnx8n//RHhpBp1n663Uw9zyxNRwiFh7eqVT7M8wYtzbHSUR/9EtuZd4HU16ESuQbScyHc243MYpWPRwYs88kqr6vL4tZNm8YNgfXxmKksQFERzfxXFa/HJwv3eydjvm7ga1hi+BhjHrWPK1H3TmIoHQ3ZIOVGCVvjD1K4bjzIQVDnwXeRNt2dumRuWpgyicgC1ciW0PAfCSbnj3pV/Xcbm5QfJgjGKOOCp3H+OXatRsXZGwxv6MGLW4nAdscH9vzDH7ynVglrcuqw==;25:QVAAWLHaqQPNFh32WVqNSrsw3Yr41JbWZJuPu088/pbgx4waXFRNuPJdcdSASk+CCEWypp2TNulwgMWtxze3DqoFLWdK66JXYXqSDS19m1Vx6tNLPNkWQLdaNrNk7u0myV2kGWuZfxPxF1gRMEvbeeE2sqfnn/vCVHYP2EYGwoXrifzh/Oira8+9jpElycR10aKcj+8OTXFTfNnnFcddtbug2pAGsGuHAYKcvtVmCdSNnG/+DNz2Lb9hcgLmlbjOF/GPj5UegoxgKs1bxmq2ziSOBfjw8lGu9lEr6k5i7YblP9xBmHjDebQFcR0vIyWIn2hF5YTQDSr+efwJedjT1NHlv4r31JcBQ0nYEGcOWslcRtf/mJ+HH6md/4w9F7cqgajx5KtMFrp9hTZbRITP8VffnToKeWa9B7/tnaV/60GY4fXUQW+dl+2HzXskyAne X-Microsoft-Exchange-Diagnostics: 1;DB6PR0802MB2280;31:hPd6N7V6JCOK2AxTDgqZVO41JTKZ61PLzkxucmVWUcseiNx498RtQCfSBA7apE+J8RMuWwpR2Yn1JvT3SJex09zk93/xv0jIYT0LscqJDx6JFl2ry+XrJcskY1AP7DtdTX/FNKpWyTnwwdOU5DiHJcnhMe3Zycb8PP3jKZ407avhQSWXsKC2DYi7jLG/oQ62f+VjSsJ7cmVI1qU7natdAtwfR/IVlz0RliXH/Rft3wMbs8yIAO1EDeKyNKspr2k3;20:watYvF0nc6pq0mL+fvLxaaCh9xAaGPj6YnwpWBqUwquDGaCqcGtDpCZWTGKdssyUrxGzUhSgz3IKH4laf5j/DC9AW3c+UZA9pucOzLblzwF89PP/XxgVwtiRbykBpjqVDMyl+jLu6Y7udAGflLZd9QUxvpzmPBQrJ0XlZryT1Fp+LLFRu+OD4i18oI/8WLO7VqtZ+qvDs6Mwoa4K8M3DgapFcXbSdnGQfWtQWvDGR5K4bi4t/9w4linstxlprYu39+ActqGkzPHWZXwDwY5rTH7QEBH2tcwnTHPfXOKvDZKq7eGzzExKENmKXDxn1jj5gmGgixXFedH8LvZHztHNlShnJOYR8Y2ouUDD3uXMRxcngEWwP8mzxSbA/F0KkLG8KoNN7WUJ3sMa40is4oQXHiKzNGE66QgxGf2l3aSsPc4= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040450)(601004)(2401047)(8121501046)(5005006)(93006095)(93001095)(3002001)(10201501046)(6041248)(20161123558100)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123560025)(20161123555025)(20161123562025)(20161123564025)(6072148);SRVR:DB6PR0802MB2280;BCL:0;PCL:0;RULEID:;SRVR:DB6PR0802MB2280; X-Microsoft-Exchange-Diagnostics: 1;DB6PR0802MB2280;4:oGxQ8RYSFSoBsdylYDpJrEgFVg5lPG9mpGIpNLtfkGVl1suFgKRFZnnlSM4ke8x8j5SilQTBdLVXd0Ry1x/ZM+pRmHHUvD+oNAVPccGc71FTXc8o5qwQTkhpnOFX4H4SykUMt4DSUP9ZFvjRizk0xe3efTxxjkeVhdnAOwT5EBxfTULUm68b5Ki6xuziQoJMiDrTkSlw3LWLmfXFIKI1VeV9wjvNOtnUK5cSyjEIjSV1dhDahL1o33o9jidvqj4JoqUjpGfQVWE5eXsrzCD9AMFYJDSeuLoIi7Smzy8QCcy8T6wOEy6HbhtwCZkMXH+KMk3jZCUsvRk8fRuRjJDshbzT5Iv7ghDb26spz7yqBsHcaABRo/W3ojM/1ncbAzdHn4UGt4gh7yhGmb0mPVd2YtZz/W4PmUsCuk25/CEy1X6uycPg8JcH8dVacFfIJSvuV4HiyEvFTGF/MeDZ97YdO8OUBgejdEbKJlckgJi1hfUsQ7CkK17kN4xbBeiifn8HzltHgxd72uU4X4MoZ3NWxGtUA4cG486EQXe+CoTwwE3vbfWrXs5gXhH7TOqb0xdIvC0EnorREcP5kfMaKa4MbETV5JDdB9M/XeZU6GHxJbXgSzj0A+oJh5pFRfU14VUO4oqy7JBOrQO5GmRdzhr9QIngrY9L2ZChe32YXtAcNa6K31bHB1rlLsDweHdYvU5WNUCOZaGWJxnqR5JFUfuFVdOsHp0RVAbaEwAYtSJGA6zSzcqTt5MyOZNBnzGW2f/KMiQJcbrLRIwOHi3I5SVuz3Z7kHHnYGnKetKX0okTw7Y= X-Forefront-PRVS: 02951C14DC X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6049001)(6009001)(39830400002)(39400400002)(39410400002)(39450400003)(24454002)(47776003)(93886004)(66066001)(50466002)(2906002)(5660300001)(6116002)(83506001)(23676002)(3846002)(229853002)(31686004)(38730400002)(6246003)(42186005)(478600001)(110136004)(6916009)(50986999)(33646002)(31696002)(54356999)(305945005)(53936002)(4326008)(76176999)(4001350100001)(36756003)(575784001)(230700001)(65826007)(189998001)(8676002)(25786009)(53546009)(7736002)(81166006)(86362001)(6486002)(77096006)(2950100002);DIR:OUT;SFP:1102;SCL:1;SRVR:DB6PR0802MB2280;H:[172.16.25.137];FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtEQjZQUjA4MDJNQjIyODA7MjM6STRMelFreFRQMnFiMzJzMmQ0dzFmWDJN?= =?utf-8?B?OWJoeUNaVEFKV3FuYXRudnVINWxwQlZVazVyYUJXSk5ROGRpcmNXMlV4M2lI?= =?utf-8?B?V3AvZHovdUc5WmJFanpLZUZvT1ZSSFZBY1RNNXlzSEhDSXRZRCs5WHZ4UWFj?= =?utf-8?B?T3NoTHcra3gyUTBWamRDMzZibGV0bHN4VDNNWE5maThTcUJ1a2Z1MnczY1lZ?= =?utf-8?B?NVZOVVZtK1dLVGFCNWpkeEhSUENUcFIrd2UrYjFrWWJ6NmMwL2RHcmRoSFJx?= =?utf-8?B?MTk4SjNqZVNZaGNPWHFwak9QbHVCaDdYVVVVTW1JcU9wSFp6SC92SU1JaUZZ?= =?utf-8?B?YnlYUk5CVmYwV0ZWYkF4dWhkeUFPZmxvQ21LRHVUSUIyZDBBdTJYVTJ5YXg2?= =?utf-8?B?V1pjRWQvU3BqSjk5eHJ3UEdNYUowa2l4b1BYM1gwc3ZlaU1rWDZkbDRZa2Nq?= =?utf-8?B?b1JZSnJIeEcxWTlud29tZ3BsS1Z1dEJOMXVwaHVHSUhWcWdNYklXa3ZtUnhN?= =?utf-8?B?VVdEL0tsL1JvQ0RSd2ErUG5rRHhCb0F3OFljMjFaeU0waWlOZzNWUW1TaW5M?= =?utf-8?B?Tmg0YVFvUXp1YjdVbm01b3NVRUZkejRaQUpyMnNIblZ5NFp4bUllREdSWFdK?= =?utf-8?B?K04zZG50dUJ0YzJFWFd2ZXhZTXNtS2ZYMHR5SnBWTncvcnAxbkRLSDFNZ2NH?= =?utf-8?B?Q2xkU0xIT1FKTUFhTjZWT1NiMldXWDIxTjluRUZrZFVCVUZxWnZiZ0NOYU43?= =?utf-8?B?REtScUI3Y0JNNHVqSXI4QkN1RVhVN1JYQ2dkUThxUW5iT21vUVV4VDcxeEtC?= =?utf-8?B?dUhOQzVnRDNHRVlobU1LZzJPQmVuZUc3SDBoM00zNkFFeWIwNWRoOHFOclhl?= =?utf-8?B?QmtUMlo0b2wxVjl2c2Zia2FCTzRDRzYwdUJGZ3dKMXpuN3pFbVIxNDFWcGlC?= =?utf-8?B?YlJxbmtqWUlLSFBNa1JTVXh2OGl6TUdINTNyZG16NVNSRlgvS1dDL3ZEdGlO?= =?utf-8?B?Tmp1Y2luTFpreThBeDlCeXR0RnEwZkNuNFdkUTJ0VGVnN0psYUJKdGRldlVU?= =?utf-8?B?NWtVK05CUkwyKzdacWhIbkY3SEJnaVNxUWF2dG1RaW93eWxQQWZsRGQ2cm9l?= =?utf-8?B?aU5Mc1JEdStQRCtmTDBhejl2Y09SamsyaHJSTXVWR2FORlNmMk5TOGhXS3Rn?= =?utf-8?B?R1ZwWDJkY0FLQmEzS1NVRWVrWUhwRnplb3F0aklrR2MwQmFjNS9KaGhvbWZt?= =?utf-8?B?eGs1ZVNVRGVHV3kzeVpxOGZSQzhNTSt5Y0UzbEhkWnRzVXkwMWRKTFlvQ0sz?= =?utf-8?B?RzR0ZFc2WVF0SWRvTlowN3FnZ0xaLy9MMGVxekhXaW1ta0tIREtValdnTzFj?= =?utf-8?B?UFcrSDZQa1I1Q3NYektIVVNicXlFdlhoVmg3bHdwODY4ZE4xcnpOYkNpVy9k?= =?utf-8?B?VDA1ZURybTdYUHltMkNCdEVINTFSVVd5bmJHbXlJeENobU1jNkQ1VThaOTBt?= =?utf-8?B?OSt1eDBTT2FkbUFaemZZdGtLcnppNWRyYWU0VjFPRXFXZjJEU3BZakZYMWNz?= =?utf-8?B?ZWhqWlNqbFpQaHVnZTVKaXU4elFhSnpoSk9ML1hTUDU5cXg3bFhRRVdXOWNX?= =?utf-8?B?UTg2Z2YxcEQ4a2tMSndkMnBqRVV1b3VYWGxwdnVKUVRweEc0ZG9OMHlib1E9?= =?utf-8?Q?=3D?= X-Microsoft-Exchange-Diagnostics: 1;DB6PR0802MB2280;6:rFOX6Ku6hjdnYyFFurlGAMeeGdv69msFGUhBfzlVrbyMsqtDnJpmeDhzbtsgFtQEZMZX25JI1I38SkQfBOS3jUAKCG5NLNwdpPOTTcrJQ0/0nLDPxegpMSMdDCwLHJdL8qZuL3wVbWb0HBaXqfjzDrL0+vbKa4WqWQe8KFufghTbwBn9jNBBzz8i0cchs6rTZhAJIm+DeQ0AswfpzFrtt4rxQtGrWrJuPk5a6HB9Hva0soqty/vTlmzHZ/EWvb40IeSX8AGlUTFuEiQvQgvk3hj0ZvwmD7G5Rql1cghorppBhfPLfR2dD+EwoWFR89l5JPWBfi8siSY1p6DmcuXDYkRq+c+za5YUVhR5kPYux2x7a9rEDULaW0SSinBXuF76bpgMgioLkGzK5b2o7QG6tWraDOKDeNvz7EDxQif85cq7Qyu0GJurUXWlKZMKnkqsJ59NCmw65faOygguEtDlmuTPwPkcT0rS7R5s6WG3vBWW8xXcKvYaWZiwI5JcdlRQNonkzbTR/NEp8juL2TM01Q==;5:vC/QMEIvDyovInXhvMRTHNmYLDPYRg7XC8wb/0VqJkJnvDNtheebUBV47vfJjCcRluM/SITwdMVfQrhjaAQI7lcuGQlLf+/kNzY8PHmzsI4unI0GNV8g4+zRKOoDxGGqd4aqVldFPJjm+x4VqghqWJVoxLDtuwLsRlOmh4apeyo=;24:Cm+wLM6ICMDNCTTuZJzNMRULAyzJdswm2bVpQ98dyqfS6jk7DdVO86xp3wy30d2dAIPBqt1rltkzYo2UbiTUTiE09NZ3wTCgh2IkDkUKsV0= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;DB6PR0802MB2280;7:y4ZM+15+fbMeSOYY4rdddzH3QYinN9xQtU5ODyNIak+ew1/zAXquhxBYQS6JAy+sMu/NLtJ9kMaOsoUGgdPoH5DoGxGoMHh34fr2IyihEF72t3KE4YSP87S5kK/2OuQxHXmPI/qLzYDQJCT/CATfHECfpO9hvAEboa7W949BM2eOxGpS4yaUie9OWRR/gkiLhO3JbXKQSXJFbi++WP6KNB02rkO6HseoZC3q4wq9T4UAi+Yqv+IevVNLNNutkvXAAZgP0rzHtS05a3Ehmxjf9WsT5ccKq/i17ZjXSGPaJdTiRKQEywdgwKFWiE6YU9/1yvo0MttjqjHQ1tIdV1M9sQ==;20:NdJe8p10BorT2HRO8DYxvG4wwFeUAvPcVXLmPm4CWiYVcj9yDrkUAVgMEGrEx8H8dD4Nr3x/cV+fPDi6N7RpfDNFFC+M1z6CzLN+1RRq06dJ78Oq95n+H70OBeePk7jCRv+scHuosvL3PziqhBxgwJn8m27xKfBFncRBK2m7a5I= X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 May 2017 09:53:14.0405 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR0802MB2280 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 29.04.2017 22:12, Eric W. Biederman wrote: > Kirill Tkhai writes: > >> On 27.04.2017 19:07, Eric W. Biederman wrote: >>> Kirill Tkhai writes: >>> >>>> On 27.04.2017 18:15, Eric W. Biederman wrote: >>>>> Kirill Tkhai writes: >>>>> >>>>>> On implementing of nested pid namespaces support in CRIU >>>>>> (checkpoint-restore in userspace tool) we run into >>>>>> the situation, that it's impossible to create a task with >>>>>> specific NSpid effectively. After commit 49f4d8b93ccf >>>>>> "pidns: Capture the user namespace and filter ns_last_pid" >>>>>> it is impossible to set ns_last_pid on any pid namespace, >>>>>> except task's active pid_ns (before the commit it was possible >>>>>> to write to pid_ns_for_children). Thus, if a restored task >>>>>> in a container has more than one pid_ns levels, the restorer >>>>>> code must have a task helper for every pid namespace >>>>>> of the task's pid_ns hierarhy. >>>>>> >>>>>> This is a big problem, because of communication with >>>>>> a helper for every pid_ns in the hierarchy is not cheap. >>>>>> It's not performance-good as it implies many helpers wakeups >>>>>> to create a single task (independently, how you communicate >>>>>> with the helpers). This patch tries to decide the problem. >>>>> >>>>> I see the problem and we definitely need to do something. >>>>> Your patch does appear better than what we have been doing. >>>>> So a tenative conceptual ack. >>>>> >>>>> At the same time it is legitimate to claim that the use of >>>>> task_active_pid_ns(current) rather than >>>>> current->nsproxy->pid_ns_for_children is a regression caused by the >>>>> above commit. So we can fix the original issue as well. >>>>> >>>>> I do have to ask when was this problem discovered, and why did it take >>>>> so long to discover? The regeression happened nearly 5 years ago. >>>>> >>>>> Was CRIU already using this? >>>> >>>> CRIU uses ns_last_pid, but we never had nested pid namespace hierarchy. >>>> When there is only one level of pid namespaces, then active pid namespace >>>> is the save as pid_ns_for_children, so we never faced with this >>>> problem. >>> >>> Ok. So not a regression then. >>> >>>> Now we're working on Docker support, and its recent versions create nested >>>> pid namespaces (I have no information, when they begun to do that). So, >>>> we met this problem. >>>> >>>>> It looks like the fix is a one line low danger change to >>>>> /proc/sys/kernel/ns_last_pid. With a low danger as pid_ns_for_children >>>>> rarely differs from task_active_pid_ns(). >>>>> >>>>> >>>>>> It introduces a new pid_ns ioctl(NS_SET_LAST_PID_VEC), >>>>>> which allows to write a vector of last pids on pid_ns hierarchy. >>>>>> The vector is passed as array of pids in struct ns_ioc_pid_vec, >>>>>> written in reverse order. The first number corresponds to >>>>>> the opened namespace ns_last_pid, the second is to its parent, etc. >>>>>> So, if you have the pid namespaces hierarchy like: >>>>>> >>>>>> pid_ns1 (grand father) >>>>>> | >>>>>> v >>>>>> pid_ns2 (father) >>>>>> | >>>>>> v >>>>>> pid_ns3 (child) >>>>>> >>>>>> and the pid_ns3 is open, then the corresponding vector will be >>>>>> {last_ns_pid3, last_ns_pid2, last_ns_pid1}. This vector may be >>>>>> short and it may contain less levels. For example, >>>>>> {last_ns_pid3, last_ns_pid2} or even {last_ns_pid3}, in dependence >>>>>> of which levels you want to populate. >>>>>> >>>>>> v3: Use __u32 in uapi instead of unsigned int. >>>>>> >>>>>> v2: Kill pid_ns->child_reaper check as it's impossible to have >>>>>> such a pid namespace file open. >>>>>> Use generic namespaces ioctl() number. >>>>>> Pass pids as array, not as a string. >>>>>> >>>>>> Signed-off-by: Kirill Tkhai >>>>>> --- >>>>>> fs/nsfs.c | 5 +++++ >>>>>> include/linux/pid_namespace.h | 12 ++++++++++++ >>>>>> include/uapi/linux/nsfs.h | 7 +++++++ >>>>>> kernel/pid_namespace.c | 35 +++++++++++++++++++++++++++++++++++ >>>>>> 4 files changed, 59 insertions(+) >>>>>> >>>>>> diff --git a/fs/nsfs.c b/fs/nsfs.c >>>>>> index 323f492e0822..f669a1552003 100644 >>>>>> --- a/fs/nsfs.c >>>>>> +++ b/fs/nsfs.c >>>>>> @@ -6,6 +6,7 @@ >>>>>> #include >>>>>> #include >>>>>> #include >>>>>> +#include >>>>>> #include >>>>>> #include >>>>>> >>>>>> @@ -186,6 +187,10 @@ static long ns_ioctl(struct file *filp, unsigned int ioctl, >>>>>> argp = (uid_t __user *) arg; >>>>>> uid = from_kuid_munged(current_user_ns(), user_ns->owner); >>>>>> return put_user(uid, argp); >>>>>> + case NS_SET_LAST_PID_VEC: >>>>>> + if (ns->ops->type != CLONE_NEWPID) >>>>>> + return -EINVAL; >>>>>> + return pidns_set_last_pid_vec(ns, (void *)arg); >>>>>> default: >>>>>> return -ENOTTY; >>>>>> } >>>>>> diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h >>>>>> index c2a989dee876..c8dc4173a4e8 100644 >>>>>> --- a/include/linux/pid_namespace.h >>>>>> +++ b/include/linux/pid_namespace.h >>>>>> @@ -9,6 +9,7 @@ >>>>>> #include >>>>>> #include >>>>>> #include >>>>>> +#include >>>>> >>>>> No need for the extra include and slowing down the build. Just >>>>> declare the relevant structures. >>>> >>>> So, I'll write just: >>>> >>>> struct ns_ioc_pid_vec; >>>> >>>> instead of that. >>>> >>>>>> >>>>>> struct pidmap { >>>>>> atomic_t nr_free; >>>>>> @@ -103,6 +104,17 @@ static inline int reboot_pid_ns(struct pid_namespace *pid_ns, int cmd) >>>>>> } >>>>>> #endif /* CONFIG_PID_NS */ >>>>>> >>>>>> +#if defined(CONFIG_PID_NS) && defined(CONFIG_CHECKPOINT_RESTORE) >>>>>> +extern long pidns_set_last_pid_vec(struct ns_common *ns, >>>>>> + struct ns_ioc_pid_vec __user *vec); >>>>>> +#else /* CONFIG_PID_NS && CONFIG_CHECKPOINT_RESTORE */ >>>>>> +static inline long pidns_set_last_pid_vec(struct ns_common *ns, >>>>>> + struct ns_ioc_pid_vec __user *vec) >>>>>> +{ >>>>>> + return -ENOTTY; >>>>>> +} >>>>>> +#endif /* CONFIG_PID_NS && CONFIG_CHECKPOINT_RESTORE */ >>>>> >>>>> Just CONFIG_PID_NS please. Either this is good enough for everyone who >>>>> has pid namespace support enabled or it isn't. >>>> >>>> Great, if it's so. For me it looks better too. >>>> >>>>>> extern struct pid_namespace *task_active_pid_ns(struct task_struct *tsk); >>>>>> void pidhash_init(void); >>>>>> void pidmap_init(void); >>>>>> diff --git a/include/uapi/linux/nsfs.h b/include/uapi/linux/nsfs.h >>>>>> index 1a3ca79f466b..1254b02a47fa 100644 >>>>>> --- a/include/uapi/linux/nsfs.h >>>>>> +++ b/include/uapi/linux/nsfs.h >>>>>> @@ -14,5 +14,12 @@ >>>>>> #define NS_GET_NSTYPE _IO(NSIO, 0x3) >>>>>> /* Get owner UID (in the caller's user namespace) for a user namespace */ >>>>>> #define NS_GET_OWNER_UID _IO(NSIO, 0x4) >>>>>> +/* Set a vector of ns_last_pid for a pid namespace stack */ >>>>>> +#define NS_SET_LAST_PID_VEC _IO(NSIO, 0x5) >>>>>> + >>>>>> +struct ns_ioc_pid_vec { >>>>>> + __u32 nr; >>>>>> + pid_t pid[0]; >>>>>> +}; >>>>>> >>>>>> #endif /* __LINUX_NSFS_H */ >>>>>> diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c >>>>>> index de461aa0bf9a..08b5fef23534 100644 >>>>>> --- a/kernel/pid_namespace.c >>>>>> +++ b/kernel/pid_namespace.c >>>>>> @@ -21,6 +21,7 @@ >>>>>> #include >>>>>> #include >>>>>> #include >>>>>> +#include >>>>>> >>>>>> struct pid_cache { >>>>>> int nr_ids; >>>>>> @@ -428,6 +429,40 @@ static struct ns_common *pidns_get_parent(struct ns_common *ns) >>>>>> return &get_pid_ns(pid_ns)->ns; >>>>>> } >>>>>> >>>>>> +#ifdef CONFIG_CHECKPOINT_RESTORE >>>>>> +long pidns_set_last_pid_vec(struct ns_common *ns, >>>>>> + struct ns_ioc_pid_vec __user *vec) >>>>>> +{ >>>>>> + struct pid_namespace *pid_ns = to_pid_ns(ns); >>>>>> + pid_t pid, __user *pid_ptr; >>>>>> + u32 nr; >>>>>> + >>>>>> + if (get_user(nr, &vec->nr)) >>>>>> + return -EFAULT; >>>>>> + if (nr > 32 || nr < 1) >>>>> >>>>> The maximum needs not to be hard coded. >>>> >>>> Ah, I've missed MAX_PID_NS_LEVEL, thanks. >>>> >>>>>> + return -EINVAL; >>>>>> + >>>>>> + pid_ptr = &vec->pid[0]; >>>>> >>>>> All of the rest of the vector needs to be read in, in one go. >>>> >>>> Hm, Oleg said we shouldn't allocate a memory for that. Should >>>> I create array of MAX_PID_NS_LEVEL pids on stack? >>> >>> *scratches head* >>> >>> The really important part is that we perform all of the permission >>> checks before we perform the rest of the work. >>> >>> I would like to be able to say that the permission checks and >>> the rest of it all happen atomically. Which requires copying the >>> data into kernel memory and sanitizing it (aka all checks) before >>> we apply the changes. >> >> This way, we better check the permissions on the top pid namespace >> of the passed vector, because every children's pid_ns->user_ns is >> the same as its parent's, or it's descendant. > > In practice this makes sense and is a useful simplification. > > Looking at your suggesting I am noticing we don't actually enforce this > constraint, and that with careful usage of setns I can get around that. > > This seems like a hazard for kernel developers and not at all useful > for userspace developers. So it looks like we need a patch to enforce > this constraint. Patch to fix this issue in a moment. > > >>> "BUILD_BUG_ON(sizeof(u32) * MAX_PID_NS_LEVEL < 64);" if we are >> >> What does this check mean? Why do we have to limit minimal MAX_PID_NS_LEVEL? > > That should have been paranenthesized as: > BUILD_BUG_ON((sizeof(u32) * MAX_PID_NS_LEVEL) < 128); > or possibly writen as: > BUILD_BUG_ON(sizeof(on_stack_array) < 128); > > The point being that if someone changes MAX_PID_NS_LEVEL and the stack > usage goes up noticably we have a warning, and then someone can > determine if the array is still small enough to fit on the stack > or if it needs to be kmalloced. > > The goal is not to leave a trap for maintainers in the future. Thanks for the explanation, Eric, but it's not the question I asked you :) You limit *minimal* MAX_PID_NS_LEVEL, while you write about *maximal* MAX_PID_NS_LEVEL. That's what about I wanted to know. So, the "<" is just a mistake, OK.