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=-6.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED 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 8B0FFC433DB for ; Tue, 16 Feb 2021 07:37:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 324A364E04 for ; Tue, 16 Feb 2021 07:37:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229872AbhBPHh1 (ORCPT ); Tue, 16 Feb 2021 02:37:27 -0500 Received: from mail.kernel.org ([198.145.29.99]:33376 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229713AbhBPHhW (ORCPT ); Tue, 16 Feb 2021 02:37:22 -0500 Received: by mail.kernel.org (Postfix) with ESMTPSA id 3975264DC3 for ; Tue, 16 Feb 2021 07:36:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1613461001; bh=HcYqW8/qRuo3kSh4HBmRqKKVZrvA9wI0nKwnFND/7Es=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=eb7eAeA3BV05Wb/Z+2UfjpMtlj/qV/Ly3mxPD/v9iQrr+56N6vWTTK+Mv6FrtKpOJ eBPTb/sZbX9XXpGCRPn87lcpoYJdtQtcHRrEl05HEnOGRI99VAt8JvjISlDOdJ/gVO Kau7+gfGtwjnsLbZTadj+vkTTM47dKANWYZqqqyV4xKItQEnJnieTkpRYLeUuh+dcY qW7Xj0vY7Rw/8wV6nN6G+QSEDCg9aDz6Re2Cnse/EJUzhvVKLKU5H5g5wipWcqXD6M JYFN9N/PklVelRL4mwNI5XJC7gmMe5sVgbFhirR0zDw2p7Wd1CKTxvXd/xgsP+1qiH cI4qUKs00V7zA== Received: by mail-oi1-f177.google.com with SMTP id h17so6417065oih.5 for ; Mon, 15 Feb 2021 23:36:41 -0800 (PST) X-Gm-Message-State: AOAM532DZR0zMn9Gb2mLVgq8hUUDkywPW8Xtx+ajDhFe3DUhK67rAlD8 71brLNVVfnV47+i7NXpIc450TsNhgYD+LvJKZr4= X-Google-Smtp-Source: ABdhPJxGsfTmY4tueuaxx9cadKNbKIICvQuus4c+2Y+WGUaRCs7tMad57+GGr17En0d7WAgajt08A5qc1qvv4euvVKs= X-Received: by 2002:aca:307:: with SMTP id 7mr1730193oid.174.1613461000391; Mon, 15 Feb 2021 23:36:40 -0800 (PST) MIME-Version: 1.0 References: <20210215192237.362706-1-pasha.tatashin@soleen.com> <20210215192237.362706-2-pasha.tatashin@soleen.com> <1790afff-eebd-1eda-a1b4-0062908f1f32@arm.com> In-Reply-To: <1790afff-eebd-1eda-a1b4-0062908f1f32@arm.com> From: Ard Biesheuvel Date: Tue, 16 Feb 2021 08:36:29 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 1/1] arm64: mm: correct the inside linear map boundaries during hotplug check To: Anshuman Khandual Cc: Pavel Tatashin , Tyler Hicks , James Morris , Catalin Marinas , Will Deacon , Andrew Morton , Mike Rapoport , Logan Gunthorpe , Linux ARM , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 16 Feb 2021 at 04:12, Anshuman Khandual wrote: > > > > On 2/16/21 1:21 AM, Pavel Tatashin wrote: > > On Mon, Feb 15, 2021 at 2:34 PM Ard Biesheuvel wrote: > >> > >> On Mon, 15 Feb 2021 at 20:30, Pavel Tatashin wrote: > >>> > >>>> Can't we simply use signed arithmetic here? This expression works fine > >>>> if the quantities are all interpreted as s64 instead of u64 > >>> > >>> I was thinking about that, but I do not like the idea of using sign > >>> arithmetics for physical addresses. Also, I am worried that someone in > >>> the future will unknowingly change it to unsigns or to phys_addr_t. It > >>> is safer to have start explicitly set to 0 in case of wrap. > >> > >> memstart_addr is already a s64 for this exact reason. > > > > memstart_addr is basically an offset and it must be negative. For > > example, this would not work if it was not signed: > > #define vmemmap ((struct page *)VMEMMAP_START - (memstart_addr >> PAGE_SHIFT)) > > > > However, on powerpc it is phys_addr_t type. > > > >> > >> Btw, the KASLR check is incorrect: memstart_addr could also be > >> negative when running the 52-bit VA kernel on hardware that is only > >> 48-bit VA capable. > > > > Good point! > > > > if (IS_ENABLED(CONFIG_ARM64_VA_BITS_52) && (vabits_actual != 52)) > > memstart_addr -= _PAGE_OFFSET(48) - _PAGE_OFFSET(52); > > > > So, I will remove IS_ENABLED(CONFIG_RANDOMIZE_BASE) again. > > > > I am OK to change start_linear_pa, end_linear_pa to signed, but IMO > > what I have now is actually safer to make sure that does not break > > again in the future. > An explicit check for the flip over and providing two different start > addresses points would be required in order to use the new framework. I don't think so. We no longer randomize over the same range, but take the support PA range into account. (97d6786e0669d) This should ensure that __pa(_PAGE_OFFSET(vabits_actual)) never assumes a negative value. And to Pavel's point re 48/52 bit VAs: the fact that vabits_actual appears in this expression means that it already takes this into account, so you are correct that we don't have to care about that here. So even if memstart_addr could be negative, this expression should never produce a negative value. And with the patch above applied, it should never do so when running under KASLR either. So question to Pavel and Tyler: could you please check whether you have that patch, and whether it fixes the issue? It was introduced in v5.11, and hasn't been backported yet (it wasn't marked for -stable) 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=-4.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED 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 58891C433E0 for ; Tue, 16 Feb 2021 07:42: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 E17AD64DDA for ; Tue, 16 Feb 2021 07:42:15 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E17AD64DDA 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=RSGnUPYcXy8iPtbxOK5ibwEQ8nxbC32d+XH11PrOea8=; b=eYpzcdb2SihAR4kcTEdoYjOyl pwE+3CCs21t3hab4TCOwcNtuMv2YgNhEbMfw3rnDPMGr8uqZ6IUe/5DS5+IERKUdLhj1ytGpZK2TD rTq3nR7B8jkw6e/jhZSfIAMapcAo1sakI4WilhulcNpkhbwo7oc8pdPLtXjidjKQ7LUgXkNY2909N JFH/aoSyf9i9e+noVD+TZan5H3DMDqGlYrXpzckBljGM9al9S8wI8+gBoSOt4T5wnVwNSdPt+80KE 8RP8lSwB8RKyzwUa7amVo32tiGPqPHV/t759YRJLG6sGBIPEA26VZoP9qdAn3DdTY10Fy38mVBIdh quBS1Wznw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1lBuuc-0001km-HG; Tue, 16 Feb 2021 07:36:46 +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 1lBuuZ-0001kH-9d for linux-arm-kernel@lists.infradead.org; Tue, 16 Feb 2021 07:36:44 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id 4359564E10 for ; Tue, 16 Feb 2021 07:36:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1613461001; bh=HcYqW8/qRuo3kSh4HBmRqKKVZrvA9wI0nKwnFND/7Es=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=eb7eAeA3BV05Wb/Z+2UfjpMtlj/qV/Ly3mxPD/v9iQrr+56N6vWTTK+Mv6FrtKpOJ eBPTb/sZbX9XXpGCRPn87lcpoYJdtQtcHRrEl05HEnOGRI99VAt8JvjISlDOdJ/gVO Kau7+gfGtwjnsLbZTadj+vkTTM47dKANWYZqqqyV4xKItQEnJnieTkpRYLeUuh+dcY qW7Xj0vY7Rw/8wV6nN6G+QSEDCg9aDz6Re2Cnse/EJUzhvVKLKU5H5g5wipWcqXD6M JYFN9N/PklVelRL4mwNI5XJC7gmMe5sVgbFhirR0zDw2p7Wd1CKTxvXd/xgsP+1qiH cI4qUKs00V7zA== Received: by mail-oi1-f181.google.com with SMTP id v193so10316473oie.8 for ; Mon, 15 Feb 2021 23:36:41 -0800 (PST) X-Gm-Message-State: AOAM531DB+XieeSEe1P9o8HtL/DflnepKtzJw4/8m9kWs6Ag1i6yLdps 0MoDilyz/kgpyK/HwvOr2i45FNwAHnrxjhrES74= X-Google-Smtp-Source: ABdhPJxGsfTmY4tueuaxx9cadKNbKIICvQuus4c+2Y+WGUaRCs7tMad57+GGr17En0d7WAgajt08A5qc1qvv4euvVKs= X-Received: by 2002:aca:307:: with SMTP id 7mr1730193oid.174.1613461000391; Mon, 15 Feb 2021 23:36:40 -0800 (PST) MIME-Version: 1.0 References: <20210215192237.362706-1-pasha.tatashin@soleen.com> <20210215192237.362706-2-pasha.tatashin@soleen.com> <1790afff-eebd-1eda-a1b4-0062908f1f32@arm.com> In-Reply-To: <1790afff-eebd-1eda-a1b4-0062908f1f32@arm.com> From: Ard Biesheuvel Date: Tue, 16 Feb 2021 08:36:29 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 1/1] arm64: mm: correct the inside linear map boundaries during hotplug check To: Anshuman Khandual X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210216_023643_557327_CD635EFC X-CRM114-Status: GOOD ( 27.15 ) 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: Pavel Tatashin , Catalin Marinas , James Morris , Linux Kernel Mailing List , Logan Gunthorpe , Tyler Hicks , Linux ARM , Andrew Morton , Will Deacon , Mike Rapoport 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, 16 Feb 2021 at 04:12, Anshuman Khandual wrote: > > > > On 2/16/21 1:21 AM, Pavel Tatashin wrote: > > On Mon, Feb 15, 2021 at 2:34 PM Ard Biesheuvel wrote: > >> > >> On Mon, 15 Feb 2021 at 20:30, Pavel Tatashin wrote: > >>> > >>>> Can't we simply use signed arithmetic here? This expression works fine > >>>> if the quantities are all interpreted as s64 instead of u64 > >>> > >>> I was thinking about that, but I do not like the idea of using sign > >>> arithmetics for physical addresses. Also, I am worried that someone in > >>> the future will unknowingly change it to unsigns or to phys_addr_t. It > >>> is safer to have start explicitly set to 0 in case of wrap. > >> > >> memstart_addr is already a s64 for this exact reason. > > > > memstart_addr is basically an offset and it must be negative. For > > example, this would not work if it was not signed: > > #define vmemmap ((struct page *)VMEMMAP_START - (memstart_addr >> PAGE_SHIFT)) > > > > However, on powerpc it is phys_addr_t type. > > > >> > >> Btw, the KASLR check is incorrect: memstart_addr could also be > >> negative when running the 52-bit VA kernel on hardware that is only > >> 48-bit VA capable. > > > > Good point! > > > > if (IS_ENABLED(CONFIG_ARM64_VA_BITS_52) && (vabits_actual != 52)) > > memstart_addr -= _PAGE_OFFSET(48) - _PAGE_OFFSET(52); > > > > So, I will remove IS_ENABLED(CONFIG_RANDOMIZE_BASE) again. > > > > I am OK to change start_linear_pa, end_linear_pa to signed, but IMO > > what I have now is actually safer to make sure that does not break > > again in the future. > An explicit check for the flip over and providing two different start > addresses points would be required in order to use the new framework. I don't think so. We no longer randomize over the same range, but take the support PA range into account. (97d6786e0669d) This should ensure that __pa(_PAGE_OFFSET(vabits_actual)) never assumes a negative value. And to Pavel's point re 48/52 bit VAs: the fact that vabits_actual appears in this expression means that it already takes this into account, so you are correct that we don't have to care about that here. So even if memstart_addr could be negative, this expression should never produce a negative value. And with the patch above applied, it should never do so when running under KASLR either. So question to Pavel and Tyler: could you please check whether you have that patch, and whether it fixes the issue? It was introduced in v5.11, and hasn't been backported yet (it wasn't marked for -stable) _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel