From patchwork Tue Sep 6 17:54:59 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Long X-Patchwork-Id: 75569 Delivered-To: patches@linaro.org Received: by 10.140.106.11 with SMTP id d11csp671494qgf; Tue, 6 Sep 2016 10:55:02 -0700 (PDT) X-Received: by 10.55.41.232 with SMTP id p101mr32920504qkp.93.1473184502213; Tue, 06 Sep 2016 10:55:02 -0700 (PDT) Return-Path: Received: from mail-qk0-x231.google.com (mail-qk0-x231.google.com. [2607:f8b0:400d:c09::231]) by mx.google.com with ESMTPS id r126si22073892qkb.211.2016.09.06.10.55.02 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 06 Sep 2016 10:55:02 -0700 (PDT) Received-SPF: pass (google.com: domain of dave.long@linaro.org designates 2607:f8b0:400d:c09::231 as permitted sender) client-ip=2607:f8b0:400d:c09::231; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org; spf=pass (google.com: domain of dave.long@linaro.org designates 2607:f8b0:400d:c09::231 as permitted sender) smtp.mailfrom=dave.long@linaro.org; dmarc=pass (p=NONE dis=NONE) header.from=linaro.org Received: by mail-qk0-x231.google.com with SMTP id l2so226288113qkf.3 for ; Tue, 06 Sep 2016 10:55:02 -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; bh=9bQHU4tM4Dp4Bc1h+lm3XkXefdsXGxGluv+dlKsTxCE=; b=hPyn0HnQob6E758ce1y/yPO/ZmXEBDHfYiJEDSuDxUIy/FU2FR248wkuU743E6gS9a ZdMOUkV/CXTS3YnNfR3nzFfPRTNR3CAQACc63+0mkskfCPY+7AYtjCwXryBh+LoTcsmh c8kmDXPL2oxXabucar5XioS7ooGhYZjrrh/SA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=9bQHU4tM4Dp4Bc1h+lm3XkXefdsXGxGluv+dlKsTxCE=; b=RdizJ5n83pNPxeAGlVHXqMGH4Suz4GFWh9JpfkoYSWtbwZLI3m3s8Ow7lgBhZnxtIe uZhutQN+JUtcdBWn7RS+B5+etls19rFhzRveWd+n7SpBY32zmSWl/Q+39gdZ5O9i46M7 o5Nxsktbj8zfpkyb34jkHZyIffTiLQy32LOkJPCzbA/wwQQAFn4LU1kLs3EjTBIMdoXo fiz27wRJt/dXxH/C+T4Xfdf6XMDhhOFeOSN/gxtCsq2CaYkau5xoOCSBst0hjCa1Zyez LTqI2zK4xFSL5fhl0fm/vd08tK4qBFP78pA5TFVSR+00fpZ5c1Vbg/u+qusmZDN4WyHF F5FA== X-Gm-Message-State: AE9vXwNckicFU14fvwLl31kFVdbQUlkoXc1pxYMb1YS8oYrdEI7ao6fPPWHvti3a3iabjey+REo= X-Received: by 10.55.48.9 with SMTP id w9mr11492768qkw.147.1473184501985; Tue, 06 Sep 2016 10:55:01 -0700 (PDT) Return-Path: Received: from localhost.localdomain (pool-72-71-243-24.cncdnh.fast00.myfairpoint.net. [72.71.243.24]) by smtp.googlemail.com with ESMTPSA id s56sm12767991qts.4.2016.09.06.10.55.00 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 06 Sep 2016 10:55:01 -0700 (PDT) From: David Long To: Masami Hiramatsu , Ananth N Mavinakayanahalli , Anil S Keshavamurthy , "David S. Miller" , Will Deacon , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com, Sandeepa Prabhu , William Cohen , Pratyush Anand Cc: Mark Brown Subject: [PATCH v2] arm64: Improve kprobes test for atomic sequence Date: Tue, 6 Sep 2016 13:54:59 -0400 Message-Id: <1473184499-6018-1-git-send-email-dave.long@linaro.org> X-Mailer: git-send-email 2.5.0 From: "David A. Long" Kprobes searches backwards a finite number of instructions to determine if there is an attempt to probe a load/store exclusive sequence. It stops when it hits the maximum number of instructions or a load or store exclusive. However this means it can run up past the beginning of the function and start looking at literal constants. This has been shown to cause a false positive and blocks insertion of the probe. To fix this, further limit the backwards search to stop if it hits a symbol address from kallsyms. The presumption is that this is the entry point to this code (particularly for the common case of placing probes at the beginning of functions). This also improves efficiency by not searching code that is not part of the function. There may be some possibility that the label might not denote the entry path to the probed instruction but the likelihood seems low and this is just another example of how the kprobes user really needs to be careful about what they are doing. Signed-off-by: David A. Long --- arch/arm64/kernel/probes/decode-insn.c | 48 +++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 15 deletions(-) -- 2.5.0 diff --git a/arch/arm64/kernel/probes/decode-insn.c b/arch/arm64/kernel/probes/decode-insn.c index 37e47a9..356ee52 100644 --- a/arch/arm64/kernel/probes/decode-insn.c +++ b/arch/arm64/kernel/probes/decode-insn.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -122,7 +123,7 @@ arm_probe_decode_insn(kprobe_opcode_t insn, struct arch_specific_insn *asi) static bool __kprobes is_probed_address_atomic(kprobe_opcode_t *scan_start, kprobe_opcode_t *scan_end) { - while (scan_start > scan_end) { + while (scan_start >= scan_end) { /* * atomic region starts from exclusive load and ends with * exclusive store. @@ -144,26 +145,43 @@ arm_kprobe_decode_insn(kprobe_opcode_t *addr, struct arch_specific_insn *asi) kprobe_opcode_t insn = le32_to_cpu(*addr); kprobe_opcode_t *scan_start = addr - 1; kprobe_opcode_t *scan_end = addr - MAX_ATOMIC_CONTEXT_SIZE; + unsigned long size = 0, offset = 0; #if defined(CONFIG_MODULES) && defined(MODULES_VADDR) struct module *mod; #endif - if (addr >= (kprobe_opcode_t *)_text && - scan_end < (kprobe_opcode_t *)_text) - scan_end = (kprobe_opcode_t *)_text; + /* + * If there's a symbol defined in front of and near enough to + * the probe address assume it is the entry point to this + * code and use it to further limit how far back we search + * when determining if we're in an atomic sequence. + */ + if (kallsyms_lookup_size_offset((unsigned long) addr, &size, &offset)) + if (offset < (MAX_ATOMIC_CONTEXT_SIZE*sizeof(kprobe_opcode_t))) + scan_end = addr - (offset / sizeof(kprobe_opcode_t)); + + if (scan_end <= scan_start) { + if (addr >= (kprobe_opcode_t *)_text && + scan_end < (kprobe_opcode_t *)_text) + scan_end = (kprobe_opcode_t *)_text; #if defined(CONFIG_MODULES) && defined(MODULES_VADDR) - else { - preempt_disable(); - mod = __module_address((unsigned long)addr); - if (mod && within_module_init((unsigned long)addr, mod) && - !within_module_init((unsigned long)scan_end, mod)) - scan_end = (kprobe_opcode_t *)mod->init_layout.base; - else if (mod && within_module_core((unsigned long)addr, mod) && - !within_module_core((unsigned long)scan_end, mod)) - scan_end = (kprobe_opcode_t *)mod->core_layout.base; - preempt_enable(); - } + else { + preempt_disable(); + mod = __module_address((unsigned long)addr); + if (mod && + within_module_init((unsigned long)addr, mod) && + !within_module_init((unsigned long)scan_end, mod)) + scan_end = + (kprobe_opcode_t *)mod->init_layout.base; + else if (mod && + within_module_core((unsigned long)addr, mod) && + !within_module_core((unsigned long)scan_end, mod)) + scan_end = + (kprobe_opcode_t *)mod->core_layout.base; + preempt_enable(); + } #endif + } decoded = arm_probe_decode_insn(insn, asi); if (decoded == INSN_REJECTED ||