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.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS, URIBL_BLOCKED,USER_AGENT_NEOMUTT autolearn=ham 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 7D7B6C10F0E for ; Fri, 12 Apr 2019 13:31:03 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 3E0222084D for ; Fri, 12 Apr 2019 13:31:03 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="js/zh5Z0"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=armlinux.org.uk header.i=@armlinux.org.uk header.b="cKSal2bd" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3E0222084D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=armlinux.org.uk Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-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=bombadil.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=rFlhyGHtLmWpYUZ4kwnIMviRExuo2FuL8useRPoVpnQ=; b=js/zh5Z0QOBLad xQx1tBFiQ40COD3cIQ01h4f/Cbt+cf1IwVREk2k5xlYKfkPLuBj2F6I8POTtlyvdg3ck3yXUy5qVT OW2JY5oSTkaGEtQG3UED8tRb00ucqgt+9VfTUB4DtymOSQR2a3mcfdq0C5WDUNm/kjLP48LzqIFlX 7csSPoLlji2XEXtawu7c1bYagza0+4AmGYc2b6DqfSxOD8vlH85VcKxV05BE/+/kzD6LuicWK3URG 74wgtcvQEuAtlYbPkaiiUxSsiU1+ZUoL0aSy+jJJfqTvPWw+HVxgN177itPhuEh+XbW+i10e2AXpJ hGRhAOW+alxSp1ZFEkFA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hEwGi-0004QW-63; Fri, 12 Apr 2019 13:31:00 +0000 Received: from pandora.armlinux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:1be6]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1hEwGc-0004Q1-LD for linux-arm-kernel@lists.infradead.org; Fri, 12 Apr 2019 13:30:58 +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=s5RTpBeSPla3xoKXscafNP+2TvgnmCYcPARC8ng0Z28=; b=cKSal2bdi/Un4TlIoDMZxG6OY hQUpUgdjCMttZCYmm41TqiFtDi6lE32DOPyeBMJ0rAlzxF3fXArR4+x/zROhuknVnSssQurfnTIW1 vjcpEf5llxKQDOu3ZFGuoILF/QVM4PJdRKPrnXM10llMh0ztnudY9n3ZUMpKWQpDdtTtno68tVZI1 0FgPi+IP+hR8MlC3m+H7c/zyw1aWuXU8/cpNDpNfciR8YGq7O6+LM5I6KmY3IFh5VZGuptHgHbKHi V9WWGy1VVrILIr5o5re0qlpIYapJ8j+DwqQBP7fKaxuUfveaqDEhYwsbRqxeYpCDqiYeE1HMMxCYC U5JIUOG9w==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:52076) by pandora.armlinux.org.uk with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1hEwGY-0001DR-34; Fri, 12 Apr 2019 14:30:50 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.89) (envelope-from ) id 1hEwGX-0004u2-8l; Fri, 12 Apr 2019 14:30:49 +0100 Date: Fri, 12 Apr 2019 14:30:49 +0100 From: Russell King - ARM Linux admin To: Will Deacon Subject: Re: [PATCH v3 1/4] arm64: compat: Alloc separate pages for vectors and sigpage Message-ID: <20190412133049.v6fospdhj3uhcqkt@shell.armlinux.org.uk> References: <20190402162757.13491-1-vincenzo.frascino@arm.com> <20190402162757.13491-2-vincenzo.frascino@arm.com> <20190410181017.GA25618@fuggles.cambridge.arm.com> <7a4e4984-5448-5b93-5270-5b27abbec7f7@arm.com> <20190412115507.GA28260@fuggles.cambridge.arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190412115507.GA28260@fuggles.cambridge.arm.com> User-Agent: NeoMutt/20170113 (1.7.2) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190412_063055_925753_ED0F6D1C X-CRM114-Status: GOOD ( 39.20 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Catalin Marinas , Vincenzo Frascino , 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+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Apr 12, 2019 at 12:55:07PM +0100, Will Deacon wrote: > On Fri, Apr 12, 2019 at 12:08:30PM +0100, Vincenzo Frascino wrote: > > On 10/04/2019 19:10, Will Deacon wrote: > > > On Tue, Apr 02, 2019 at 05:27:54PM +0100, Vincenzo Frascino wrote: > > >> +static int aarch32_kuser_helpers_setup(struct mm_struct *mm) > > >> { > > >> - struct mm_struct *mm = current->mm; > > >> - unsigned long addr = AARCH32_VECTORS_BASE; > > >> - static const struct vm_special_mapping spec = { > > >> - .name = "[vectors]", > > >> - .pages = vectors_page, > > >> + void *ret; > > >> + > > >> + /* The kuser helpers must be mapped at the ABI-defined high address */ > > >> + ret = _install_special_mapping(mm, AARCH32_KUSER_BASE, PAGE_SIZE, > > >> + VM_READ | VM_EXEC | > > >> + VM_MAYREAD | VM_MAYEXEC, > > > > > > How come you don't need VM_MAYWRITE here... > > > > > > > This is to keep it consistent with what the arm (32 bit) implementation does. > > > > My understanding is that the kuser code is executed in user mode for efficiency > > reasons but it is too close to the kernel to be implemented in user libraries > > and that the kernel can change its internal implementation from version to > > version as far as it guarantees the "interface" (entry points and results). > > Based on this gdb should not need to put a breakpoint inside the kuser helpers code. That is not the point at all. The kuser code is to give userspace independence of two things: 1. The CPU architecture it is running on (whether it be SMP or UP.) 2. The configuration of the kernel. 3. The method with which the per-thread value is obtained. 4. Atomic compare-exchange. If it was only (1), then maybe it would be possible to have had the userspace dynamic linker figure out which libraries to load, but (2), (3) and (4) make that extremely complex to manage. For (3), there are two ways to get the thread value: 1. Reading it from the userspace visible thread-private CPU register. This is fine for CPUs that implement it, but not all ARM CPUs implement this register. 2. Reading it from 0xffff0ffc, but only when the kernel is configured to write it there. Which one works is a function of the kernel configuration (hence 2) and the CPU capabilities. For (4), there is a need to provide modern libraries with a way to implement the cmpxchg() semantics. This is easy in ARMv6+, as there are the load-exclusive and store-exclusive instructions, but on earlier architectures, there is only one atomic instruction which is essentially an exchange operation - and that can't be used to provide cmpxchg() semantics. To work around that, there is code in the kuser page which provides cmpxchg() semantics with the help of the kernel fixing things up if an exception happens while userspace is executing that code. Pushing that code into a library means that the kernel has to be aware, whenever _any_ exception occurs, whether the PC is in that code, and dealing with that efficiently if it isn't at a fixed address becomes problematical - it's overhead incurred on almost every exception. What's more is that the kernel has to be aware of the exact code so it knows which range of PC values are required to be fixed up - since the fixup involves changing the userland state, inappropriately changing it will corrupt the program execution. > Hmm, but couldn't you apply the same reasoning to the sigpage? The sigpage is mapped in the userspace address range, and gdb has the expectation that it can set breakpoints (which involves writing via ptrace) to pages. If we can't set breakpoints in the sigpage or vdso, we end up losing control of the executable while trying to single step it if the executable branches into such places - or gdb refuses to step. > [also, talking to Russell, he makes the very good point that you can't CoW > the page containing the vectors because that would give userspace control > over the vectors themselves!] Yes, the vectors page is very special and gdb knows about it - it can't be CoW'd because that would give userspace a way to modify the machine vectors, thereby taking control of the machine in a privileged execution state. > > And if we consider as well that the fixed address nature of the helpers could be > > used from ROP authors during the creation of exploits probably we want to > > prevent gdb to set a breakpoint there hence the proposed patch does not contain > > VM_MAYWRITE. > > I'm not sure I buy the ROP angle... you need to GUP the thing to get the > write to happen. Maybe you could do it with a futex or something, but I'm > also not sure that's really a viable attack. This came up on 32-bit. Yes, ROP is a concern, that's why we changed things around a bit a few years ago, and also made it possible to disable the kuser helpers page entirely for maximum security - but doing so has been biting people. Last merge window, I merged a patch which made the kernel state when such an executable was run, because people were complaining that it was difficult to work out what was happening. ROP here is not about changing the data in the page, but exploiting the instructions already there. In the old says, most of the page was filled with zeros, which meant you could branch before a kuser helper and we'd execute "andeq r0, r0, r0" instructions until we hit one. One of the changes to improve security was to fill the page with instructions guaranteed to fault in either ARM or Thumb mode, thereby reducing the possibility of ROP based attack on systems where the kuser page was still present. Another change that was made was to move the vector stubs out of the vectors page into a page at 0xffff1000 which was inaccessible to the user, so the only instructions userspace can see are the branches in each of the vectors to the inaccessible code at 0xffff1000. This also has the effect of hiding the address of the kernel entry points from userspace. > > I had a look to arm implementation and it seems the it defines the vector page > > as READONLY_EXEC and there is no VM_MAYWRITE in the vm_flags. > > > > I could extend the comment accordingly. > > I initially thought that the gate VMA would mean that VM_MAYWRITE was > implicit for GUP, but actually that's handled explicitly in get_gate_page(): > > /* user gate pages are read-only */ > if (gup_flags & FOLL_WRITE) > return -EFAULT; > > so yes, I agree with you that this is consistent with 32-bit. What I'm not > sure about is why we need to CoW the sigpage. But I suppose being compatible > with 32-bit is the aim of the game, so this is all moot. On 32-bit ARM, the "gate VMA" describes the vectors page mapping, not the sigpage. The sigpage is CoW-able to support gdb. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel