From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751485AbdBXJ1V (ORCPT ); Fri, 24 Feb 2017 04:27:21 -0500 Received: from merlin.infradead.org ([205.233.59.134]:54674 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751435AbdBXJ1Q (ORCPT ); Fri, 24 Feb 2017 04:27:16 -0500 Date: Fri, 24 Feb 2017 10:26:46 +0100 From: Peter Zijlstra To: Masami Hiramatsu Cc: Borislav Petkov , linux-kernel@vger.kernel.org, Ingo Molnar , Thomas Gleixner Subject: Re: kprobes vs __ex_table[] Message-ID: <20170224092646.GL6515@twins.programming.kicks-ass.net> References: <20170223183002.GD6557@twins.programming.kicks-ass.net> <20170224100451.31ca3855ddb36963b93d0768@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170224100451.31ca3855ddb36963b93d0768@kernel.org> User-Agent: Mutt/1.5.23.1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Feb 24, 2017 at 10:04:51AM +0900, Masami Hiramatsu wrote: > On Thu, 23 Feb 2017 19:30:02 +0100 > Peter Zijlstra wrote: > > > Hi Masami, > > > > I just wondered what would happen if I put a probe on an instruction > > that was listed in __ex_table[] or __bug_table[]. > > Ah, thanks for reporting, I know __ex_table issue and fixed, but > I didn't care about __bug_table. > > > And it looks like it will happily do that. It will then run the > > instruction out-of-line, and when said instruction traps, the > > instruction address will not match the one listed in either __ex_table[] > > or __bug_table[] and badness will happen. > > For the __ex_table[], at least on x86, kprobes already handles it in > kprobe_fault_handler, which restore regs->ip to original place when > a pagefault happens on singlestepping. Ah, but that is only #PF, we also use __ex_table on other faults/traps, like #GP which would need help in do_general_protection(), and I have a patch (that might not ever go anywhere) that uses it on #UD (but for all I know we already use #UD to probe existence of instructions). In any case, we call fixup_exception() from pretty much all exception vectors, not only #PF. But see below. > > If kprobes does indeed not check this, we should probably fix it, if it > > does do check this, could you point me to it? > > Yeah, for BUG() case, as far as I can see, there is no check about that. So I've a patch that extends __bug_table[] to WARN (like many other architectures already have). http://lkml.kernel.org/r/20170223132813.GB6515@twins.programming.kicks-ass.net > So, there are 2 ways to fix it up, one is to just reject to put kprobes on > UD2, another is fixup trap address as we did for exceptions_table. > I think latter is better because if there is a divide error happening > on single-step, anyway we should fixup the address... Right. So I like the fixup idea, just not sure the current kprobe_fault_handler() is sufficient or correct. It looks like it rewrites regs->ip, which would make return from fault return to the wrong place, no? Would it not be better to do the fixup in fixup_exception()/fixup_bug()? Because then we cover all callers, not just #PF. One more complication with __ex_table and optimized kprobes is that we need to be careful not to clobber __ex_table[].fixup. It would be very bad if the optimized probe were to clobber the address we let the fixup return to -- or that needs fixups too, _after_ running __ex_table[].handler().