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=-1.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS 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 5892BC433E1 for ; Tue, 23 Jun 2020 17:16:16 +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 231D420781 for ; Tue, 23 Jun 2020 17:16:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="ibWF5fEM" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 231D420781 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com 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=jhvAcQ0ruNFRlhFyc7wSgUk9zENDrVqv28v0A87Wakk=; b=ibWF5fEMBWc1HQcPOAZI+kWJj 2db4ifWPEBMa5hdLrF6QOjUkx0/aIrx+9rI3aYtMjm0vkLBLbMJ2AnLbPbG1GWLUIfEqLBk86KatC F0qXIRq9Ad73YlFQxXKHrSFeB7mVJLjLefjxVsO+HXo00VhPAyJgROfn3Ns49nBw0hijkeXcXRVIr TQ8atV5EB9uW9jtad5IFezHJZ2fywgFVsEojGebW6WGx2RrePi+4g/CvRBoFKiv6dIjOyGyHKMRgS D4UDYjPvsl8bNFoXclH19xCX2GHftbTaa8Gwnqy0Z5YTEzc6cR0JNGkXxPD5OFrHp2eG0nIoY75ev rDPoE+x3g==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jnmUD-0004Kf-Pj; Tue, 23 Jun 2020 17:13:29 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jnmDQ-0003L0-4i for linux-arm-kernel@lists.infradead.org; Tue, 23 Jun 2020 16:56:12 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D6B3B31B; Tue, 23 Jun 2020 09:56:06 -0700 (PDT) Received: from C02TD0UTHF1T.local (unknown [10.57.20.203]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id AD50F3F7BB; Tue, 23 Jun 2020 09:56:04 -0700 (PDT) Date: Tue, 23 Jun 2020 17:55:57 +0100 From: Mark Rutland To: Will Deacon Subject: Re: [PATCH] arm64: don't preempt_disable in do_debug_exception Message-ID: <20200623165557.GA12767@C02TD0UTHF1T.local> References: <1592501369-27645-1-git-send-email-paul.gortmaker@windriver.com> <20200623155900.GA4777@willie-the-truck> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200623155900.GA4777@willie-the-truck> 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 , Naresh Kamboju , stable@vger.kernel.org, Paul Gortmaker , James Morse , Masami Hiramatsu , "Paul E . McKenney" , 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 Tue, Jun 23, 2020 at 04:59:01PM +0100, Will Deacon wrote: > On Thu, Jun 18, 2020 at 01:29:29PM -0400, Paul Gortmaker wrote: > > In commit d8bb6718c4db ("arm64: Make debug exception handlers visible > > from RCU") debug_exception_enter and exit were added to deal with better > > tracking of RCU state - however, in addition to that, but not mentioned > > in the commit log, a preempt_disable/enable pair were also added. > > > > Based on the comment (being removed here) it would seem that the pair > > were not added to address a specific problem, but just as a proactive, > > preventative measure - as in "seemed like a good idea at the time". > > > > The problem is that do_debug_exception() eventually calls out to > > generic kernel code like do_force_sig_info() which takes non-raw locks > > and on -rt enabled kernels, results in things looking like the following, > > since on -rt kernels, it is noticed that preemption is still disabled. > > > > BUG: sleeping function called from invalid context at kernel/locking/rtmutex.c:975 > > in_atomic(): 1, irqs_disabled(): 0, pid: 35658, name: gdbtest > > Preemption disabled at: > > [] do_debug_exception+0x38/0x1a4 > > Call trace: > > dump_backtrace+0x0/0x138 > > show_stack+0x24/0x30 > > dump_stack+0x94/0xbc > > ___might_sleep+0x13c/0x168 > > rt_spin_lock+0x40/0x80 > > do_force_sig_info+0x30/0xe0 > > force_sig_fault+0x64/0x90 > > arm64_force_sig_fault+0x50/0x80 > > send_user_sigtrap+0x50/0x80 > > brk_handler+0x98/0xc8 > > do_debug_exception+0x70/0x1a4 > > el0_dbg+0x18/0x20 > > > > The reproducer was basically an automated gdb test that set a breakpoint > > on a simple "hello world" program and then quit gdb once the breakpoint > > was hit - i.e. "(gdb) A debugging session is active. Quit anyway? " > > Hmm, the debug exception handler path was definitely written with the > expectation that preemption is disabled, so this is unfortunate. For > exceptions from kernelspace, we need to keep that guarantee as we implement > things like BUG() using this path. For exceptions from userspace, it's > plausible that we could re-enable preemption, but then we should also > re-enable interrupts and debug exceptions too because we don't > context-switch pstate in switch_to() and we would end up with holes in our > kernel debug coverage (and these might be fatal if e.g. single step doesn't > work in a kprobe OOL buffer). However, that then means that any common code > when handling user and kernel debug exceptions needs to be re-entrant, > which it probably isn't at the moment (I haven't checked). I'm pretty certain existing code is not reentrant, and regardless it's going to be a mess to reason about this generally if we have to undo our strict exception nesting rules. I reckon we need to treat this like an NMI instead -- is that plausible? Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel