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=-3.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no 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 12A28C2D0A3 for ; Tue, 3 Nov 2020 09:15:26 +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 8057820756 for ; Tue, 3 Nov 2020 09:15:25 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="KXHZ5EOP"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="b7nT9cEn" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8057820756 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:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=yz1XLl39YUL5Rv3Ag1woiJemlF8Zcs/I8UvhpCAzwAE=; b=KXHZ5EOP6woBTjeonJF/lRJPZ 1gYBThyAORjbg5XdwwaF9R/LdNGnpulHj/0TBEIS7VbfRIi22P/PtAMiIZPYfH14WfyG6n9oAHCpV /g0EQYCB7hRL9fKogVgJstrRePk/KMqPir6IFfj/jgCjgnrXQG1M0i5jNCZ/V39qR2ZaA5i//mf3o //STK2gBuT/oEOYZyM2kW63fG7h1SFP0WjfcgpjNaqhPElqHFS8sxwyc6lNnSzln3idsWAQVgSHJK b/gyovtznbfvHJUtRdEliYy5O6bP9KnncoE/CFGmTdInmpBIx5dnuzNH1mtpT0jzqrAmMB5HFtC7n KCBQ46pMw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kZsNf-0003su-1E; Tue, 03 Nov 2020 09:13:31 +0000 Received: from mail-ed1-x544.google.com ([2a00:1450:4864:20::544]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kZsNb-0003rp-Pr for linux-arm-kernel@lists.infradead.org; Tue, 03 Nov 2020 09:13:28 +0000 Received: by mail-ed1-x544.google.com with SMTP id a71so12108242edf.9 for ; Tue, 03 Nov 2020 01:13:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=DEw0oZo3RDymH+Jx0x80m0dXjKMn+DPqJ4Cha8G7SLQ=; b=b7nT9cEnlj4MVzYGG9EGKNqj2hzJqbJ7TSQN2gUdLBV0FkhPFyWyqauu0dbcYMMcF6 kW5fQIbI73GKAUdIFosmzgBuSPl1GW2qpqY36H7CtbDUux33Cp+nqU6erS+cblZ4tfCm vTyBI8K4YHwtR4+vpJ1SFNK1PrI1dqFeFnWn8qXm0W6h1S7lR6D/LVGikrTk8HB7F3TV 8KmtfG2An/TJZhi1eJh+IjAnY0Sh9i1Y97GhW0x2wq0+GeTVJKS077MfQZoaauBDB64w xsNPiJ85oLvySFCfUbh8BDzVTLFOrTh5x5+Q2Kt8QzKHNglhdLkWlZgdQHfcIOB36sue nY1g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=DEw0oZo3RDymH+Jx0x80m0dXjKMn+DPqJ4Cha8G7SLQ=; b=O78hzoIQhCTot1EbZjNXEO9qOtC9khiYMopPNekprV8PSeQG4PBR2zmA4p3EmBhL5R S3+JawAF/HzlYfK4d8fEwGe/sufr2xB4nqUBwneUulDWV+so9QMhflhc+dt4ubCJhMrT RlQT5xFtQDf1GFN3kQnqkb3kmGXHlLsxsg1UoDzcfEtvc0pc4VI2Ql3yXV8bVly6pOX5 0nmsX91EHdksuJvlejcR5H7D8UQHUR9VsG3gyLBEBfbxchV7y0JMuwmNTH5f9GX/zaLY RzxmITxEmxZRUldzPQatZEBM65OBP33seNDkOQveNXA8Gh24H9hUc9nmLO9EUVajtrQt j7Pw== X-Gm-Message-State: AOAM532aGeBotEr5V9JCK6XqEi5rXFvTqGc541can+oaD9utWYjM4+j9 i38QvapxjFOzOI04kp6TBw/yVQ== X-Google-Smtp-Source: ABdhPJwLFrZhWUXtILJvPY1SXKh5ZqWE6EvOp2YrOB9e7I0MXezLUK45q+Pf+BAMDzK4ELNPRqClGg== X-Received: by 2002:aa7:d703:: with SMTP id t3mr9161277edq.375.1604394804430; Tue, 03 Nov 2020 01:13:24 -0800 (PST) Received: from myrica ([2001:1715:4e26:a7e0:116c:c27a:3e7f:5eaf]) by smtp.gmail.com with ESMTPSA id u10sm10576292ejh.54.2020.11.03.01.13.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Nov 2020 01:13:23 -0800 (PST) Date: Tue, 3 Nov 2020 10:13:05 +0100 From: Jean-Philippe Brucker To: Masami Hiramatsu Subject: Re: [PATCH] arm64: kprobes: Use BRK instead of single-step when executing instructions out-of-line Message-ID: <20201103091305.GA6723@myrica> References: <20201029173440.117174-1-jean-philippe@linaro.org> <20201102114152.GA3452@willie-the-truck> <20201102225255.fa991f3607c45bbbb161803c@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20201102225255.fa991f3607c45bbbb161803c@kernel.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201103_041327_886715_A9F0ED59 X-CRM114-Status: GOOD ( 22.39 ) 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: catalin.marinas@arm.com, Will Deacon , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Nov 02, 2020 at 10:52:55PM +0900, Masami Hiramatsu wrote: > > > - patch_text(p->addr, p->opcode); > > > + patch_text(p->addr, p->opcode, 0); > > > > I don't think patch_text() is offering an awful lot to these callers. Why > > don't we just call aarch64_insn_patch_text() directly, which I think will > > make the code easier to read too? > > Agreed. I guess it's just because histrically used. Yes it's easy to remove, sending a v2 > > > > 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; > > > > Can we set the D bit now? Would be one less thing to worry about. > > Hmm, I think we just ignore D bit above. If someone (kgdb?) is set D flag > then there may be no reason to prohibit it on the trampoline. > (Of course they must handle this case) Masking debug just for the trampoline would simplify reasoning about nesting exceptions. Although it prevents from debugging kprobes using KGDB, it seems reasonable to me because I don't think we can provide guarantees that KGDB works reliably with kprobes. For example it doesn't seem like a watchpoint would fire if the instruction performing the access is simulated by kprobe. Thanks, Jean _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel