From patchwork Thu Mar 6 03:38:49 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bharat Bhushan X-Patchwork-Id: 3780911 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id CDFCEBF540 for ; Thu, 6 Mar 2014 03:40:16 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id A3B242021B for ; Thu, 6 Mar 2014 03:40:15 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 5CEF9201F2 for ; Thu, 6 Mar 2014 03:40:14 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WLP9z-0007FN-OH; Thu, 06 Mar 2014 03:39:51 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1WLP9x-0001NT-FM; Thu, 06 Mar 2014 03:39:49 +0000 Received: from mail-bl2ln0107.outbound.protection.outlook.com ([2a01:111:f400:7c09::107] helo=na01-bl2-obe.outbound.protection.outlook.com) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WLP9r-0001Mu-Sx for linux-arm-kernel@lists.infradead.org; Thu, 06 Mar 2014 03:39:47 +0000 Received: from BN1PR03MB266.namprd03.prod.outlook.com (10.255.200.15) by DM2PR03MB349.namprd03.prod.outlook.com (10.141.54.11) with Microsoft SMTP Server (TLS) id 15.0.888.9; Thu, 6 Mar 2014 03:38:50 +0000 Received: from BN1PR03MB266.namprd03.prod.outlook.com ([169.254.13.55]) by BN1PR03MB266.namprd03.prod.outlook.com ([169.254.13.55]) with mapi id 15.00.0893.001; Thu, 6 Mar 2014 03:38:50 +0000 From: "Bharat.Bhushan@freescale.com" To: Laura Abbott , Will Deacon Subject: RE: [PATCH v2] ARM64: Kernel managed pages are only flushed Thread-Topic: [PATCH v2] ARM64: Kernel managed pages are only flushed Thread-Index: AQHPOGYcsrSGWDWrVkyN25sgZ9qA3ZrSqr0AgAABMoCAAD9bAIAAep3g Date: Thu, 6 Mar 2014 03:38:49 +0000 Message-ID: <93f096142bd64a7b8f51930277d82c55@BN1PR03MB266.namprd03.prod.outlook.com> References: <1394018716-17075-1-git-send-email-Bharat.Bhushan@freescale.com> <20140305161255.GG29309@mudshark.cambridge.arm.com> <06b7685849ef4682878556ea1ea8f9d6@BN1PR03MB266.namprd03.prod.outlook.com> <5317832E.9020809@codeaurora.org> In-Reply-To: <5317832E.9020809@codeaurora.org> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [192.88.169.1] x-forefront-prvs: 0142F22657 x-forefront-antispam-report: SFV:NSPM; SFS:(10009001)(6009001)(428001)(479174003)(24454002)(377454003)(164054003)(13464003)(51704005)(199002)(189002)(50986001)(49866001)(59766001)(92566001)(83322001)(81686001)(19580395003)(93516002)(93136001)(33646001)(80976001)(69226001)(19580405001)(4396001)(47446002)(31966008)(95416001)(63696002)(74502001)(54316002)(94946001)(54356001)(77982001)(79102001)(575784001)(53806001)(66066001)(46102001)(56816005)(86362001)(81542001)(77096001)(90146001)(80022001)(76576001)(94316002)(74316001)(76482001)(74706001)(85852003)(85306002)(2656002)(47736001)(47976001)(81342001)(74366001)(81816001)(83072002)(95666003)(51856001)(87266001)(87936001)(65816001)(76786001)(76796001)(74876001)(24736002); DIR:OUT; SFP:1101; SCL:1; SRVR:DM2PR03MB349; H:BN1PR03MB266.namprd03.prod.outlook.com; CLIP:192.88.169.1; FPR:2C3DF1D6.BE22BF0D.8E530BB.EC86AC1.20552; MLV:sfv; PTR:InfoNoRecords; A:1; MX:1; LANG:en; received-spf: None (: freescale.com does not designate permitted sender hosts) MIME-Version: 1.0 X-OriginatorOrg: freescale.com X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140305_223946_303158_651480D3 X-CRM114-Status: GOOD ( 32.17 ) X-Spam-Score: -1.9 (-) Cc: Scott Wood , Catalin Marinas , Stuart Yoder , "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP > -----Original Message----- > From: Laura Abbott [mailto:lauraa@codeaurora.org] > Sent: Thursday, March 06, 2014 1:34 AM > To: Bhushan Bharat-R65777; Will Deacon > Cc: Wood Scott-B07421; Catalin Marinas; Yoder Stuart-B08248; linux-arm- > kernel@lists.infradead.org > Subject: Re: [PATCH v2] ARM64: Kernel managed pages are only flushed > > On 3/5/2014 8:27 AM, Bharat.Bhushan@freescale.com wrote: > > > > > >> -----Original Message----- > >> From: Will Deacon [mailto:will.deacon@arm.com] > >> Sent: Wednesday, March 05, 2014 9:43 PM > >> To: Bhushan Bharat-R65777 > >> Cc: Catalin Marinas; linux-arm-kernel@lists.infradead.org; Bhushan > >> Bharat-R65777 > >> Subject: Re: [PATCH v2] ARM64: Kernel managed pages are only flushed > >> > >> On Wed, Mar 05, 2014 at 11:25:16AM +0000, Bharat Bhushan wrote: > >>> Kernel can only access pages which maps to managed memory. > >>> So flush only valid kernel pages. > >>> > >>> I observed kernel crash direct assigning a device using VFIO and > >>> found that it was caused because of accessing invalid page > >>> > >>> Signed-off-by: Bharat Bhushan > >>> --- > >>> v1->v2 > >>> Getting pfn usin pte_pfn() in pfn_valid. > >>> > >>> arch/arm64/mm/flush.c | 13 ++++++++++++- > >>> 1 files changed, 12 insertions(+), 1 deletions(-) > >>> > >>> diff --git a/arch/arm64/mm/flush.c b/arch/arm64/mm/flush.c index > >>> e4193e3..319826a 100644 > >>> --- a/arch/arm64/mm/flush.c > >>> +++ b/arch/arm64/mm/flush.c > >>> @@ -72,7 +72,18 @@ void copy_to_user_page(struct vm_area_struct > >>> *vma, struct page *page, > >>> > >>> void __sync_icache_dcache(pte_t pte, unsigned long addr) { > >>> - struct page *page = pte_page(pte); > >>> + struct page *page; > >>> + > >>> +#ifdef CONFIG_HAVE_ARCH_PFN_VALID > >>> + /* > >>> + * We can only access pages that the kernel maps > >>> + * as memory. Bail out for unmapped ones. > >>> + */ > >>> + if (!pfn_valid(pte_pfn(pte))) > >>> + return; > >>> + > >>> +#endif > >>> + page = pte_page(pte); > >> > >> How do you get into this function without a valid, userspace, executable pte? > >> > >> I suspect you've got changes elsewhere and are calling this function > >> in a context where it's not supposed to be called. > > > > Below I will describe the context in which this function is called: > > > > When we direct assign a bus device (we have a different freescale > > specific bus > > device but we can take PCI device for discussion as this logic applies > equally > for PCI device I think) to user space using VFIO. Then userspace > needs to > mmap(PCI_BARx_offset: this PCI bar offset in not a kernel visible > memory). > > Then VFIO-kernel mmap() ioctl code calls remap_pfn_range() for > > mapping the > >requested address. While remap_pfn_range() internally calls this function. > > > > As someone who likes calling functions in context where they aren't supposed to > be called, I took a look a this because I was curious. Are we saying that remap_pfn_range() should not be called in such case (described earlier the case of direct assigning PCI device to user space using VFIO) ? But x86/powerpc calls this function only. > > I can confirm the same problem trying to mmap arbitrary io address space with > remap_pfn_range. We should only be hitting this if the pte is marked as exec per > set_pte_at. With my test case, even mmaping with only PROT_READ and PROT_WRITE > was setting PROT_EXEC as well which was triggering the bug. This seems to be > because READ_IMPLIES_EXEC personality was set which was derived from > > #define elf_read_implies_exec(ex,stk) (stk != EXSTACK_DISABLE_X) > > and none of the binaries I'm generating seem to be setting the stack execute bit > either way (all are EXECSTACK_DEFAULT). Yes I agree that even if we set PROT_READ and PROT_WRITE but it internally end up setting PROT_EXEC, so we enter in flow. But I see this as a second issue. I am not sure but theoretically it can still happen that we set PROT_EXEC for anonymous page. So either __sync_icache_dcache() should check that it does not access anonymous struct page (which this patch is doing) or __sync_icache_dcache() should not be called for anonymous page. Maybe something like this: Please suggest if some other solution. Thanks -Bharat > > It's not obvious what the best solution is here. > > Thanks, > Laura > > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The > Linux Foundation > diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h index f0bebc5..9493f3e 100644 --- a/arch/arm64/include/asm/pgtable.h +++ b/arch/arm64/include/asm/pgtable.h @@ -167,7 +167,7 @@ static inline void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep, pte_t pte) { if (pte_valid_user(pte)) { - if (pte_exec(pte)) + if (pte_exec(pte) && pfn_valid(pte_pfn(pte))) __sync_icache_dcache(pte, addr); if (!pte_dirty(pte)) pte = pte_wrprotect(pte);