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=-11.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING, NICE_REPLY_A,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 C4947C433FE for ; Fri, 11 Dec 2020 01:46:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 88A602312A for ; Fri, 11 Dec 2020 01:46:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390644AbgLKBpb (ORCPT ); Thu, 10 Dec 2020 20:45:31 -0500 Received: from mail.kernel.org ([198.145.29.99]:47188 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2390028AbgLKBpK (ORCPT ); Thu, 10 Dec 2020 20:45:10 -0500 Date: Fri, 11 Dec 2020 10:44:24 +0900 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1607651069; bh=Ajksm1kJ6PClgxq9ucUQckZNGNZpfv16/uxGN3OJN0k=; h=From:To:Cc:Subject:In-Reply-To:References:From; b=oJvhjmNZRRifIqD9fjjbxxEs/2a4QSNAQjO+T4ilPokWu3xMnp+yaqlxTWtrkEvDY GblZ6G5Ierm2/I8/Mq6wCORynwUa5GiUr9HoocTwsiqvHC7N7mQvdeZo0+QeX1AbNm /fr2tkoCep2aE0nCM4m6LaUa8norM8ZfS9WfkXfPwVZ5iEHgRfMVL5EiXkxe3GqZoC J0+cDHdl1q7EJY3AfPgUVedLnZ+tq7K3gtGkXqptRchCwl8eXRnt2q2uSHZS0rWxDq zhOyn/qi6OBzpJct1ojY9YEPBu3kWefqDiDuVo4FJ86moWMf3mXUEK1c6cENbURUvg IMcbnBTsr9BoQ== From: Masami Hiramatsu To: Adam Zabrocki Cc: Thomas Gleixner , Ingo Molnar , Borislav Petkov , x86@kernel.org, "H. Peter Anvin" , linux-kernel@vger.kernel.org, "Naveen N. Rao" , Anil S Keshavamurthy , "David S. Miller" , Solar Designer Subject: Re: KRETPROBES are broken since kernel 5.8 Message-Id: <20201211104424.e9e422f8be00e12b5d90260c@kernel.org> In-Reply-To: <20201210171430.GA20584@pi3.com.pl> References: <20201209215001.GA8593@pi3.com.pl> <20201210102507.6bd47a08191852b9f8b5e3f0@kernel.org> <20201210071742.GA14484@pi3.com.pl> <20201210220937.5232571bf5b03237e7018012@kernel.org> <20201210171430.GA20584@pi3.com.pl> X-Mailer: Sylpheed 3.7.0 (GTK+ 2.24.32; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 10 Dec 2020 18:14:30 +0100 Adam Zabrocki wrote: > Hi, > > > > However, there might be another issue which I wanted to brought / discuss - > > > problem with optimizer. Until kernel 5.9 KRETPROBE on e.g. > > > 'ftrace_enable_sysctl' function was correctly optimized without any problems. > > > > Did you check it on other functions? Did you see it only on the "ftrace_enable_sysctl"? > > > > Yes, I see it in most of the functions with padding. Thanks for the confirmation. > > > > Since 5.9 it can't be optimized anynmore. I didn't see any changes in the > > > sources regarding the optimizer, neither function itself. > > > When I looked at the generated vmlinux binary, I can see that GCC generated > > > padding at the end of this function using INT3 opcode: > > > > > > ... > > > ffffffff8130528b: 41 bd f0 ff ff ff mov $0xfffffff0,%r13d > > > ffffffff81305291: e9 fe fe ff ff jmpq ffffffff81305194 > > > ffffffff81305296: cc int3 > > > ffffffff81305297: cc int3 > > > ffffffff81305298: cc int3 > > > ffffffff81305299: cc int3 > > > ffffffff8130529a: cc int3 > > > ffffffff8130529b: cc int3 > > > ffffffff8130529c: cc int3 > > > ffffffff8130529d: cc int3 > > > ffffffff8130529e: cc int3 > > > ffffffff8130529f: cc int3 > > > > So these int3 is generated by GCC for padding, right? > > > > I've just browsed a few commits and I've found that one: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7705dc8557973d8ad8f10840f61d8ec805695e9e > > It looks like INT3 is now a default padding used by linker. Thanks for the information! OK, I will add Fixed: tag and backport it. > > > > However, that's not the case here. INT3_INSN_OPCODE is placed at the end of > > > the function as padding (and protect from NOP-padding problems). > > > > > > I wonder, if optimizer should take this special case into account? Is it worth > > > to still optimize such functions when they are padded with INT3? > > > > Indeed. I expected int3 is used from other subsystems (e.g. kgdb) and, > > in that case the optimization can confuse them. > > Right. The same can happen when text section is being actively modified. > However, this case could be covered by running the optimizer logic under > text_mutex. No, this check is needed because of the instruction decoding. Usually, the int3 will be put a the first byte of the existing instruction whose length is usually 1-6 bytes. If the instruction's opcode is overwritten by the int3, kprobes can not get the original opcode and this means it can not get the original length of the instruction. To optimize the probe, kprobes have to ensure the other jump instruction doesn't jump into the instructions which will be overwritten by optimized jump instruction. This is why the can_optimize() decodes all instructions in the function (note that ksyms has no information of padding bytes, it returns the function size with the padding bytes.) Thus, when the kprobes detects the int3 in the function, it gives up the decoding and optimizing. > > > But if the gcc uses int3 to pad the room between functions, it should be > > reconsidered. > > > > Looks like it's a default behavior now. OK, let me fix that. If the int3 is only used for the padding between functions, those int3 should continue to the end of the function. So kprobes can distinguish the int3 comes from other subsystems or linker. Thank you, > > > Thank you, > > > > > If it is OK, we should backport those to stable tree. > > > > Agreed. > > It is also important to make sure that distro kernels would pick-up such > backported fix. > > Thanks, > Adam > > -- > pi3 (pi3ki31ny) - pi3 (at) itsec pl > http://pi3.com.pl > -- Masami Hiramatsu