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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B2947C4725D for ; Fri, 19 Jan 2024 12:15:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc: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=aBH9kZqaY7XsyPTI9g89T3JSuAJ9+yvrMrpyKWkMLZU=; b=bbwr12vQr0gtno JJFN/CGgwutS/6qbcQ1HBqMXaI3AB0kof14JpeMrDAJZqXLonhbbcpYH4bZQNxPVvKf7Q3u+2/WHJ kv0d1oJpzSHjMJNMVq+YCzdH8wnAYAg7FfnNWCtCpN/mNcsryuVaZFUKB8ZqE+i7G/071iTebJ+zp OK4zLo3m+DvPpuCNMxkq+KZFrVdmHgOhEaLww8Ox1Q6yd+t+6kqwPi2sSkjD+RQ/fzWzqUzkwUmzO knC8Jgw8hUqyatYdWJNFktwNpa0zXR6yWPxso2elqRNw3YfshntDrUwnzj0EE2YH68g2/UTmNfv2+ 0gIycg6abahvW+48mfmg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1rQnlf-005SOE-1d; Fri, 19 Jan 2024 12:14:39 +0000 Received: from pandora.armlinux.org.uk ([2001:4d48:ad52:32c8:5054:ff:fe00:142]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1rQnlb-005SNe-38 for linux-arm-kernel@lists.infradead.org; Fri, 19 Jan 2024 12:14:38 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=w78D1n+JlQDHNUL2B3PbP7luQWKLRSx8WHKXjoF7kRU=; b=b8PtxN+0VYaIGIV5f2OB/cgdk5 ZV7qk01rkPJnQSpq743zi8yc+M3FwdXzikW8ssqk7J6WcVKXK8O8GUdDmEBcEZFNg4k1ldXNKqzEC i/v1INedBZSqU1dZdjqlNxlS06JdlnYeBB7lEvETASi2Cwklx3ASHpOgPoAdHKZNAVuWFTZIRh+cR +Pmlq4gNEzFKCoHNGE+WW16O8Sfega64D0fzocSqxM8JCtafI5sHDLNzRqdRhvSJdeBvCKsTMu88S jHkzRlAdv60TWAhhJQO9uVAElCXK3O8ykpj6gt2L4fqwL3yAOWLIWfLIxVOGHdAnUeE2camWnYgpD IGCa1slw==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:41148) by pandora.armlinux.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1rQnlT-0006hZ-1B; Fri, 19 Jan 2024 12:14:27 +0000 Received: from linux by shell.armlinux.org.uk with local (Exim 4.94.2) (envelope-from ) id 1rQnlR-0006ec-7H; Fri, 19 Jan 2024 12:14:25 +0000 Date: Fri, 19 Jan 2024 12:14:25 +0000 From: "Russell King (Oracle)" To: Ard Biesheuvel Cc: Kees Cook , Ard Biesheuvel , linux-arm-kernel@lists.infradead.org, Mark Brown , Zhen Lei , Linus Walleij Subject: Re: [PATCH] ARM: mm: Disregard user space addresses in BUG() address check Message-ID: References: <20240117150733.2608655-2-ardb+git@google.com> <202401181125.D48DCB4C@keescook> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240119_041436_011545_D8B5A047 X-CRM114-Status: GOOD ( 35.48 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 Fri, Jan 19, 2024 at 12:52:21PM +0100, Ard Biesheuvel wrote: > On Thu, 18 Jan 2024 at 21:15, Kees Cook wrote: > > > > On Wed, Jan 17, 2024 at 04:07:34PM +0100, Ard Biesheuvel wrote: > > > From: Ard Biesheuvel > > > > > > is_valid_bugaddr() dereferences the faulting PC to fetch the instruction > > > that triggered the fault, to decide whether it is a BRK instruction used > > > to force an exception. This is used by the BUG() infrastructure to keep > > > the handling logic (which should never execute) separate from the code > > > that normally runs. > > > > > > This dereference may attempt to access user memory if the faulting PC > > > happens to contain a user address. One way this might happen is when > > > the kernel is tricked into executing from user space while PAN > > > protections (Privileged Access Never) are in effect: the instruction > > > fetch will trigger a prefetch abort, the handling of which involves a > > > check whether the instruction that caused it is a BRK, requiring a > > > load from the same address. This load is privileged too, and so it will > > > trigger another exception, which we fail to recover from. > > > > > > Given that BRK instructions tied to BUG() handling can only appear in > > > kernel code, let's check first that the PC actually points into kernel > > > memory. > > > > > > Cc: Kees Cook > > > Cc: Russell King > > > Cc: Mark Brown > > > Cc: Zhen Lei > > > Cc: Linus Walleij > > > Link: https://lkml.kernel.org/r/202401111544.18EBB6AA%40keescook > > > Signed-off-by: Ard Biesheuvel > > > --- > > > arch/arm/kernel/traps.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c > > > index 3bad79db5d6e..f342bd6b2a5d 100644 > > > --- a/arch/arm/kernel/traps.c > > > +++ b/arch/arm/kernel/traps.c > > > @@ -402,6 +402,9 @@ int is_valid_bugaddr(unsigned long pc) > > > u32 insn = __opcode_to_mem_arm(BUG_INSTR_VALUE); > > > #endif > > > > > > + if (pc < TASK_SIZE) > > > + return 0; > > > + > > > if (get_kernel_nofault(bkpt, (void *)pc)) > > > return 0; > > > > > > > Okay, I've finally had a chance to test this and it doesn't fix it > > for me. I still see the double domain fault, and I still lose a CPU. > > I patched in a pr_info() to show that the "pc < TASK_SIZE" was hitting > > (which it is) so I see: > > > > > > kdtm: Performing direct entry EXEC_USERSPACE > > lkdtm: attempting ok execution at 80761b4c > > lkdtm: attempting bad execution at 76ff4000 > > Unhandled prefetch abort: page domain fault (0x01b) at 0x76ff4000 > > Skipping is_valid_bugaddr(76ff4000) > > Internal error: : 1b [#12] SMP ARM > > Modules linked in: > > ... > > Process cat (pid: 1301, stack limit = 0x3c8ec418) > > Stack: (0xf0975e58 to 0xf0976000) > > ... > > Backtrace: > > lkdtm_EXEC_USERSPACE from lkdtm_do_action+0x2c/0x4c > > ... > > Exception stack(0xf0975fa8 to 0xf0975ff0) > > ... > > 8<--- cut here --- > > Unhandled fault: page domain fault (0x01b) at 0x76ff3ff0 > > [76ff3ff0] *pgd=44baa835, *pte=00000000, *ppte=00000000 > > Internal error: : 1b [#13] SMP ARM > > Modules linked in: > > ... > > Process cat (pid: 1301, stack limit = 0x3c8ec418) > > Stack: (0xf0975ce0 to 0xf0976000) > > ... > > Backtrace: > > copy_from_kernel_nofault from dump_instr+0x1cc/0x208 > > r7:f0975d27 r6:80ddf3d8 r5:f0975e08 r4:fffffffc > > dump_instr from die+0x294/0x2f0 > > This is indeed a similar but different issue (as you conclude below). > dump_instr() attempts to dereference PC to generate the 'Code: xxxx > xxxx' line with the opcodes leading up to the fault, and this triggers > a PAN fault for the same reason. Both of these have faulted in copy_from_kernel_nofault(), so rather than adding these tests all over the place, wouldn't it be better to implement copy_from_kernel_nofault_allowed() so that we verify that the source is actually something we can copy from? E.g. bool copy_from_kernel_nofault_allowed(const void *src, size_t size) { unsigned long srcv = (unsigned long)src; return src >= TASK_SIZE && src + size - 1 >= TASK_SIZE; } -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel