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=-7.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,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 397E7C46499 for ; Fri, 5 Jul 2019 10:30:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0540D21850 for ; Fri, 5 Jul 2019 10:30:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1562322644; bh=vdie7+J+Qf4GNlmeSDn4F1crcE1sfbOq0CuCbskf0vU=; h=Date:From:To:Cc:Subject:In-Reply-To:References:List-ID:From; b=eZennA2Imp+csc5RGSpQF/OG1gBMXpLiGq7E63j0Y7YwO9BN5GWKanCZ/f3qyROFL DiR5WjvnFKvWu6rjEYPe6Gmig7Z+pznBpH9Ngw7XKzCQgMCL7HVf1/7WHHyFn8yXSY ob06DQKSOAw1R1jMsktLz2WF9921fPvC2tzndHbE= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728342AbfGEKan (ORCPT ); Fri, 5 Jul 2019 06:30:43 -0400 Received: from mail.kernel.org ([198.145.29.99]:33198 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728335AbfGEKan (ORCPT ); Fri, 5 Jul 2019 06:30:43 -0400 Received: from devnote2 (NE2965lan1.rev.em-net.ne.jp [210.141.244.193]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id A56D620989; Fri, 5 Jul 2019 10:30:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1562322641; bh=vdie7+J+Qf4GNlmeSDn4F1crcE1sfbOq0CuCbskf0vU=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=Kq4ViJhUXTwy17K6QtkI7Vgvv3oSdL+I0TLtGx+v8EUfeovLTzNfzVXLQgNLninxb qLlMDKh4k7qQItlyyCTgMVv4xap+V6ywI1evbHEnkc1K9O5rl74j9XCgX8/BPQ/gN3 o754U8Rqt58OFPXq3KHQSJj8NWK6R4Yt4ZpRe8AE= Date: Fri, 5 Jul 2019 19:30:28 +0900 From: Masami Hiramatsu To: Anshuman Khandual Cc: linux-mm@kvack.org, Vineet Gupta , Russell King , Catalin Marinas , Will Deacon , Tony Luck , Fenghua Yu , Ralf Baechle , Paul Burton , James Hogan , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Heiko Carstens , Vasily Gorbik , Christian Borntraeger , Yoshinori Sato , Rich Felker , "David S. Miller" , Thomas Gleixner , Ingo Molnar , Borislav Petkov , "H. Peter Anvin" , "Naveen N. Rao" , Anil S Keshavamurthy , Masami Hiramatsu , Allison Randal , Greg Kroah-Hartman , Enrico Weigelt , Richard Fontana , Kate Stewart , Mark Rutland , Andrew Morton , Guenter Roeck , x86@kernel.org, linux-snps-arc@lists.infradead.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-ia64@vger.kernel.org, linux-mips@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-s390@vger.kernel.org, linux-sh@vger.kernel.org, sparclinux@vger.kernel.org Subject: Re: [PATCH] mm/kprobes: Add generic kprobe_fault_handler() fallback definition Message-Id: <20190705193028.f9e08fe9cf1ee86bc5c0bb82@kernel.org> In-Reply-To: <1562304629-29376-1-git-send-email-anshuman.khandual@arm.com> References: <1562304629-29376-1-git-send-email-anshuman.khandual@arm.com> X-Mailer: Sylpheed 3.5.1 (GTK+ 2.24.32; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-mips-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-mips@vger.kernel.org Hi Anshuman, On Fri, 5 Jul 2019 11:00:29 +0530 Anshuman Khandual wrote: > Architectures like parisc enable CONFIG_KROBES without having a definition > for kprobe_fault_handler() which results in a build failure. Hmm, as far as I can see, kprobe_fault_handler() is closed inside each arch specific code. The reason why include/linux/kprobes.h defines dummy inline function is only for !CONFIG_KPROBES case. > Arch needs to > provide kprobe_fault_handler() as it is platform specific and cannot have > a generic working alternative. But in the event when platform lacks such a > definition there needs to be a fallback. Wait, indeed that each arch need to implement it, but that is for calling kprobe->fault_handler() as user expected. Hmm, why not fixing those architecture implementations? > This adds a stub kprobe_fault_handler() definition which not only prevents > a build failure but also makes sure that kprobe_page_fault() if called will > always return negative in absence of a sane platform specific alternative. I don't like introducing this complicated macro only for avoiding (not fixing) build error. To fix that, kprobes on parisc should implement kprobe_fault_handler correctly (and call kprobe->fault_handler). BTW, even if you need such generic stub, please use a weak function instead of macros for every arch headers. > While here wrap kprobe_page_fault() in CONFIG_KPROBES. This enables stud > definitions for generic kporbe_fault_handler() and kprobes_built_in() can > just be dropped. Only on x86 it needs to be added back locally as it gets > used in a !CONFIG_KPROBES function do_general_protection(). If you want to remove kprobes_built_in(), you should replace it with IS_ENABLED(CONFIG_KPROBES), instead of this... Thank you, > > Cc: Vineet Gupta > Cc: Russell King > Cc: Catalin Marinas > Cc: Will Deacon > Cc: Tony Luck > Cc: Fenghua Yu > Cc: Ralf Baechle > Cc: Paul Burton > Cc: James Hogan > Cc: Benjamin Herrenschmidt > Cc: Paul Mackerras > Cc: Michael Ellerman > Cc: Heiko Carstens > Cc: Vasily Gorbik > Cc: Christian Borntraeger > Cc: Yoshinori Sato > Cc: Rich Felker > Cc: "David S. Miller" > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: Borislav Petkov > Cc: "H. Peter Anvin" > Cc: "Naveen N. Rao" > Cc: Anil S Keshavamurthy > Cc: Masami Hiramatsu > Cc: Allison Randal > Cc: Greg Kroah-Hartman > Cc: Enrico Weigelt > Cc: Richard Fontana > Cc: Kate Stewart > Cc: Mark Rutland > Cc: Andrew Morton > Cc: Guenter Roeck > Cc: x86@kernel.org > Cc: linux-snps-arc@lists.infradead.org > Cc: linux-kernel@vger.kernel.org > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-ia64@vger.kernel.org > Cc: linux-mips@vger.kernel.org > Cc: linuxppc-dev@lists.ozlabs.org > Cc: linux-s390@vger.kernel.org > Cc: linux-sh@vger.kernel.org > Cc: sparclinux@vger.kernel.org > > Signed-off-by: Anshuman Khandual > --- > arch/arc/include/asm/kprobes.h | 1 + > arch/arm/include/asm/kprobes.h | 1 + > arch/arm64/include/asm/kprobes.h | 1 + > arch/ia64/include/asm/kprobes.h | 1 + > arch/mips/include/asm/kprobes.h | 1 + > arch/powerpc/include/asm/kprobes.h | 1 + > arch/s390/include/asm/kprobes.h | 1 + > arch/sh/include/asm/kprobes.h | 1 + > arch/sparc/include/asm/kprobes.h | 1 + > arch/x86/include/asm/kprobes.h | 6 ++++++ > include/linux/kprobes.h | 32 ++++++++++++++++++------------ > 11 files changed, 34 insertions(+), 13 deletions(-) > > diff --git a/arch/arc/include/asm/kprobes.h b/arch/arc/include/asm/kprobes.h > index 2134721dce44..ee8efe256675 100644 > --- a/arch/arc/include/asm/kprobes.h > +++ b/arch/arc/include/asm/kprobes.h > @@ -45,6 +45,7 @@ struct kprobe_ctlblk { > struct prev_kprobe prev_kprobe; > }; > > +#define kprobe_fault_handler kprobe_fault_handler > int kprobe_fault_handler(struct pt_regs *regs, unsigned long cause); > void kretprobe_trampoline(void); > void trap_is_kprobe(unsigned long address, struct pt_regs *regs); > diff --git a/arch/arm/include/asm/kprobes.h b/arch/arm/include/asm/kprobes.h > index 213607a1f45c..660f877b989f 100644 > --- a/arch/arm/include/asm/kprobes.h > +++ b/arch/arm/include/asm/kprobes.h > @@ -38,6 +38,7 @@ struct kprobe_ctlblk { > struct prev_kprobe prev_kprobe; > }; > > +#define kprobe_fault_handler kprobe_fault_handler > void arch_remove_kprobe(struct kprobe *); > int kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr); > int kprobe_exceptions_notify(struct notifier_block *self, > diff --git a/arch/arm64/include/asm/kprobes.h b/arch/arm64/include/asm/kprobes.h > index 97e511d645a2..667773f75616 100644 > --- a/arch/arm64/include/asm/kprobes.h > +++ b/arch/arm64/include/asm/kprobes.h > @@ -42,6 +42,7 @@ struct kprobe_ctlblk { > struct kprobe_step_ctx ss_ctx; > }; > > +#define kprobe_fault_handler kprobe_fault_handler > void arch_remove_kprobe(struct kprobe *); > int kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr); > int kprobe_exceptions_notify(struct notifier_block *self, > diff --git a/arch/ia64/include/asm/kprobes.h b/arch/ia64/include/asm/kprobes.h > index c5cf5e4fb338..c321e8585089 100644 > --- a/arch/ia64/include/asm/kprobes.h > +++ b/arch/ia64/include/asm/kprobes.h > @@ -106,6 +106,7 @@ struct arch_specific_insn { > unsigned short slot; > }; > > +#define kprobe_fault_handler kprobe_fault_handler > extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr); > extern int kprobe_exceptions_notify(struct notifier_block *self, > unsigned long val, void *data); > diff --git a/arch/mips/include/asm/kprobes.h b/arch/mips/include/asm/kprobes.h > index 68b1e5d458cf..d1efe991ea22 100644 > --- a/arch/mips/include/asm/kprobes.h > +++ b/arch/mips/include/asm/kprobes.h > @@ -40,6 +40,7 @@ do { \ > > #define kretprobe_blacklist_size 0 > > +#define kprobe_fault_handler kprobe_fault_handler > void arch_remove_kprobe(struct kprobe *p); > int kprobe_fault_handler(struct pt_regs *regs, int trapnr); > > diff --git a/arch/powerpc/include/asm/kprobes.h b/arch/powerpc/include/asm/kprobes.h > index 66b3f2983b22..c94f375ec957 100644 > --- a/arch/powerpc/include/asm/kprobes.h > +++ b/arch/powerpc/include/asm/kprobes.h > @@ -84,6 +84,7 @@ struct arch_optimized_insn { > kprobe_opcode_t *insn; > }; > > +#define kprobe_fault_handler kprobe_fault_handler > extern int kprobe_exceptions_notify(struct notifier_block *self, > unsigned long val, void *data); > extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr); > diff --git a/arch/s390/include/asm/kprobes.h b/arch/s390/include/asm/kprobes.h > index b106aa29bf55..0ecaebb78092 100644 > --- a/arch/s390/include/asm/kprobes.h > +++ b/arch/s390/include/asm/kprobes.h > @@ -73,6 +73,7 @@ struct kprobe_ctlblk { > void arch_remove_kprobe(struct kprobe *p); > void kretprobe_trampoline(void); > > +#define kprobe_fault_handler kprobe_fault_handler > int kprobe_fault_handler(struct pt_regs *regs, int trapnr); > int kprobe_exceptions_notify(struct notifier_block *self, > unsigned long val, void *data); > diff --git a/arch/sh/include/asm/kprobes.h b/arch/sh/include/asm/kprobes.h > index 6171682f7798..637a698393c0 100644 > --- a/arch/sh/include/asm/kprobes.h > +++ b/arch/sh/include/asm/kprobes.h > @@ -45,6 +45,7 @@ struct kprobe_ctlblk { > struct prev_kprobe prev_kprobe; > }; > > +#define kprobe_fault_handler kprobe_fault_handler > extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr); > extern int kprobe_exceptions_notify(struct notifier_block *self, > unsigned long val, void *data); > diff --git a/arch/sparc/include/asm/kprobes.h b/arch/sparc/include/asm/kprobes.h > index bfcaa6326c20..9aa4d25a45a8 100644 > --- a/arch/sparc/include/asm/kprobes.h > +++ b/arch/sparc/include/asm/kprobes.h > @@ -47,6 +47,7 @@ struct kprobe_ctlblk { > struct prev_kprobe prev_kprobe; > }; > > +#define kprobe_fault_handler kprobe_fault_handler > int kprobe_exceptions_notify(struct notifier_block *self, > unsigned long val, void *data); > int kprobe_fault_handler(struct pt_regs *regs, int trapnr); > diff --git a/arch/x86/include/asm/kprobes.h b/arch/x86/include/asm/kprobes.h > index 5dc909d9ad81..1af2b6db13bd 100644 > --- a/arch/x86/include/asm/kprobes.h > +++ b/arch/x86/include/asm/kprobes.h > @@ -101,11 +101,17 @@ struct kprobe_ctlblk { > struct prev_kprobe prev_kprobe; > }; > > +#define kprobe_fault_handler kprobe_fault_handler > extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr); > extern int kprobe_exceptions_notify(struct notifier_block *self, > unsigned long val, void *data); > extern int kprobe_int3_handler(struct pt_regs *regs); > extern int kprobe_debug_handler(struct pt_regs *regs); > +#else > +static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr) > +{ > + return 0; > +} > > #endif /* CONFIG_KPROBES */ > #endif /* _ASM_X86_KPROBES_H */ > diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h > index 04bdaf01112c..e106f3018804 100644 > --- a/include/linux/kprobes.h > +++ b/include/linux/kprobes.h > @@ -182,11 +182,19 @@ DECLARE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk); > /* > * For #ifdef avoidance: > */ > -static inline int kprobes_built_in(void) > + > +/* > + * Architectures need to override this with their own implementation > + * if they care to call kprobe_page_fault(). This will just ensure > + * that kprobe_page_fault() returns false when called without having > + * a proper platform specific definition for kprobe_fault_handler(). > + */ > +#ifndef kprobe_fault_handler > +static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr) > { > - return 1; > + return 0; > } > - > +#endif > #ifdef CONFIG_KRETPROBES > extern void arch_prepare_kretprobe(struct kretprobe_instance *ri, > struct pt_regs *regs); > @@ -375,14 +383,6 @@ void free_insn_page(void *page); > > #else /* !CONFIG_KPROBES: */ > > -static inline int kprobes_built_in(void) > -{ > - return 0; > -} > -static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr) > -{ > - return 0; > -} > static inline struct kprobe *get_kprobe(void *addr) > { > return NULL; > @@ -458,12 +458,11 @@ static inline bool is_kprobe_optinsn_slot(unsigned long addr) > } > #endif > > +#ifdef CONFIG_KPROBES > /* Returns true if kprobes handled the fault */ > static nokprobe_inline bool kprobe_page_fault(struct pt_regs *regs, > unsigned int trap) > { > - if (!kprobes_built_in()) > - return false; > if (user_mode(regs)) > return false; > /* > @@ -476,5 +475,12 @@ static nokprobe_inline bool kprobe_page_fault(struct pt_regs *regs, > return false; > return kprobe_fault_handler(regs, trap); > } > +#else > +static nokprobe_inline bool kprobe_page_fault(struct pt_regs *regs, > + unsigned int trap) > +{ > + return false; > +} > +#endif > > #endif /* _LINUX_KPROBES_H */ > -- > 2.20.1 > -- Masami Hiramatsu