Received: by 2002:ab2:620c:0:b0:1ef:ffd0:ce49 with SMTP id o12csp1001870lqt; Tue, 19 Mar 2024 09:52:53 -0700 (PDT) X-Forwarded-Encrypted: i=3; AJvYcCVCqDeL26kt6lNGj5J+NXAM6xaPS/hK5BOth6VrHDnqz85Xo6cas/NFRofRJswwEON9A1tpHoLqjo/B5K1vd7jqfdLssSa0EewzwkR40A== X-Google-Smtp-Source: AGHT+IEhOY7Dg6AFXv+GriErcECb+t3gu93VRTGvTRiPHKjGqog6tKivyanPb5L5K/0J6N1atUD0 X-Received: by 2002:a17:906:6545:b0:a45:2e21:c78c with SMTP id u5-20020a170906654500b00a452e21c78cmr10922980ejn.13.1710867172993; Tue, 19 Mar 2024 09:52:52 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1710867172; cv=pass; d=google.com; s=arc-20160816; b=UQ+aTvrLniq82avU4oPUWhe4IJu+wFo5CdlruYi+B4rLi2RJUaqg4HEPz2EkuxnQpz lPt+ZwBitsOZMDVsxYrHZSRZ+aalk0MctEAMM8S/l59UgJpM8fW5tjpTau5w1XXlhaTD SLfQwagz+C0vdf/O0L36V4l3S4blsQz/NLxtYXokNXHZqRprPzURFBzqay9lOr33YsIu CHOQ4YKXowDJ/DW6E5KxMP3wn7t5rRTC2vvorTFjDzKgFKHxrQJpdPSJPyoW8Sa4wDk0 PqbLoUcCpsgx4diGFoOBb+/7T+GDP8VFerHLcdB6MO4BXceCxEJTrSjYfEsdDiBB8Mhp BZDA== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:date:message-id; bh=luz80OlPr9qkaxncvliZv04/iCX9t1pKjvlPPjCB8uI=; fh=aHweQPKCeY3e8a/YkoNItfcIeODR2vW66+re4hunHt0=; b=kuIl/LgPd0Llbf5/f99BehSjtiorUk4CTaJm8kB0UEebKMvTEcDDT591jQDqPftfSS sdM6qJzXvXoVI6oOMBKyr51mo+pUhbwo/LUjs7wpbz/nYhqT4gAxL4/7odYGJjI7QXDZ phVNgTQ9rsyXmPIZRu8b8Opx2rF55mK0wrqFWcDdJcQM6XKrtQxl6izuaC1APWAs/ppW BJbduMWMtuskKp730zAhd8YX/SyQfwp8E8Ij6a+DQMQViHHC5eGy4lXqKzTH+mtrjqyN 3FT/e46bOkR+zLJ4lJEDQvJQ7Hmv8Kpr3oWSLpod9dMunZ+f8zAUwqjHbUX+WsUL/4s5 oYtg==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; arc=pass (i=1 spf=pass spfdomain=ghiti.fr); spf=pass (google.com: domain of linux-kernel+bounces-107866-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) smtp.mailfrom="linux-kernel+bounces-107866-linux.lists.archive=gmail.com@vger.kernel.org" Return-Path: Received: from am.mirrors.kernel.org (am.mirrors.kernel.org. [2604:1380:4601:e00::3]) by mx.google.com with ESMTPS id kz25-20020a17090777d900b00a44438f8f24si5627074ejc.192.2024.03.19.09.52.52 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 Mar 2024 09:52:52 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel+bounces-107866-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3; Authentication-Results: mx.google.com; arc=pass (i=1 spf=pass spfdomain=ghiti.fr); spf=pass (google.com: domain of linux-kernel+bounces-107866-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) smtp.mailfrom="linux-kernel+bounces-107866-linux.lists.archive=gmail.com@vger.kernel.org" Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id 5E7791F2324F for ; Tue, 19 Mar 2024 16:52:18 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id CB9CC210FB; Tue, 19 Mar 2024 16:52:10 +0000 (UTC) Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [217.70.183.196]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 35AA0208D0 for ; Tue, 19 Mar 2024 16:52:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.70.183.196 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710867130; cv=none; b=RPgygpGYSBjOdjnxogqLdgtxAFrF9CbjqZ+4JEVFnYdawNCawUlXA9216TsfLleXLCv3yIhfCYFERrDrB+oZolciwBmUxamfr6rXFbudlaI82Iz86c2g45ljGpNA7MLrr9hy8eh8iYWr8m61tuyED90SLocHcdWFW4TE/2WBOk8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710867130; c=relaxed/simple; bh=NBq5fLIj7FPk154JFi6u2xDwt6s8Dxfvu1J7YpM4Dmg=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=nr8OG1z59UiARQ4tsfy4cav8DfD9BI3qUt7mzH+8L7REzElDT9sSMpDykKyxmYxPfFlcNt25TWnol+aHSSGIU9HyAzahl7jN0VHXt2UiNucyyFILDodyjWQZ383TKbhHFXhNtdDFBIAoHtusZe/tbBGRXEA422Enk9+xw+TLHaY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ghiti.fr; spf=pass smtp.mailfrom=ghiti.fr; arc=none smtp.client-ip=217.70.183.196 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ghiti.fr Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ghiti.fr Received: by mail.gandi.net (Postfix) with ESMTPSA id E4B1FE0004; Tue, 19 Mar 2024 16:51:54 +0000 (UTC) Message-ID: Date: Tue, 19 Mar 2024 17:51:54 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] riscv: Define TASK_SIZE_MAX for __access_ok() Content-Language: en-US To: Samuel Holland , Alexandre Ghiti Cc: Palmer Dabbelt , linux-riscv@lists.infradead.org, Albert Ou , Andrew Morton , Charlie Jenkins , Guo Ren , Jisheng Zhang , Kemeng Shi , "Matthew Wilcox (Oracle)" , "Mike Rapoport (IBM)" , Paul Walmsley , Xiao Wang , Yangyu Chen , linux-kernel@vger.kernel.org References: <20240313180010.295747-1-samuel.holland@sifive.com> <88de4a1a-047e-4be9-b5b0-3e53434dc022@sifive.com> From: Alexandre Ghiti In-Reply-To: <88de4a1a-047e-4be9-b5b0-3e53434dc022@sifive.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-GND-Sasl: alex@ghiti.fr Hi  Samuel, On 18/03/2024 22:29, Samuel Holland wrote: > Hi Alex, > > On 2024-03-18 3:50 PM, Alexandre Ghiti wrote: >> On Wed, Mar 13, 2024 at 7:00 PM Samuel Holland >> wrote: >>> TASK_SIZE_MAX should be set to the largest userspace address under any >>> runtime configuration. This optimizes the check in __access_ok(), which >>> no longer needs to compute the current value of TASK_SIZE. It is still >>> safe because addresses between TASK_SIZE and TASK_SIZE_MAX are invalid >>> at the hardware level. >>> >>> This removes about half of the references to pgtable_l[45]_enabled. >>> >>> Signed-off-by: Samuel Holland >>> --- >>> >>> arch/riscv/include/asm/pgtable-64.h | 1 + >>> arch/riscv/include/asm/pgtable.h | 1 + >>> 2 files changed, 2 insertions(+) >>> >>> diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h >>> index b99bd66107a6..a677ef3c0fe2 100644 >>> --- a/arch/riscv/include/asm/pgtable-64.h >>> +++ b/arch/riscv/include/asm/pgtable-64.h >>> @@ -17,6 +17,7 @@ extern bool pgtable_l5_enabled; >>> #define PGDIR_SHIFT_L4 39 >>> #define PGDIR_SHIFT_L5 48 >>> #define PGDIR_SIZE_L3 (_AC(1, UL) << PGDIR_SHIFT_L3) >>> +#define PGDIR_SIZE_L5 (_AC(1, UL) << PGDIR_SHIFT_L5) >>> >>> #define PGDIR_SHIFT (pgtable_l5_enabled ? PGDIR_SHIFT_L5 : \ >>> (pgtable_l4_enabled ? PGDIR_SHIFT_L4 : PGDIR_SHIFT_L3)) >>> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h >>> index 6066822e7396..2032f8ac5fc5 100644 >>> --- a/arch/riscv/include/asm/pgtable.h >>> +++ b/arch/riscv/include/asm/pgtable.h >>> @@ -867,6 +867,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte) >>> #ifdef CONFIG_64BIT >>> #define TASK_SIZE_64 (PGDIR_SIZE * PTRS_PER_PGD / 2) >>> #define TASK_SIZE_MIN (PGDIR_SIZE_L3 * PTRS_PER_PGD / 2) >>> +#define TASK_SIZE_MAX (PGDIR_SIZE_L5 * PTRS_PER_PGD / 2) >>> >>> #ifdef CONFIG_COMPAT >>> #define TASK_SIZE_32 (_AC(0x80000000, UL)) >>> -- >>> 2.43.1 >>> >> I think you also need to change the check in handle_page_fault() by >> using TASK_SIZE_MAX instead of TASK_SIZE, otherwise the fixup can't >> happen (https://elixir.bootlin.com/linux/latest/source/arch/riscv/mm/fault.c#L273). > It is not necessary to change that check in fault.c unless we expect to handle > exceptions (outside of userspace access routines) for addresses between > TASK_SIZE and TASK_SIZE_MAX. Which I think could be the case if some code is only using the "new" access_ok() without the uaccess routines (which is wrong yes, but such code is at the origin of this check). > It looks like the call to fixup_exception() [added > in 416721ff05fd ("riscv, mm: Perform BPF exhandler fixup on page fault")] is > only intended to catch null pointer dereferences. So making the change wouldn't > have any functional impact, but it would still be a valid optimization. > >> Or I was wondering if it would not be better to do like x86 and use an >> alternative, it would be more correct (even though I believe your >> solution works) >> https://elixir.bootlin.com/linux/latest/source/arch/x86/include/asm/page_64.h#L82. > What would be the benefit of using an alternative? Any access to an address > between TASK_SIZE and TASK_SIZE_MAX is guaranteed to generate a page fault, so > the only benefit I see is returning -EFAULT slightly faster at the cost of > applying a few hundred alternatives at boot. But it's possible I'm missing > something. The use of alternatives allows to return right away if the buffer is beyond the usable user address space, and it's not just "slightly faster" for some cases (a very large buffer with only a few bytes being beyond the limit or someone could fault-in all the user pages and fail very late...etc). access_ok() is here to guarantee that such situations don't happen, so actually it makes more sense to use an alternative to avoid that. Alex > Regards, > Samuel > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv