From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752420AbcDVMSH (ORCPT ); Fri, 22 Apr 2016 08:18:07 -0400 Received: from mail-db3on0145.outbound.protection.outlook.com ([157.55.234.145]:20309 "EHLO emea01-db3-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751675AbcDVMSE (ORCPT ); Fri, 22 Apr 2016 08:18:04 -0400 Authentication-Results: virtuozzo.com; dkim=none (message not signed) header.d=none;virtuozzo.com; dmarc=none action=none header.from=virtuozzo.com; Subject: Re: [PATCHv7 2/3] x86/vdso: add mremap hook to vm_special_mapping To: Andy Lutomirski References: <1460987025-30360-2-git-send-email-dsafonov@virtuozzo.com> <1460989402-5468-1-git-send-email-dsafonov@virtuozzo.com> CC: "linux-kernel@vger.kernel.org" , "Thomas Gleixner" , Ingo Molnar , "H. Peter Anvin" , X86 ML , Andrew Morton , "linux-mm@kvack.org" , Dmitry Safonov <0x7f454c46@gmail.com> From: Dmitry Safonov Message-ID: <571A00CF.1020600@virtuozzo.com> Date: Fri, 22 Apr 2016 13:45:35 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [195.214.232.10] X-ClientProxiedBy: DB5PR02CA0020.eurprd02.prod.outlook.com (10.161.237.30) To HE1PR0801MB1307.eurprd08.prod.outlook.com (10.167.247.149) X-MS-Office365-Filtering-Correlation-Id: f72c1fcb-3b11-4f1d-b600-08d36a9b65bb X-Microsoft-Exchange-Diagnostics: 1;HE1PR0801MB1307;2:8Qz1/hKbQMHfNjmWJDZwaeLR0vKRzYjymZ2imly1wmUMZNHn5QZb8Vo7vZbJwxr6+XEQKZMhhasc+OBegAaD777pQmlUBPg3E/9CVBj3Hw1iAQFlMdbmvyMdE0uksB7zvk8JR+uIXLIY1Q7mVehxLKFLhG3bLN3+4tp26t7Uh7QfGLdbiJGrCQQwlMViHQeY;3:OAKAQpYQvl4Kw35/5dnpjK1j33gYCURViLlheAZJ+dfKXk3baN8YqjMZWTG0zVsQZ4PEZzzSIDMbswkLZznJsvCdEoNNsmQi5ItdwoqpuPY45WtVWjXU7DTdFQQ2H1Vi;25:JfJy1rq+D1ez6T5m/Fsspo7Wx+ck/2l5cppc3GsuoPk7ZGbNXr8om6MVK7NYg2NEJnJeayJByw7Euk0qAUooXREBvDRWjPMyp+wmZTyYWWOxwYuITuaZtR20o8+Vcery3ZA9KMhvQYX9eJ8j0gozsNHhCOCDxzBeg/s4I5s3lYVx40vqN74gw9EegOOnsxOnIVPq0ZykSke0i6tVxRLLXyie/zh+ga8HSt7r1BUCvFzqmpTYW9M0e9izq6aro29QtXuOdu0THKx2uDLdikli3Jf5ewLrcevcw5MLoKSoGPBGgI9W7MTGV6KTKiu4NthNaXG1EyIB1tam//3LPx4TEg== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:HE1PR0801MB1307; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(9101521026)(6040130)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6041072)(6043046);SRVR:HE1PR0801MB1307;BCL:0;PCL:0;RULEID:;SRVR:HE1PR0801MB1307; X-Microsoft-Exchange-Diagnostics: 1;HE1PR0801MB1307;4:PDrKxRkPi39fwxIaG2Wv5eN7CMdbNkEA6iaB5dBUW1sCGe6GcQZYCgr2g9Vje8JGWUG2hvhR3OQ9++g1M+iO2cshIg1FfvmWIFXkZhNMnHjn4w7pdK1mGnzXVMlzHcQ7RervxnQDTmL48QmgD+3d9deGwH7xYtrvDx8LVmJEXMuaQ1C6NXXAMI+53+kNUhndN6udw8HLH5mLMobHWZEURylnWcsphV5jzj1aIEhToyYPLEPC3Rygi8+b+EdH7S5K4o3xVseID38WcLd0JxXT07LbB5Iic1SQjCaoC4KI9nrZbAcooWbvGekTRNf7QPKdSWQpAnOhjgTPQ/63pxjWcthMTxza1EcY0tAmwC8L2vREmW0geRa7ubnrnq0JjwrvGrLgrF4mRc0aBV2pKcCdbZx0eBn7Hr/m9gdeKPZMmIVVIjabrPh6nDfbYmFaqwiZS618mpgV4zt/4MrRnRsk5Q== X-Forefront-PRVS: 0920602B08 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6049001)(6009001)(24454002)(377454003)(189998001)(110136002)(83506001)(77096005)(4001350100001)(81166005)(230700001)(3846002)(586003)(19580395003)(6116002)(19580405001)(1096002)(5004730100002)(80316001)(23676002)(4326007)(86362001)(50986999)(66066001)(64126003)(76176999)(42186005)(33656002)(50466002)(92566002)(2906002)(65806001)(36756003)(5008740100001)(65956001)(47776003)(87266999)(65816999)(2950100001)(54356999);DIR:OUT;SFP:1102;SCL:1;SRVR:HE1PR0801MB1307;H:[10.30.26.154];FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtIRTFQUjA4MDFNQjEzMDc7MjM6NHg0d3ROZ3pTaUt0YXM0bEdJMVl3NW5n?= =?utf-8?B?NUYxT2JVSDBHYzg2TGcyRUY1alFaT0RZYWhUdkN2WHFXZGhxRXd0YmZNdk5I?= =?utf-8?B?amQybXZEaW1ZcUpvcDFGd1lLVnpEeERONnRjWEFORXJVeEZ6VWs1RUtacFF6?= =?utf-8?B?OEtKa2xoRGhuS3FKNnptL2xvMXkrVVZ1VlpmNXVTcnRHbnNZeDY4Tkt3MHhm?= =?utf-8?B?ejNrN3ZqNmUycjlGK2FMM1lYL2w3TG5kQVhRUnFaRks0TkswbkN5MVBTQUlT?= =?utf-8?B?dU5tTmVJYy9xQTIwa2NiRDFlQVgzZlVaWnEyRjBDYnBneTZXTmlOcEVxOEo1?= =?utf-8?B?VXZ2bXdFbUQ1T3dSbVo3RlhDYi9FaUUxTHFnbllISzBKVHF2a29YK3cwRTlx?= =?utf-8?B?U1FpZTk0dnJSR2NNb1pyN3F5WTJ6NjA4dm9DQTdqR00zS09GamxEVGVLLzJ6?= =?utf-8?B?ZnAyUTFqRHZXZVZNUzFlWktKMzZBT0JSV1kzQnpoY3ROUjVFMHBJN3pKbGhh?= =?utf-8?B?VjBISkxUdDJFenVaYnNLdG54dEwxVDN5aDd0V1ppbzJKL0Q1LzBweWlrRWJY?= =?utf-8?B?VkZRVCt0ZnhhUE9GR0t3VFZ4MTN5KzQyVjUzT09hT01OdVJzeHYvUGpPN1Mw?= =?utf-8?B?QXJ4UElGcnlTak5URmt3MHBtWnBJd0o5MjN1MmJvZXRWa25vM203UG1oM1Jz?= =?utf-8?B?elBtRkU5VFdsS2tFd1oraHRmQWE1SWZPaTlrdXU0cWU0NWJJa1hpNHh2ZzZX?= =?utf-8?B?UFVvSW1mU2xOdEFBbUxoc1ZmYTd3T1NVcm9YK0YyVmhxUmRvbmFsRTQyc3Vz?= =?utf-8?B?OXVzdTB4RENVYTBjVzcvSFRWV0VjOXNiYys5NUUxRm4rcFpaTE50RlV0QWxu?= =?utf-8?B?U2lKa3ZtTStOVElLMjAva0s3VHpucTRGblgzWTJMVUE1dVAzN3hVNHJlL3JQ?= =?utf-8?B?c0ZPRW1IUlJzMEVNRk50Qit0a0p1bVlnWXhMSGlJdHEvdWFUaWY4Y20yUU5q?= =?utf-8?B?Z1l1OXYyUFBHZytadHRDb0tpMDFqYTN6WnJ3bUUrcEJqK09IRHlKNHZ5SVBQ?= =?utf-8?B?R0xHMXJVRUVEc3Jxd2Zabm1iMWxLOUM1MUNpcFR5VkhSbXNjcHZtT3ArVFkr?= =?utf-8?B?aUlzVDBsUE1ISUxRZFFqb1JiZTc1Wk53T3BYRWtmR0ZDdnhSOEhlU2RzZ2dF?= =?utf-8?B?dTZKZmpJc0lySkVYV2NUQjRaNys1MzMyaGVEZTdxMzJ1NXpPT09YYkNVZWtr?= =?utf-8?B?Q2prTitRLytYVjVRKzZvYUJQN3ZGMzVsZXZ1T1NjdTZGS3B3L2JQeU81NEQ1?= =?utf-8?B?MFRJdnBMaEFCejZ5ZzdxT0tSbk1QcmtSY2lsajZHUFdTS3YxOC9MeVNZbnRm?= =?utf-8?B?UWVvS0FkbDR5RWdjZjNrZWROVnVHQXNvdXhwajIxZ3R2b3AxZWxsam1vSUpY?= =?utf-8?Q?A16pdekwxfnUZb5NZxWqu0FVkHxdX?= X-Microsoft-Exchange-Diagnostics: 1;HE1PR0801MB1307;5:elVX6KiHuEMl7+Ors5tGCtg2Nh8hc4MRX51kBfllZfz0tPgyMdeb48lvQktfCUTUhpDR62lxkKpIiEEPbde+bQjD6fsWrmFf/Lfm8mtocIixPVd27BOAMojBLeKsmP57LBxL6Ucy0jWJSKqF/O4f26SZUq7bt21ErCwQBWHFFSwd0NSv18hEWQOALX+Nl6b1;24:84Y4ZiGov8D5+s+ad2V3WR+e2DayVlU/TZ3h0Hvipa4t6/TWdDTyyDVFJ/GfZNzGADe7GoEFCNuN13I51yK3buz7nb5rIz8kbctJzZGDv0Y=;7:rcmr+ugSD0yHXKlLte+XJTSAmrJSLPGl6nqbsbTqvXcQJ2mpkBTJM37kZdAci8+HpMEUp16H1WolDALyHwpZ09J7e/uIj9j/n0F///DfB0cM9yWHZw4JvvTHT5Q+oYCM2uwOzi30KmofpjmMGgq2VwdjDqCIkvHnaLdYh8NawHq8dUBJHsE7HTBgsu4HZeYHwjgf5WMBlsU5F41wDZfaXE448uhlEG1JLiP05AcJWrM=;20:t3IUBsNfs6AGBm3DC8/BXc4GXu5NkvU7TH+eXoheJT1LjI8bgFn11J6PZ3DJCwENjhYxfmva0xmsOA0vmFToWOXoOLrJcJhiw68KrHzGsFeaqSWMl0EByOHXWQeIJJ8V92DUQFu1fTct/5vLQlFkpwahLbki8wPbHQgbyPKqcgM= SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Apr 2016 10:46:43.2240 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0801MB1307 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 04/21/2016 10:52 PM, Andy Lutomirski wrote: > On Mon, Apr 18, 2016 at 7:23 AM, Dmitry Safonov wrote: >> Add possibility for userspace 32-bit applications to move >> vdso mapping. Previously, when userspace app called >> mremap for vdso, in return path it would land on previous >> address of vdso page, resulting in segmentation violation. >> Now it lands fine and returns to userspace with remapped vdso. >> This will also fix context.vdso pointer for 64-bit, which does not >> affect the user of vdso after mremap by now, but this may change. >> >> As suggested by Andy, return EINVAL for mremap that splits vdso image. >> >> Renamed and moved text_mapping structure declaration inside >> map_vdso, as it used only there and now it complement >> vvar_mapping variable. >> >> There is still problem for remapping vdso in glibc applications: >> linker relocates addresses for syscalls on vdso page, so >> you need to relink with the new addresses. Or the next syscall >> through glibc may fail: >> Program received signal SIGSEGV, Segmentation fault. >> #0 0xf7fd9b80 in __kernel_vsyscall () >> #1 0xf7ec8238 in _exit () from /usr/lib32/libc.so.6 >> >> Signed-off-by: Dmitry Safonov >> --- >> v7: that's just not my day: add new_vma parameter to vdso_fix_landing >> sorry for the noise >> v6: moved vdso_image_32 check and fixup code into vdso_fix_landing function >> with ifdefs around >> v5: as Andy suggested, add a check that new_vma->vm_mm and current->mm are >> the same, also check not only in_ia32_syscall() but image == &vdso_image_32 >> v4: drop __maybe_unused & use image from mm->context instead vdso_image_32 >> v3: as Andy suggested, return EINVAL in case of splitting vdso blob on mremap; >> used is_ia32_task instead of ifdefs >> v2: added __maybe_unused for pt_regs in vdso_mremap >> >> arch/x86/entry/vdso/vma.c | 50 ++++++++++++++++++++++++++++++++++++++++++----- >> include/linux/mm_types.h | 3 +++ >> mm/mmap.c | 10 ++++++++++ >> 3 files changed, 58 insertions(+), 5 deletions(-) >> >> diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c >> index 10f704584922..d94291a19b6e 100644 >> --- a/arch/x86/entry/vdso/vma.c >> +++ b/arch/x86/entry/vdso/vma.c >> @@ -12,6 +12,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -98,10 +99,43 @@ static int vdso_fault(const struct vm_special_mapping *sm, >> return 0; >> } >> >> -static const struct vm_special_mapping text_mapping = { >> - .name = "[vdso]", >> - .fault = vdso_fault, >> -}; >> +#if defined CONFIG_X86_32 || defined CONFIG_COMPAT > I think you mean defined(CONFIG_IA32_EMULATION). IIRC CONFIG_COMPAT > is set for X32, !IA32 kernels, too. Sure, I just copied it from vdso_image_32 declaration. Should it be there CONFIG_IA32_EMULATION there instead of CONFIG_COMPAT? Or I miss something? >> +static void vdso_fix_landing(const struct vdso_image *image, >> + struct vm_area_struct *new_vma) >> +{ >> + if (in_ia32_syscall() && image == &vdso_image_32) { >> + struct pt_regs *regs = current_pt_regs(); >> + unsigned long vdso_land = image->sym_int80_landing_pad; >> + unsigned long old_land_addr = vdso_land + >> + (unsigned long)current->mm->context.vdso; >> + >> + /* Fixing userspace landing - look at do_fast_syscall_32 */ >> + if (regs->ip == old_land_addr) >> + regs->ip = new_vma->vm_start + vdso_land; >> + } >> +} >> +#else >> +static void vdso_fix_landing(const struct vdso_image *image, >> + struct vm_area_struct *new_vma) {} >> +#endif > You could also define the function regardless and simply ifdef out the > body, thus saving a few lines of code. Yep, will do. >> + >> +static int vdso_mremap(const struct vm_special_mapping *sm, >> + struct vm_area_struct *new_vma) >> +{ >> + unsigned long new_size = new_vma->vm_end - new_vma->vm_start; >> + const struct vdso_image *image = current->mm->context.vdso_image; >> + >> + if (image->size != new_size) >> + return -EINVAL; >> + >> + if (current->mm != new_vma->vm_mm) >> + return -EFAULT; > Can you make that if (WARN_ON_ONCE(current->mm != new_vma->vm_mm))? > It should be impossible. Sure. >> + >> + vdso_fix_landing(image, new_vma); >> + current->mm->context.vdso = (void __user *)new_vma->vm_start; >> + >> + return 0; >> +} >> >> static int vvar_fault(const struct vm_special_mapping *sm, >> struct vm_area_struct *vma, struct vm_fault *vmf) >> @@ -162,6 +196,12 @@ static int map_vdso(const struct vdso_image *image, bool calculate_addr) >> struct vm_area_struct *vma; >> unsigned long addr, text_start; >> int ret = 0; >> + >> + static const struct vm_special_mapping vdso_mapping = { >> + .name = "[vdso]", >> + .fault = vdso_fault, >> + .mremap = vdso_mremap, >> + }; >> static const struct vm_special_mapping vvar_mapping = { >> .name = "[vvar]", >> .fault = vvar_fault, >> @@ -195,7 +235,7 @@ static int map_vdso(const struct vdso_image *image, bool calculate_addr) >> image->size, >> VM_READ|VM_EXEC| >> VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC, >> - &text_mapping); >> + &vdso_mapping); >> >> if (IS_ERR(vma)) { >> ret = PTR_ERR(vma); >> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h >> index c2d75b4fa86c..4d16ab9287af 100644 >> --- a/include/linux/mm_types.h >> +++ b/include/linux/mm_types.h >> @@ -586,6 +586,9 @@ struct vm_special_mapping { >> int (*fault)(const struct vm_special_mapping *sm, >> struct vm_area_struct *vma, >> struct vm_fault *vmf); >> + >> + int (*mremap)(const struct vm_special_mapping *sm, >> + struct vm_area_struct *new_vma); >> }; >> >> enum tlb_flush_reason { >> diff --git a/mm/mmap.c b/mm/mmap.c >> index bd2e1a533bc1..ba71658dd1a1 100644 >> --- a/mm/mmap.c >> +++ b/mm/mmap.c >> @@ -2930,9 +2930,19 @@ static const char *special_mapping_name(struct vm_area_struct *vma) >> return ((struct vm_special_mapping *)vma->vm_private_data)->name; >> } >> >> +static int special_mapping_mremap(struct vm_area_struct *new_vma) >> +{ >> + struct vm_special_mapping *sm = new_vma->vm_private_data; >> + >> + if (sm->mremap) >> + return sm->mremap(sm, new_vma); >> + return 0; >> +} >> + >> static const struct vm_operations_struct special_mapping_vmops = { >> .close = special_mapping_close, >> .fault = special_mapping_fault, >> + .mremap = special_mapping_mremap, >> .name = special_mapping_name, >> }; >> >> -- >> 2.8.0 >> > > -- Regards, Dmitry Safonov