From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi0-f72.google.com (mail-oi0-f72.google.com [209.85.218.72]) by kanga.kvack.org (Postfix) with ESMTP id F05F56B000C for ; Mon, 26 Mar 2018 05:36:33 -0400 (EDT) Received: by mail-oi0-f72.google.com with SMTP id r132-v6so3021902oig.16 for ; Mon, 26 Mar 2018 02:36:33 -0700 (PDT) Received: from foss.arm.com (usa-sjc-mx-foss1.foss.arm.com. [217.140.101.70]) by mx.google.com with ESMTP id i46-v6si4822246otb.122.2018.03.26.02.36.32 for ; Mon, 26 Mar 2018 02:36:32 -0700 (PDT) Date: Mon, 26 Mar 2018 10:36:18 +0100 From: Mark Rutland Subject: Re: [RFC PATCH 11/14] khwasan: add brk handler for inline instrumentation Message-ID: <20180326093241.lba7k4pdjskr4gsv@lakrids.cambridge.arm.com> References: <20180305145111.bbycnzpgzkir2dz4@lakrids.cambridge.arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: owner-linux-mm@kvack.org List-ID: To: Andrey Konovalov Cc: Andrey Ryabinin , Alexander Potapenko , Dmitry Vyukov , Jonathan Corbet , Catalin Marinas , Will Deacon , Theodore Ts'o , Jan Kara , Christopher Li , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Masahiro Yamada , Michal Marek , Ard Biesheuvel , Yury Norov , Nick Desaulniers , Marc Zyngier , Suzuki K Poulose , Kristina Martsenko , Punit Agrawal , Dave Martin , James Morse , Julien Thierry , Michael Weiser , Steve Capper , Ingo Molnar , Thomas Gleixner , Sandipan Das , Paul Lawrence , David Woodhouse , Kees Cook , Geert Uytterhoeven , Josh Poimboeuf , Arnd Bergmann , kasan-dev , linux-doc@vger.kernel.org, LKML , Linux ARM , linux-ext4@vger.kernel.org, linux-sparse@vger.kernel.org, Linux Memory Management List , Linux Kbuild mailing list , Kostya Serebryany , Evgeniy Stepanov , Lee Smith , Ramana Radhakrishnan , Jacob Bramley , Ruben Ayrapetyan , Kees Cook , Jann Horn , Mark Brand On Fri, Mar 23, 2018 at 04:59:36PM +0100, Andrey Konovalov wrote: > On Mon, Mar 5, 2018 at 3:51 PM, Mark Rutland wrote: > > On Fri, Mar 02, 2018 at 08:44:30PM +0100, Andrey Konovalov wrote: > >> +static int khwasan_handler(struct pt_regs *regs, unsigned int esr) > >> +{ > >> + /* If thread survives, skip over the BUG instruction and continue: */ > >> + arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE); > > > > This is for fast-forwarding user instruction streams, and isn't correct > > to call for kernel faults (as it'll mess up the userspace single step > > logic). > > I saw BUG handler using this (which also inserts a brk), so I used it > as well. Ah; I think that's broken today. > What should I do instead to jump over the faulting brk instruction? I don't think we have anything to do this properly today. The simplest fix would be to split arm64_skip_faulting_instruction() into separate functions for user/kernel, something like the below. It would be nice to drop _user_ in the name of the userspace-specific helper, though. Thanks Mark. ---->8---- diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c index eb2d15147e8d..101e3d4ed6c8 100644 --- a/arch/arm64/kernel/traps.c +++ b/arch/arm64/kernel/traps.c @@ -235,9 +235,14 @@ void arm64_notify_die(const char *str, struct pt_regs *regs, } } -void arm64_skip_faulting_instruction(struct pt_regs *regs, unsigned long size) +void __arm64_skip_faulting_instruction(struct pt_regs *regs, unsigned long size) { regs->pc += size; +} + +void arm64_skip_faulting_instruction(struct pt_regs *regs, unsigned long size) +{ + __arm64_skip_faulting_instruction(regs, size); /* * If we were single stepping, we want to get the step exception after @@ -761,7 +766,7 @@ static int bug_handler(struct pt_regs *regs, unsigned int esr) } /* If thread survives, skip over the BUG instruction and continue: */ - arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE); + __arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE); return DBG_HOOK_HANDLED; }