From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 59202C04AB6 for ; Fri, 31 May 2019 08:49:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3AC0F24895 for ; Fri, 31 May 2019 08:49:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726922AbfEaItG (ORCPT ); Fri, 31 May 2019 04:49:06 -0400 Received: from mx2.suse.de ([195.135.220.15]:55838 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726002AbfEaItF (ORCPT ); Fri, 31 May 2019 04:49:05 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id BDDB7AF55; Fri, 31 May 2019 08:49:03 +0000 (UTC) Date: Fri, 31 May 2019 10:49:02 +0200 (CEST) From: Miroslav Benes To: Josh Poimboeuf cc: Steven Rostedt , Jiri Kosina , Petr Mladek , Jessica Yu , Joe Lawrence , linux-kernel@vger.kernel.org, live-patching@vger.kernel.org, Johannes Erdfelt , Ingo Molnar Subject: Re: [PATCH] livepatch: Fix ftrace module text permissions race In-Reply-To: Message-ID: References: User-Agent: Alpine 2.21 (LSU 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 29 May 2019, Josh Poimboeuf wrote: > It's possible for livepatch and ftrace to be toggling a module's text > permissions at the same time, resulting in the following panic: > > BUG: unable to handle page fault for address: ffffffffc005b1d9 > #PF: supervisor write access in kernel mode > #PF: error_code(0x0003) - permissions violation > PGD 3ea0c067 P4D 3ea0c067 PUD 3ea0e067 PMD 3cc13067 PTE 3b8a1061 > Oops: 0003 [#1] PREEMPT SMP PTI > CPU: 1 PID: 453 Comm: insmod Tainted: G O K 5.2.0-rc1-a188339ca5 #1 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-20181126_142135-anatol 04/01/2014 > RIP: 0010:apply_relocate_add+0xbe/0x14c > Code: fa 0b 74 21 48 83 fa 18 74 38 48 83 fa 0a 75 40 eb 08 48 83 38 00 74 33 eb 53 83 38 00 75 4e 89 08 89 c8 eb 0a 83 38 00 75 43 <89> 08 48 63 c1 48 39 c8 74 2e eb 48 83 38 00 75 32 48 29 c1 89 08 > RSP: 0018:ffffb223c00dbb10 EFLAGS: 00010246 > RAX: ffffffffc005b1d9 RBX: 0000000000000000 RCX: ffffffff8b200060 > RDX: 000000000000000b RSI: 0000004b0000000b RDI: ffff96bdfcd33000 > RBP: ffffb223c00dbb38 R08: ffffffffc005d040 R09: ffffffffc005c1f0 > R10: ffff96bdfcd33c40 R11: ffff96bdfcd33b80 R12: 0000000000000018 > R13: ffffffffc005c1f0 R14: ffffffffc005e708 R15: ffffffff8b2fbc74 > FS: 00007f5f447beba8(0000) GS:ffff96bdff900000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: ffffffffc005b1d9 CR3: 000000003cedc002 CR4: 0000000000360ea0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Call Trace: > klp_init_object_loaded+0x10f/0x219 > ? preempt_latency_start+0x21/0x57 > klp_enable_patch+0x662/0x809 > ? virt_to_head_page+0x3a/0x3c > ? kfree+0x8c/0x126 > patch_init+0x2ed/0x1000 [livepatch_test02] > ? 0xffffffffc0060000 > do_one_initcall+0x9f/0x1c5 > ? kmem_cache_alloc_trace+0xc4/0xd4 > ? do_init_module+0x27/0x210 > do_init_module+0x5f/0x210 > load_module+0x1c41/0x2290 > ? fsnotify_path+0x3b/0x42 > ? strstarts+0x2b/0x2b > ? kernel_read+0x58/0x65 > __do_sys_finit_module+0x9f/0xc3 > ? __do_sys_finit_module+0x9f/0xc3 > __x64_sys_finit_module+0x1a/0x1c > do_syscall_64+0x52/0x61 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > The above panic occurs when loading two modules at the same time with > ftrace enabled, where at least one of the modules is a livepatch module: > > CPU0 CPU1 > klp_enable_patch() > klp_init_object_loaded() > module_disable_ro() > ftrace_module_enable() > ftrace_arch_code_modify_post_process() > set_all_modules_text_ro() > klp_write_object_relocations() > apply_relocate_add() > *patches read-only code* - BOOM > > A similar race exists when toggling ftrace while loading a livepatch > module. > > Fix it by ensuring that the livepatch and ftrace code patching > operations -- and their respective permissions changes -- are protected > by the text_mutex. > > Reported-by: Johannes Erdfelt > Signed-off-by: Josh Poimboeuf For the code Reviewed-by: Miroslav Benes However, shouldn't the patch be split in two? One adding text_mutex protection to livepatch and ftrace. The other adding lockdep_assert_held() and __module_enable_ro()? The current changelog does not mention lockdep changes at all. Miroslav