From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751335AbdDZQeA (ORCPT ); Wed, 26 Apr 2017 12:34:00 -0400 Received: from mail-he1eur01on0113.outbound.protection.outlook.com ([104.47.0.113]:30528 "EHLO EUR01-HE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750830AbdDZQdv (ORCPT ); Wed, 26 Apr 2017 12:33:51 -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 2/2] pid_ns: Introduce ioctl to set vector of ns_last_pid's on ns hierarhy To: Oleg Nesterov References: <149245014695.17600.12640895883798122726.stgit@localhost.localdomain> <149245057248.17600.1341652606136269734.stgit@localhost.localdomain> <20170426155352.GA12131@redhat.com> <785e1986-da03-72aa-06c0-234ed2dbc0fd@virtuozzo.com> CC: , , , , , , , , , , , , , , From: Kirill Tkhai Message-ID: <005f52d9-efbe-9eaa-7f36-19945c8b06c3@virtuozzo.com> Date: Wed, 26 Apr 2017 19:33:46 +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: <785e1986-da03-72aa-06c0-234ed2dbc0fd@virtuozzo.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [195.214.232.6] X-ClientProxiedBy: AM4PR0501CA0004.eurprd05.prod.outlook.com (2603:10a6:200:6::14) To AM4PR0802MB2276.eurprd08.prod.outlook.com (2603:10a6:200:5f::9) X-MS-Office365-Filtering-Correlation-Id: e51f79b9-3c2b-4e1b-bd46-08d48cc2038b X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(201703131423075)(201703031133081);SRVR:AM4PR0802MB2276; X-Microsoft-Exchange-Diagnostics: 1;AM4PR0802MB2276;3:GFVhdz3FO3qq7yKabxeqtspQC+x3iQaBYXvROEIVsMXPmxRg9eRjenWUJxpJ1GzS4j7MIPznricTKfWCqzuJdsUoEyHdbS+ZZMtYgbGq3Irhc7Q+pPOs+eV/hT2Z3c1k/4XiJSWMvb+Mcqh2OXti+/Nnscq0XkdVyipwU6w+/BkhkwrEWZP/bICL9Xc2H6Ky14d3+QgD2qwx/5SzWk7ahnKgPVx9ym00e0j7y4td9DbA3KTX/tvQ6fNWeEFQOFnVBwIObjIYzeEhk32aTTlzG/I0mTJjVoVLi/WmMX2fEU4F5GWKSg/oX6iLp8RvDyMxWc832G+4FthpoEsisxjlPg==;25:mBljC6eajyT7yVNBfE6j4iKRkpMdchLBzLCI0d2ZyfeyOmAOpOCArRcZNwslXu3jLEL/3A3Xyf+xdh1uWLPuWrofQxg0Y6+oBeLEgopMwbMlwLjEqDis31jnaydxwCPjJpRbKNy2kXsqSydHdO+WODfMdb1d4+cTciMckojsnY9cyRdsslhe0RbSR98W30i8lTgNkGMAm4bKIkXSyw+SlAd6kzxxvn4YOxrY1x6ybfG1DdyJiI7iivjqtebycVmOH5Bu/wFU6VSbpOg1N4sWl/HwXRA6+V5TJkJJfUQLv0a6RzkOxvb+kTXUg8eKW/JwInHNpeOE7ZP0ZJkyW01Q4owJ1e//SuAU+2cB3EODKBlcsSQbkByDxH3JWHIeEG8tvmvhuD/hK//UpWKElCl9Ht6CjPA2VSO6sE2UT15ZSfq4b932rXzR05Vft24+1TepR6uKBQPuAZQF422r5cwKJg== X-Microsoft-Exchange-Diagnostics: 1;AM4PR0802MB2276;31:GJKD0NRLc3TDTSVvyaM58CncLiyHcGCpNxrH3a0dwV5Tv/CjJRBCADEWFqGQxOUlBFfGaTMxlPKbjnE31obza2ThoPS2P6IaOvUaSDB/LYRQjDoTEl4CYr/JTVePch94HZxNVxi1dZuhvzJ3Ngt4k+0dMkZDbyqYMykQpPvILjeTNK1pRBhfT+/HUQhTw7wsIoN90ADewki0ituWz8E4+gNqtqwW5sMTbflJWFNHFCE=;20:azxWoF6tQykhTQaZL9AFHu53MIhhwBLJ4emMA+UCGfBCBnjyBdQK0piUXHBFaHFA+4ky+FixwbK+8oqfevW6+oMrxFJNJHFfqVo+ybg++FDxpidjmtiy6S2k6D/0HW1zw1RfosQ3dArPKdMLCjkjB7gHKUa0px7CKZUl14y1nFlvsCTyAIAIBMQUfc4+M9/Xs5IXm/aetDk6hhCP4deUdA3LZzwbeOKPd5Y+xbCHY8iJHAyQ4m8P1CRu4n1VNhPhaE5CJjnEIvQIxXnNnZdkbWp2uRApnbw+cCFeEihusJHSz/7Ka2U0iIgeeES3nhUvba4D0EIroEG2udubphL2yS9UgxK6g/ZEZtvAOQs2O7RutGYkOgvUW7wT+HIYCan3h8PSKPyQOvjN7mvvg+b9LdZx011L+V7I8TgpJ65+wW4= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(84791874153150); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040450)(601004)(2401047)(8121501046)(5005006)(10201501046)(93006095)(93001095)(3002001)(6041248)(20161123564025)(20161123555025)(20161123562025)(20161123560025)(201703131423075)(201702281528075)(201703061421075)(6072148);SRVR:AM4PR0802MB2276;BCL:0;PCL:0;RULEID:;SRVR:AM4PR0802MB2276; X-Microsoft-Exchange-Diagnostics: 1;AM4PR0802MB2276;4:c7YaHagd+U33x8IZM3Zm/yrWVrbJU2ZY1FDzTkSqruTfIYmxZYKOODJ67HeQIN+y7MiMLCMpZKwh4QhKriFTpRKJnJg8zrSo/FMW1+tUHBgP1R+YfA0iuBrvu2tOKCqp+t21zlDE2HF/rTsH8J76ipanfpQVaYFW0Ht95AjtQGHNDIcw+/DqUv04AzeFQtKbKXQGe8G0FnxWKRDg2w3RzE48CJxZUvFZK7FoxoMU41PGZeRogAAInTSfBVFP1dnaHtk4zQoi9SB2FGT3Mh3XEItbqUm5MlpkLOiFtyibDWZRBnyb1fd7SFdEyfDWUKvb6mMcRqpBuby2NIdwBzfUswOpf/vvo5Gzr8NCAOGUzPIp2qHC9qYl+fDVCNgN+VRl9WlmIpN1RRRM/Dedkud6NlHcvzDAqM2Ba9rsbNQOPaO/wRRZ4fHwShj4Xx3dNynV9dX9bIG83W1XUvCPRNLwdCymGX1kFSZYRPykZ45XJ0lEPwYpN/etSIABic3zYxcvN0eyij2UEZ9iDwtqwQ9G7kHeKSqdygfga4wbm9LL8pkaTfgS06lp0xbTe3vaPHT20SU8ShbPnOGMCQBYVefWhG01ikk/cKiE9d6IjY6LN1dHBFpE62FDX2HwqMSP3JFY/n3oCdbkNXf/9ZsZ+E7rsF4s45AFpHlr7dg9cTkSYhXVBuQggOjPUn5zBo0K1OMfX+y32lamXfCIEksUHeXOOTbGSyxAqNavFOculRgIv3HZiNgAQCCNOjby2LLc4ZpB X-Forefront-PRVS: 0289B6431E X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6009001)(6049001)(39410400002)(39450400003)(39400400002)(39840400002)(24454002)(65826007)(93886004)(230700001)(86362001)(189998001)(2950100002)(6916009)(33646002)(229853002)(6246003)(5660300001)(3846002)(50466002)(64126003)(6116002)(31696002)(83506001)(25786009)(4001350100001)(53546009)(4326008)(65956001)(23746002)(54906002)(50986999)(38730400002)(47776003)(53936002)(6306002)(76176999)(66066001)(2906002)(36756003)(110136004)(65806001)(305945005)(8676002)(7736002)(77096006)(6486002)(31686004)(81166006)(54356999)(42186005);DIR:OUT;SFP:1102;SCL:1;SRVR:AM4PR0802MB2276;H:[172.16.25.137];FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1;AM4PR0802MB2276;23:rh1rEoFd7bd/oTXyHmoLCW8zyx4a2LdSy+I?= =?Windows-1252?Q?yOFrYTE8fW3tl5V6MgaOBuOQZ9VWJSaGLJvHTZVEmeSw0s7wfjOEdwEz?= =?Windows-1252?Q?l+rgLE/bwbMYxC18CeaBvgE3aeP91VQWiNQihpS/6qlIQcaCwrDMoBKF?= =?Windows-1252?Q?yo3H5deNKn8JsbyZPCn2dxNCBd5U0Fkm62rES2nNfLl6Nxn+ugbKS5kf?= =?Windows-1252?Q?9xJKvU68ZE9EKaHpOcMC4mXZeK/Ruwjqozq1cJYNNatltkTCvtWFYGbn?= =?Windows-1252?Q?6fazul8ZuRgE2UUMGzoUkp4czqMdMDehx4Grm33HlrZx84VKL+cgNvVr?= =?Windows-1252?Q?5qXuDWm9pq+9+gumWverU2tfBTfMQ2F+ZTd0p2Oj1J3RWOjmKsUpb7fP?= =?Windows-1252?Q?2rsP8AJEvYDjRfduOOwd4gUrGbXHoQJqnsXtMOovk+zlF/Zc0hGIUxcV?= =?Windows-1252?Q?6I/AiC4HnBlraUJ5soB7uvrkt2fZg/34s8EM6iIOoayphu7yr2VXpGOL?= =?Windows-1252?Q?y4gfAumSKlMr4L3BSLS60AmadE95MysEjYgngPTa15ZVtLEsTnUA4Rlo?= =?Windows-1252?Q?c3iIRTB2X23JO3Gzy18QDWEi236v1K+LZvNkx96AybAkg2G2MhlrJ1wb?= =?Windows-1252?Q?WFqEKXwhPQfvQLX8GOMXpMcKd5k2Lp3ht+Hk9FB5M75QP3ddhEm0rHvL?= =?Windows-1252?Q?2pxTi8nWGCcrkPWnrDWM4SqzFUYDs8v/CvxWOnQtHBY4C0vK380/9FCG?= =?Windows-1252?Q?c0wzP6x4ZWwkE6URad+T583WJeQQ3j23uEcHT/ABg/Ru0vBVpdVv1zDZ?= =?Windows-1252?Q?BOtsHSpJ/Nf/bRmDQeu66bV8e9oSdNwY9p5OleIV/sdWUdW4d15AQyvX?= =?Windows-1252?Q?uZi2pG2Iu6zBZwtgtlNXeboytkxehHY2JTlYu9UYAfsjbL48vpCA7Q/n?= =?Windows-1252?Q?gmFjYfoyZDNVwdbC2Eh0IkKbeE/QD6aw+PkGo1hxXHfx5I5RC9NoB2Zu?= =?Windows-1252?Q?S2ZIWtxomKuVBmM19qNOPZJ3uWFa+L5PDJt7TqZBpe+J7jCVwYCuqCIu?= =?Windows-1252?Q?f+BpXzpeveiGZMm9EOa0dzif9lrQkQ48VX1vU34EbMMS6zXCE5mqz+xK?= =?Windows-1252?Q?cVlKuCAmPuZGNURTP2a4SgQgJsxIoVPPRkpyd26I+sXNkmVbgDSaBypH?= =?Windows-1252?Q?mbQCCpAWvlAR1kJOREtx1tm7yRVJxr/42wCbpkxuUBoWY3jK7cmNuRIz?= =?Windows-1252?Q?f0J7sPfxtcmSSkRKcCgxXmKH02b7UdfW1ulNbdMaMboErNho351hojzD?= =?Windows-1252?Q?QWpmxEzxbcJQ3CX7fg/SMzJB5Xx+cVPlRC02Zexf1KwsXqUB5YHSW07i?= =?Windows-1252?Q?0siNj26FTe5Lv?= X-Microsoft-Exchange-Diagnostics: 1;AM4PR0802MB2276;6:JZ/MnsOFfM0KmMLjZUQfnWN329w8FmaGM02BFMXVAjfbIISy8UBKg4oYjLvy/PSU8d3jDLpftOZX6j+z1XY5GqYCy8Yr2JgX3hodxlGycYjhbRjuozSVkOKpt46ClkL1R5Ja0H9efO4QLWEoqFOoNvNwAjbQkLxG0XuAY2JN2Eqfu13MuqozyOfq0Bl3B8NaWQ7nRmkBeVJccbXjzpF2lX1MUu/nZhfGyx5wvZnI8U4/Dpm5olNtpr/ekVuGJ03BLYW45XK+jrw25S1e8uNzR9Y31MUM6usGIPz4cvpoM/qL8w12cqr7RlSsgYaoYV0S3qH51StYg+DYI8rpwikf6s37uSMAqvXrFe32EXLm21ImNQXi46Mr5zdTeOnyMJ+d/3MT7JHELToCLsUnOfJaf5vxhjsQ7t5ReC6vdAr5/jEjxCjrk1msYat3GcQ+gp+QDlpY5sAuzSk/c1IPl2HAVg062qLV3jSsf9V6DmfJXqErwFmMyWCbp9mzsN5V3+IUMCjHxz0FQD/C05Ldhb1Bhw==;5:koI6GTvPZrA+4/ld6adRZKbpojdBUytDTpfoiNC+YTmFP5dYcbukqv076jc6Pd9aqGQf28S4B4keRamW4YMYo36KNYKd9Ci2TWZ4ShAlpUkvfjLQ/2F333CNQ8s6yBNume1PsawocMwpyLmfehv1CQ==;24:kv33u6K5fO+byC6ck8Esq79DmbbxOAajhN031S5lVEnyzMRtpHu+8n2GsDxJkHRCMaSMWTYGM7kxPB5OU3UAJ/uPbn0JvXFTThDYpTkUGqY= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;AM4PR0802MB2276;7:KVYDcs5cHPTUwA5f6DNWmO0QBYEquWf8nKHR50KO5uS25j2cKMnPaGEBb7KOUfaUlFDrwdrPQEQZxUMVT+32VwQ5+xIKNAvdd+Ah0LozUc8S5P1aB1Hw6kaU5J3k1FZtTKfLtX7OUtZmLZpGAM8a7p3t6CkmnlggMZSOSirvpYSVgBoehNwPLLGHtAznXsbeQxh8FQXik7niSgRhY3SYc2RQBgbZYJbse75W9+aHHxGkZbkKZRk1EMlEw4ohBZlLeHAqAoz8ccWu0PgBZoYWMZpcHRlxqfF/BR35ayd0bt+uTstaZDkayyVjhGh0NrT1lqLxznqx0cyNLTxw0SaslQ==;20:SyENA7v872oddFnjIBPrgpU7GDzsSXHSnMKg5KOcBO5cwHbiYjSDWNA7iY1wI5kmbZk0O7obfo9GGgRK7Y05B1jhUk587D4ljdBPAk6YudolEvUptZ1mr+b3Mck8Mwuwt5OyoqlQCi3gxzWuAgVd/Dk0ZLwQa4ZCRLVCdKVMXiQ= X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Apr 2017 16:33:46.8449 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR0802MB2276 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 26.04.2017 19:11, Kirill Tkhai wrote: > On 26.04.2017 18:53, Oleg Nesterov wrote: >> On 04/17, Kirill Tkhai wrote: >>> >>> +struct pidns_ioc_req { >>> +/* Set vector of last pids in namespace hierarchy */ >>> +#define PIDNS_REQ_SET_LAST_PID_VEC 0x1 >>> + unsigned int req; >>> + void __user *data; >>> + unsigned int data_size; >>> + char std_fields[0]; >>> +}; >> >> see below, >> >>> +static long set_last_pid_vec(struct pid_namespace *pid_ns, >>> + struct pidns_ioc_req *req) >>> +{ >>> + char *str, *p; >>> + int ret = 0; >>> + pid_t pid; >>> + >>> + read_lock(&tasklist_lock); >>> + if (!pid_ns->child_reaper) >>> + ret = -EINVAL; >>> + read_unlock(&tasklist_lock); >>> + if (ret) >>> + return ret; >> >> why do you need to check ->child_reaper under tasklist_lock? this looks pointless. >> >> In fact I do not understand how it is possible to hit pid_ns->child_reaper == NULL, >> there must be at least one task in this namespace, otherwise you can't open a file >> which has f_op == ns_file_operations, no? > > Sure, it's impossible to pick a pid_ns, if there is no the pid_ns's tasks. I added > it under impression of > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=dfda351c729733a401981e8738ce497eaffcaa00 > but here it's completely wrong. It will be removed in v2. > >>> + if (req->data_size >= PAGE_SIZE) >>> + return -EINVAL; >>> + str = vmalloc(req->data_size + 1); >> >> then I don't understand why it makes sense to use vmalloc() >> >>> + if (!str) >>> + return -ENOMEM; >>> + if (copy_from_user(str, req->data, req->data_size)) { >>> + ret = -EFAULT; >>> + goto out_vfree; >>> + } >>> + str[req->data_size] = '\0'; >>> + >>> + p = str; >>> + while (p && *p != '\0') { >>> + if (!ns_capable(pid_ns->user_ns, CAP_SYS_ADMIN)) { >>> + ret = -EPERM; >>> + goto out_vfree; >>> + } >>> + >>> + if (sscanf(p, "%d", &pid) != 1 || pid < 0 || pid > pid_max) { >>> + ret = -EINVAL; >>> + goto out_vfree; >>> + } >> >> Well, this is ioctl(), do we really want to parse the strings? >> >> Can't we make >> >> struct pidns_ioc_req { >> ... >> int nr_pids; >> pid_t pids[0]; >> } >> >> and just use get_user() in a loop? This way we can avoid vmalloc() or anything >> else altogether. > > Since it's a generic structure for different types of the requests, it may be extended > in the future. We won't be able to add new fields, if we compose the structure the way > you suggested, will we? Though, we may go this way if just do the fields generic: struct pidns_ioc_req { unsigned int req; unsigned int data_size; union { pid_t pid[0]; }; }; Ok, I'll rework the patchset in this way.