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=-10.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable 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 117A3C4727E for ; Wed, 30 Sep 2020 11:03:52 +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 9BAAE2071E for ; Wed, 30 Sep 2020 11:03:51 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="DrJNCxF5"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="Xw8228Uz" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9BAAE2071E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.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:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=SGa3PDOyw4Z+zhxWKRAj6+lO5/mqGrlZPHXh3alTwlo=; b=DrJNCxF5/9JAiMAEP6tQwSiLW A63jrfUEvj2wa1rUOsGVx+Qxly1Eo6qLQFDDSLLOYcL4BrBv4PQW3F3sL3005XjWU71mYw3ZrJE6X 0/gk6SBR2qZew+lhUTBzPY/8OR+YFs9yGSRrosji3bCX7Yg854jr1cqZClvbf7inlt86SnY3xrKyO wZ7eubVUay4dcxF2RqoL9O+Hoptd19Z3Qxf64EGAAq9KH1w477hN1AM8xOpwRAfmgcpetTBgwPoRb l9U72Be0/oKBg+IDNbE69sbEk4nfuu0Q23pGXVSYFiYD8Duo5Jvjxz5ywgcrBaylUJx+7GeiXC5bN VfWOdRmZw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kNZsC-0008DA-14; Wed, 30 Sep 2020 11:02:12 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kNZs9-0008CI-5m for linux-arm-kernel@lists.infradead.org; Wed, 30 Sep 2020 11:02:10 +0000 Received: from mail-ot1-f41.google.com (mail-ot1-f41.google.com [209.85.210.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 69834207C3 for ; Wed, 30 Sep 2020 11:02:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1601463728; bh=nvVPSoKRhObWOZ2KtSKQHeSXtoGqWb/uiSd2fSRne8c=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=Xw8228Uz9QHJ9x9s3AzjPWXHcL3NDq0UqXFaJe5gAFS8HvE9cJRRHooxsAtcVmBK6 3IVtOshBRezgmj8s/tcZB31mRkJr5ykAlk3OllA26r3hHkEmnPQ231vRPo/yO1z36n AsI2i63VNHckQDZy/qZsvF7bc9bG7bIfO05PLwtc= Received: by mail-ot1-f41.google.com with SMTP id h17so1399615otr.1 for ; Wed, 30 Sep 2020 04:02:08 -0700 (PDT) X-Gm-Message-State: AOAM53207x3cU+5FrlHOfLpT1qiUM1s97mv6wPQoFYvKWybu9PBm2ivk SRw6Md79zCWUM0yLvAtDAGHZjNfzYMZ2Bk/pAZU= X-Google-Smtp-Source: ABdhPJyyYn7NkxZU/TLSnwDtnjsRQlSSUkbd55I5RxAogRYHH6tvYKc/dFNmVM+ivoll/kJfNr1yX350lhgV4TYDxa8= X-Received: by 2002:a9d:6250:: with SMTP id i16mr1275655otk.77.1601463727306; Wed, 30 Sep 2020 04:02:07 -0700 (PDT) MIME-Version: 1.0 References: <1600332402-30123-1-git-send-email-anshuman.khandual@arm.com> <20200928203539.GA12218@willie-the-truck> <09266aed-7eef-5b16-5d52-0dcb7dcb7246@arm.com> <20200929152221.GA13995@willie-the-truck> In-Reply-To: From: Ard Biesheuvel Date: Wed, 30 Sep 2020 13:01:56 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] arm64/mm: Validate hotplug range before creating linear mapping To: Anshuman Khandual X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200930_070209_370211_D5816A7C X-CRM114-Status: GOOD ( 42.90 ) 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: Mark Rutland , Linux Kernel Mailing List , David Hildenbrand , Catalin Marinas , Robin Murphy , Steven Price , Andrew Morton , Will Deacon , Linux ARM 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 Wed, 30 Sep 2020 at 10:03, Anshuman Khandual wrote: > > > On 09/29/2020 08:52 PM, Will Deacon wrote: > > On Tue, Sep 29, 2020 at 01:34:24PM +0530, Anshuman Khandual wrote: > >> > >> > >> On 09/29/2020 02:05 AM, Will Deacon wrote: > >>> On Thu, Sep 17, 2020 at 02:16:42PM +0530, Anshuman Khandual wrote: > >>>> During memory hotplug process, the linear mapping should not be created for > >>>> a given memory range if that would fall outside the maximum allowed linear > >>>> range. Else it might cause memory corruption in the kernel virtual space. > >>>> > >>>> Maximum linear mapping region is [PAGE_OFFSET..(PAGE_END -1)] accommodating > >>>> both its ends but excluding PAGE_END. Max physical range that can be mapped > >>>> inside this linear mapping range, must also be derived from its end points. > >>>> > >>>> When CONFIG_ARM64_VA_BITS_52 is enabled, PAGE_OFFSET is computed with the > >>>> assumption of 52 bits virtual address space. However, if the CPU does not > >>>> support 52 bits, then it falls back using 48 bits instead and the PAGE_END > >>>> is updated to reflect this using the vabits_actual. As for PAGE_OFFSET, > >>>> bits [51..48] are ignored by the MMU and remain unchanged, even though the > >>>> effective start address of linear map is now slightly different. Hence, to > >>>> reliably check the physical address range mapped by the linear map, the > >>>> start address should be calculated using vabits_actual. This ensures that > >>>> arch_add_memory() validates memory hot add range for its potential linear > >>>> mapping requirement, before creating it with __create_pgd_mapping(). > >>>> > >>>> Cc: Catalin Marinas > >>>> Cc: Will Deacon > >>>> Cc: Mark Rutland > >>>> Cc: Ard Biesheuvel > >>>> Cc: Steven Price > >>>> Cc: Robin Murphy > >>>> Cc: David Hildenbrand > >>>> Cc: Andrew Morton > >>>> Cc: linux-arm-kernel@lists.infradead.org > >>>> Cc: linux-kernel@vger.kernel.org > >>>> Fixes: 4ab215061554 ("arm64: Add memory hotplug support") > >>>> Signed-off-by: Anshuman Khandual > >>>> --- > >>>> arch/arm64/mm/mmu.c | 27 +++++++++++++++++++++++++++ > >>>> 1 file changed, 27 insertions(+) > >>>> > >>>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c > >>>> index 75df62fea1b6..d59ffabb9c84 100644 > >>>> --- a/arch/arm64/mm/mmu.c > >>>> +++ b/arch/arm64/mm/mmu.c > >>>> @@ -1433,11 +1433,38 @@ static void __remove_pgd_mapping(pgd_t *pgdir, unsigned long start, u64 size) > >>>> free_empty_tables(start, end, PAGE_OFFSET, PAGE_END); > >>>> } > >>>> > >>>> +static bool inside_linear_region(u64 start, u64 size) > >>>> +{ > >>>> + /* > >>>> + * Linear mapping region is the range [PAGE_OFFSET..(PAGE_END - 1)] > >>>> + * accommodating both its ends but excluding PAGE_END. Max physical > >>>> + * range which can be mapped inside this linear mapping range, must > >>>> + * also be derived from its end points. > >>>> + * > >>>> + * With CONFIG_ARM64_VA_BITS_52 enabled, PAGE_OFFSET is defined with > >>>> + * the assumption of 52 bits virtual address space. However, if the > >>>> + * CPU does not support 52 bits, it falls back using 48 bits and the > >>>> + * PAGE_END is updated to reflect this using the vabits_actual. As > >>>> + * for PAGE_OFFSET, bits [51..48] are ignored by the MMU and remain > >>>> + * unchanged, even though the effective start address of linear map > >>>> + * is now slightly different. Hence, to reliably check the physical > >>>> + * address range mapped by the linear map, the start address should > >>>> + * be calculated using vabits_actual. > >>>> + */ > >>>> + return ((start >= __pa(_PAGE_OFFSET(vabits_actual))) > >>>> + && ((start + size) <= __pa(PAGE_END - 1))); > >>>> +} > >>> > >>> Why isn't this implemented using the existing __is_lm_address()? > >> > >> Not sure, if I understood your suggestion here. The physical address range > >> [start..start + size] needs to be checked against maximum physical range > >> that can be represented inside effective boundaries for the linear mapping > >> i.e [__pa(_PAGE_OFFSET(vabits_actual)..__pa(PAGE_END - 1)]. > >> > >> Are you suggesting [start..start + size] should be first be converted into > >> a virtual address range and then checked against __is_lm_addresses() ? But > >> is not deriving the physical range from from know limits of linear mapping > >> much cleaner ? > > > > I just think having a function called "inside_linear_region()" as well as a > > macro called "__is_lm_address()" is weird when they have completely separate > > implementations. They're obviously trying to do the same thing, just the > > first one gets given physical address as parameters. > > > > Implementing one in terms of the other is much better for maintenance. > > /* > * The linear kernel range starts at the bottom of the virtual address > * space. Testing the top bit for the start of the region is a > * sufficient check and avoids having to worry about the tag. > */ > #define __is_lm_address(addr) (!(((u64)addr) & BIT(vabits_actual - 1))) > > __is_lm_address() currently just check the highest bit in a virtual address > where the linear mapping ends i.e the lower half and all other kernel mapping > starts i.e the upper half. But I would believe, it misses the blind range > [_PAGE_OFFSET(VA_BITS).._PAGE_OFFSET(vabits_actual)] in some configurations, > even though it does not really affect anything because it gets ignored by the > MMU. Hence in current form __is_lm_address() cannot be used to derive maximum > linear range and it's corresponding physical range for hotplug range check. > This is actually something that I brought up when the 52-bit VA changes were under review: currently, we split the VA space in half, and use the lower half for the linear range, and the upper half for vmalloc, kernel, modules, vmemmap etc When we run a 48-bit kernel on 52-bit capable hardware, we mostly stick with the same arrangement: 2^51 bytes of linear range, but only 2^47 bytes of vmalloc range (as the size is fixed), and so we are wasting 15/16 of the vmalloc region (2^51 - 2^47), by not using it to map anything. So it would be better to get away from this notion that the VA space is divided evenly between linear and vmalloc regions, and use the entire range between ~0 << 52 and ~0 << 47 for the linear region (Note that the KASAN region defimnition will overlap the linear region in this case, by we should be able to sort that at runtime) -- Ard. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel