From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754077AbcILQ3t (ORCPT ); Mon, 12 Sep 2016 12:29:49 -0400 Received: from mail.kernel.org ([198.145.29.136]:55846 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752778AbcILQ3r (ORCPT ); Mon, 12 Sep 2016 12:29:47 -0400 Date: Tue, 13 Sep 2016 01:29:36 +0900 From: Masami Hiramatsu To: David Long Cc: 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 , Mark Brown Subject: Re: [PATCH v3] arm64: Improve kprobes test for atomic sequence Message-Id: <20160913012936.489d3dc7a0b5a9e0f0670745@kernel.org> In-Reply-To: <57D60AA7.6010304@linaro.org> References: <1473449169-16032-1-git-send-email-dave.long@linaro.org> <20160910144841.1bbbffd8548128146df11a41@kernel.org> <57D60AA7.6010304@linaro.org> X-Mailer: Sylpheed 3.5.0 (GTK+ 2.24.30; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, 11 Sep 2016 21:53:43 -0400 David Long wrote: > On 09/10/2016 01:48 AM, Masami Hiramatsu wrote: > > On Fri, 9 Sep 2016 15:26:09 -0400 > > David Long wrote: > > > >> 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. > > > > Of course user should be careful, but also, in such case, kernel can reject > > to probe it. > > > > I'm not exactly sure what you mean. I'm just saying when everything > goes right we still cannot promise perfection in detecting a probe > within an atomic sequence. This patch will reject a probe that is after > a ldx and has no intervening kallsyms label (and assuming it's within > the defined maximum count of subsequent instructions). > Hmm, what I meant was the below code. > >> + /* > >> + * 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 we could > >> + * not find any symbol skip the atomic test altogether as we > >> + * could otherwise end up searching irrelevant text/literals. > >> + * KPROBES depends on KALLSYMS so this last case should never > >> + * happen. > >> + */ > >> + 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)); > >> + else > >> + scan_end = addr - MAX_ATOMIC_CONTEXT_SIZE; > > > > } else > > return INSN_REJECTED; > > > > that is what I expected... As you said above, > >> + * KPROBES depends on KALLSYMS so this last case should never > >> + * happen. If it should never happen, it also would be better to reject it because it is unexpected result. Thank you, -- Masami Hiramatsu