From patchwork Thu Oct 29 17:34:42 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jean-Philippe Brucker X-Patchwork-Id: 11867085 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=-12.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_GIT 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 5F14DC4363A for ; Thu, 29 Oct 2020 17:43:48 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id AA9A620720 for ; Thu, 29 Oct 2020 17:43:47 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="Tur1ITbz"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="QloLRnyr" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org AA9A620720 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:Message-Id:Date:Subject:To:From: Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender :Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Owner; bh=V9Pv6RxNb0uHnYQ8pWV+CLMnbURHUD3CvaI2ShkmET8=; b=Tur1ITbz9hTasHlrsIUsMqRW4/ Gw1Wmx3lsN7Bw4B/qVxo2KwW0o/2wzMYyGxJpKw+vvwGNXPQi/MLhcAmZ7arJZGU9NQ4erIGMfY26 uf4KYlNKaWXi5/f6+5Lq2yQFbYl3g9YOzT6HuN17TM5Y8FqrD2cDQGbs7EUxQY876Efv5xyzwmBOk SCHI94P9DmLpvhf4p5EHmtVqA8rpsAPmVbWNorQoy1vOYa2yvaCS/b4XhCi+pKLOL9yyAojYPzqsm RdqqNPmT++5blDVKN091xnn0DwowKMVVKYxlv5UTi/W4KEvWZdLCTqszYQFcJwqyvBOZ55H6nEt2g NaNvhFVw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kYBw2-0001wY-Qa; Thu, 29 Oct 2020 17:42:02 +0000 Received: from mail-wr1-x443.google.com ([2a00:1450:4864:20::443]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kYBvz-0001ut-9T for linux-arm-kernel@lists.infradead.org; Thu, 29 Oct 2020 17:42:00 +0000 Received: by mail-wr1-x443.google.com with SMTP id m13so3709409wrj.7 for ; Thu, 29 Oct 2020 10:41:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=Z9c+YbohQuCFW7jfEG3CCAvM7zCRxBebJf+DORtQLww=; b=QloLRnyrtsi0NOY/aODBBxPLflmcmylZCjDmP0UxFj0Cc9mbg7+NNjanweKIgAsbDy 41ZaK6wVGeAf98xMptvpnfhw++gOFUW0aUWz8UQWV05MHH4cwQKnW9+xtG4DthVsPWMp BA0QkRElJC+sg8jkBpCggzbZv0ilWD6RDrpbYMaIiI+j/xzcNp0pX+za+dd0pHtWbZ/H FoUhO2KHyXf9Y49H3FI6HjwwMp6AT7ylABOBzuWyl3hXWYjzlmrGDHsu4dgoFTmLGn83 u7puSBQUh10o3ZIXN42P7fuaNzItDvXE+d6j9G9DghHib+tyjIAoZsx6fWUMPRduI10M TlIA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=Z9c+YbohQuCFW7jfEG3CCAvM7zCRxBebJf+DORtQLww=; b=sG9Nqipp9uq+iE9otgubByC1MivColt2arrQppJ8kcu7ePH64golclvTy1wq7gouQs xYYYn7XGw4VfK8c5Jb72VLUBebgDlm+EUg5NGwQqya8rXflJpR6QlU5IkK1rKFhJxL8s NHk4J4Y7qVrxPk44dOyTyniXaPwXaAdzp6hjxeQ912Xxr9PoJNJyN5/9K7dbTPhhuKrK 58EKr8EKroH6JUNHMn5zslnBGMWXYsu5ltU4f1p61ibKNJ9YEX5bIjBfoN89YwG2IZpW 93rQj/QY/tsg7FLAkpO4b1W3NtqQxKhNG/rZ4vJ12VqbBocVaOYH9iNpUIS+rsLaM3Za xvWQ== X-Gm-Message-State: AOAM533HKmIdRufZYSr5/JFCzvHxJH3XMX+H9n5wtoV03iqqpLTAJipN EbaN1EHK+ljyAUGEpQ3dWq7hUA== X-Google-Smtp-Source: ABdhPJwcQ96l/rxsgXzNFEUjZq77DOqsNHTfIjJgNJJYOWk/J51Pkyjh6Px/1phHhTl8r4DFB3FcAQ== X-Received: by 2002:adf:e4c8:: with SMTP id v8mr6999540wrm.72.1603993316567; Thu, 29 Oct 2020 10:41:56 -0700 (PDT) Received: from localhost.localdomain ([2001:1715:4e26:a7e0:116c:c27a:3e7f:5eaf]) by smtp.gmail.com with ESMTPSA id o3sm6048596wru.15.2020.10.29.10.41.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 29 Oct 2020 10:41:55 -0700 (PDT) From: Jean-Philippe Brucker To: catalin.marinas@arm.com, will@kernel.org Subject: [PATCH] arm64: kprobes: Use BRK instead of single-step when executing instructions out-of-line Date: Thu, 29 Oct 2020 18:34:42 +0100 Message-Id: <20201029173440.117174-1-jean-philippe@linaro.org> X-Mailer: git-send-email 2.29.1 MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201029_134159_627384_5A72128F X-CRM114-Status: GOOD ( 26.10 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jean-Philippe Brucker , mhiramat@kernel.org, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Commit 36dadef23fcc ("kprobes: Init kprobes in early_initcall") enabled using kprobes from early_initcall. Unfortunately at this point the hardware debug infrastructure is not operational. The OS lock may still be locked, and the hardware watchpoints may have unknown values when kprobe enables debug monitors to single-step instructions. Rather than using hardware single-step, append a BRK instruction after the instruction to be executed out-of-line. Fixes: 36dadef23fcc ("kprobes: Init kprobes in early_initcall") Suggested-by: Will Deacon Signed-off-by: Jean-Philippe Brucker Acked-by: Masami Hiramatsu --- An alternative to fix [1]. I haven't done uprobes yet since they don't suffer the same problem, but could add it to the bottom of my list. Lightly tested with kprobe smoke test and the BPF selftests. Interestingly on Seattle when running the "overhead" BPF selftest, that triggers a kprobe a bunch of times, I see a 20-30% improvement with this patch. I'm guessing it's the cost of touching the debug sysregs? [1] https://lore.kernel.org/linux-arm-kernel/20201026172907.1468294-1-jean-philippe@linaro.org/ --- arch/arm64/include/asm/brk-imm.h | 2 + arch/arm64/include/asm/debug-monitors.h | 1 + arch/arm64/include/asm/kprobes.h | 2 +- arch/arm64/kernel/probes/kprobes.c | 56 +++++++++---------------- 4 files changed, 24 insertions(+), 37 deletions(-) diff --git a/arch/arm64/include/asm/brk-imm.h b/arch/arm64/include/asm/brk-imm.h index e3d47b52161d..ec7720dbe2c8 100644 --- a/arch/arm64/include/asm/brk-imm.h +++ b/arch/arm64/include/asm/brk-imm.h @@ -10,6 +10,7 @@ * #imm16 values used for BRK instruction generation * 0x004: for installing kprobes * 0x005: for installing uprobes + * 0x006: for kprobe software single-step * Allowed values for kgdb are 0x400 - 0x7ff * 0x100: for triggering a fault on purpose (reserved) * 0x400: for dynamic BRK instruction @@ -19,6 +20,7 @@ */ #define KPROBES_BRK_IMM 0x004 #define UPROBES_BRK_IMM 0x005 +#define KPROBES_BRK_SS_IMM 0x006 #define FAULT_BRK_IMM 0x100 #define KGDB_DYN_DBG_BRK_IMM 0x400 #define KGDB_COMPILED_DBG_BRK_IMM 0x401 diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h index 0b298f48f5bf..657c921fd784 100644 --- a/arch/arm64/include/asm/debug-monitors.h +++ b/arch/arm64/include/asm/debug-monitors.h @@ -53,6 +53,7 @@ /* kprobes BRK opcodes with ESR encoding */ #define BRK64_OPCODE_KPROBES (AARCH64_BREAK_MON | (KPROBES_BRK_IMM << 5)) +#define BRK64_OPCODE_KPROBES_SS (AARCH64_BREAK_MON | (KPROBES_BRK_SS_IMM << 5)) /* uprobes BRK opcodes with ESR encoding */ #define BRK64_OPCODE_UPROBES (AARCH64_BREAK_MON | (UPROBES_BRK_IMM << 5)) diff --git a/arch/arm64/include/asm/kprobes.h b/arch/arm64/include/asm/kprobes.h index 97e511d645a2..8699ce30f587 100644 --- a/arch/arm64/include/asm/kprobes.h +++ b/arch/arm64/include/asm/kprobes.h @@ -16,7 +16,7 @@ #include #define __ARCH_WANT_KPROBES_INSN_SLOT -#define MAX_INSN_SIZE 1 +#define MAX_INSN_SIZE 2 #define flush_insn_slot(p) do { } while (0) #define kretprobe_blacklist_size 0 diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c index deba738142ed..ec1446ceacc9 100644 --- a/arch/arm64/kernel/probes/kprobes.c +++ b/arch/arm64/kernel/probes/kprobes.c @@ -36,21 +36,25 @@ DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk); static void __kprobes post_kprobe_handler(struct kprobe_ctlblk *, struct pt_regs *); -static int __kprobes patch_text(kprobe_opcode_t *addr, u32 opcode) +static int __kprobes patch_text(kprobe_opcode_t *addr, u32 opc1, u32 opc2) { - void *addrs[1]; - u32 insns[1]; + void *addrs[2]; + u32 insns[2]; addrs[0] = addr; - insns[0] = opcode; + insns[0] = opc1; + if (opc2) { + addrs[1] = addr + 1; + insns[1] = opc2; + } - return aarch64_insn_patch_text(addrs, insns, 1); + return aarch64_insn_patch_text(addrs, insns, opc2 ? 2 : 1); } static void __kprobes arch_prepare_ss_slot(struct kprobe *p) { /* prepare insn slot */ - patch_text(p->ainsn.api.insn, p->opcode); + patch_text(p->ainsn.api.insn, p->opcode, BRK64_OPCODE_KPROBES_SS); flush_icache_range((uintptr_t) (p->ainsn.api.insn), (uintptr_t) (p->ainsn.api.insn) + @@ -128,13 +132,13 @@ void *alloc_insn_page(void) /* arm kprobe: install breakpoint in text */ void __kprobes arch_arm_kprobe(struct kprobe *p) { - patch_text(p->addr, BRK64_OPCODE_KPROBES); + patch_text(p->addr, BRK64_OPCODE_KPROBES, 0); } /* disarm kprobe: remove breakpoint from text */ void __kprobes arch_disarm_kprobe(struct kprobe *p) { - patch_text(p->addr, p->opcode); + patch_text(p->addr, p->opcode, 0); } void __kprobes arch_remove_kprobe(struct kprobe *p) @@ -163,20 +167,14 @@ static void __kprobes set_current_kprobe(struct kprobe *p) } /* - * Interrupts need to be disabled before single-step mode is set, and not - * reenabled until after single-step mode ends. - * Without disabling interrupt on local CPU, there is a chance of - * interrupt occurrence in the period of exception return and start of - * out-of-line single-step, that result in wrongly single stepping - * into the interrupt handler. + * Keep interrupts disabled while executing the instruction out-of-line. Since + * the kprobe state is per-CPU, we can't afford to be migrated. */ static void __kprobes kprobes_save_local_irqflag(struct kprobe_ctlblk *kcb, struct pt_regs *regs) { kcb->saved_irqflag = regs->pstate & DAIF_MASK; regs->pstate |= PSR_I_BIT; - /* Unmask PSTATE.D for enabling software step exceptions. */ - regs->pstate &= ~PSR_D_BIT; } static void __kprobes kprobes_restore_local_irqflag(struct kprobe_ctlblk *kcb, @@ -219,10 +217,7 @@ static void __kprobes setup_singlestep(struct kprobe *p, slot = (unsigned long)p->ainsn.api.insn; set_ss_context(kcb, slot); /* mark pending ss */ - - /* IRQs and single stepping do not mix well. */ kprobes_save_local_irqflag(kcb, regs); - kernel_enable_single_step(regs); instruction_pointer_set(regs, slot); } else { /* insn simulation */ @@ -273,12 +268,8 @@ post_kprobe_handler(struct kprobe_ctlblk *kcb, struct pt_regs *regs) } /* call post handler */ kcb->kprobe_status = KPROBE_HIT_SSDONE; - if (cur->post_handler) { - /* post_handler can hit breakpoint and single step - * again, so we enable D-flag for recursive exception. - */ + if (cur->post_handler) cur->post_handler(cur, regs, 0); - } reset_current_kprobe(); } @@ -302,8 +293,6 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr) if (!instruction_pointer(regs)) BUG(); - kernel_disable_single_step(); - if (kcb->kprobe_status == KPROBE_REENTER) restore_previous_kprobe(kcb); else @@ -365,10 +354,6 @@ static void __kprobes kprobe_handler(struct pt_regs *regs) * pre-handler and it returned non-zero, it will * modify the execution path and no need to single * stepping. Let's just reset current kprobe and exit. - * - * pre_handler can hit a breakpoint and can step thru - * before return, keep PSTATE D-flag enabled until - * pre_handler return back. */ if (!p->pre_handler || !p->pre_handler(p, regs)) { setup_singlestep(p, regs, kcb, 0); @@ -399,7 +384,7 @@ kprobe_ss_hit(struct kprobe_ctlblk *kcb, unsigned long addr) } static int __kprobes -kprobe_single_step_handler(struct pt_regs *regs, unsigned int esr) +kprobe_breakpoint_ss_handler(struct pt_regs *regs, unsigned int esr) { struct kprobe_ctlblk *kcb = get_kprobe_ctlblk(); int retval; @@ -409,16 +394,15 @@ kprobe_single_step_handler(struct pt_regs *regs, unsigned int esr) if (retval == DBG_HOOK_HANDLED) { kprobes_restore_local_irqflag(kcb, regs); - kernel_disable_single_step(); - post_kprobe_handler(kcb, regs); } return retval; } -static struct step_hook kprobes_step_hook = { - .fn = kprobe_single_step_handler, +static struct break_hook kprobes_break_ss_hook = { + .imm = KPROBES_BRK_SS_IMM, + .fn = kprobe_breakpoint_ss_handler, }; static int __kprobes @@ -486,7 +470,7 @@ int __kprobes arch_trampoline_kprobe(struct kprobe *p) int __init arch_init_kprobes(void) { register_kernel_break_hook(&kprobes_break_hook); - register_kernel_step_hook(&kprobes_step_hook); + register_kernel_break_hook(&kprobes_break_ss_hook); return 0; }