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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id A7EB9C47DB3 for ; Mon, 29 Jan 2024 17:43:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 342876B007B; Mon, 29 Jan 2024 12:43:56 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2F2506B007E; Mon, 29 Jan 2024 12:43:56 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1BACC6B0081; Mon, 29 Jan 2024 12:43:56 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 0CFBC6B007B for ; Mon, 29 Jan 2024 12:43:56 -0500 (EST) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 94C5340AA8 for ; Mon, 29 Jan 2024 17:43:55 +0000 (UTC) X-FDA: 81733071630.16.6CB921D Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf27.hostedemail.com (Postfix) with ESMTP id B39BE40016 for ; Mon, 29 Jan 2024 17:43:53 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=none; spf=pass (imf27.hostedemail.com: domain of mark.rutland@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=mark.rutland@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706550234; a=rsa-sha256; cv=none; b=K/zSS6IIhjryKFfjuqycc3tSANVBvlg47us2zJ3GnIUWU8hKtoYjuYPGIBp9dCe2Bt3hqR IgpCwxJSJs9Sy/3RWj2RTds5ernHuO/8j0ea2c0lTqzV072C/e0rbg3mXN69UBAJMWXqZ+ NzH0TTOOgvgIlnhhr/sZGKz9VUwaTx0= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=none; spf=pass (imf27.hostedemail.com: domain of mark.rutland@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=mark.rutland@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1706550234; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=SWYBLvZfUt2cLhO8iTZgbrxGUftGYF8K8yBDbQbJxm4=; b=gQIbXB3HpD8aVj6ptvVhZ42r8EmUriydFslYGgAKTuVOJuC8dgIuKPPaSMQ+PVV5EliaHu T6nD/Kl6edJCPsi00n8V/3HtgaPZsvDz0Any/MZONq4fuIH4fNyBi0L6W5Mdb2ingaoxKz YwSt0vbjEnlZ/FfkE2e+IJhhU26Ij4M= Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 719E5DA7; Mon, 29 Jan 2024 09:44:36 -0800 (PST) Received: from FVFF77S0Q05N (unknown [10.57.48.128]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6A2B13F738; Mon, 29 Jan 2024 09:43:48 -0800 (PST) Date: Mon, 29 Jan 2024 17:43:24 +0000 From: Mark Rutland To: Tong Tiangen Cc: Catalin Marinas , Will Deacon , James Morse , Robin Murphy , Andrey Ryabinin , Alexander Potapenko , Alexander Viro , Andrey Konovalov , Dmitry Vyukov , Vincenzo Frascino , Andrew Morton , Michael Ellerman , Nicholas Piggin , Christophe Leroy , "Aneesh Kumar K.V" , "Naveen N. Rao" , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, "H. Peter Anvin" , linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com, wangkefeng.wang@huawei.com, Guohanjun Subject: Re: [PATCH v10 3/6] arm64: add uaccess to machine check safe Message-ID: References: <20240129134652.4004931-1-tongtiangen@huawei.com> <20240129134652.4004931-4-tongtiangen@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240129134652.4004931-4-tongtiangen@huawei.com> X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: B39BE40016 X-Stat-Signature: hztc1pprir34jnzijc71htiaxr66zh48 X-HE-Tag: 1706550233-348480 X-HE-Meta: U2FsdGVkX1/YvzRJsYN7H9LIXrNgny0oa54vNyffAepNM7B9EacU/PUCrMelTrBDReln+nBnGGB6MH0lhpczh1TZnCjaTTBrjBJX7CVSF7GaFgMZT3YM0o84UQgas0n1jadli0fbdP1jiZqQ74cqEaeQYNOVCZpRpcWZNIGADgbfTT31oaRc0DF6XRKLX7E5tZFgFnLTNs7nKFK24Dl+CbnyiReuotcnDitkCUA263D2trfWi4OIk6fv4bHClo8KZjBMcdRrhNi9/KOhbYUIpluu46huz1lyxBWo4Bj0NCZX4Ck0hvweU0npniLMTmr1qnzs+CIXObMFXYpp8WNG4JQKefW55tRa6CsC9ymq80/uf7T1uNO34h+09FH3EUWN0Fo0C1Wmsx1LusbaJXfBvFXr107qkzgZpGK4t1hyO/PKY4PkFAvVc5bReQr2MxuZqbmdG/FO01nsPIQoOVW+wsX0Eh33IKfMNB/yP1tQmeEZBhUxqY3sPpbwxt+STEgXwJTTuK8PRWqa5ygOQMzF5Rx4EBrEup0FF42+mqg87Y9Nw1bWVDrvR/wnTiM/7hrHrSE4bLMiTUHVayO7Ovmpz2MEaW2309q9ow3AGFx1kvUW0f0AyeP0fKV8TG2EZnspL5UzyH/Dyy7w9igWn9b0fnP7T8/hANKpYeum8I21an6v1IYCvbj59Px2Rrx6kbMTcEqZ/fGcmnzCm/mVhdIllGXaXtwVB/8z8bvcvmy7OihVIMeg9AI4p94aVDvsXdzRWfBHGzBAWbKxx1++fRBWdBOwQKZpYpdD6PoWvKgettq3FXdSkWCCy45bjF/nO3zgPOyXMt02Ae5lOyyHO0LvrzrUJ84S1NO8S04AOPbTROIPBZ2990MtzrK5Dr3l9+9fCxkozbj8zeaXZhycRRONLs0qt9/teYSMDPlegJQSedxKeUGKPcwss0K+UibgH73rwOE2zc5laAHeFeFOYhx EbjasSnZ Cdhror3XFyTmVLXRRvJCuEYQwh4/BDRHi5gr9PyNz42nN/iyHAMSNqJhfHbh99/Ci+YHAxDDSxaUAdB+Ds6ZTeNITSqrPA5uKdK1PZx6boPs8SGi9oZwGht828YUCk06K826bM1DXLB/eTCzUClaNvjQ5MmWL24M3pHjRYQYAC8/3KZNymC5dEJJmTiBjHvnD1ScFjpiQvwo/Whs= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Mon, Jan 29, 2024 at 09:46:49PM +0800, Tong Tiangen wrote: > If user process access memory fails due to hardware memory error, only the > relevant processes are affected, so it is more reasonable to kill the user > process and isolate the corrupt page than to panic the kernel. > > Signed-off-by: Tong Tiangen > --- > arch/arm64/lib/copy_from_user.S | 10 +++++----- > arch/arm64/lib/copy_to_user.S | 10 +++++----- > arch/arm64/mm/extable.c | 8 ++++---- > 3 files changed, 14 insertions(+), 14 deletions(-) > > diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S > index 34e317907524..1bf676e9201d 100644 > --- a/arch/arm64/lib/copy_from_user.S > +++ b/arch/arm64/lib/copy_from_user.S > @@ -25,7 +25,7 @@ > .endm > > .macro strb1 reg, ptr, val > - strb \reg, [\ptr], \val > + USER(9998f, strb \reg, [\ptr], \val) > .endm This is a store to *kernel* memory, not user memory. It should not be marked with USER(). I understand that you *might* want to handle memory errors on these stores, but the commit message doesn't describe that and the associated trade-off. For example, consider that when a copy_form_user fails we'll try to zero the remaining buffer via memset(); so if a STR* instruction in copy_to_user faulted, upon handling the fault we'll immediately try to fix that up with some more stores which will also fault, but won't get fixed up, leading to a panic() anyway... Further, this change will also silently fixup unexpected kernel faults if we pass bad kernel pointers to copy_{to,from}_user, which will hide real bugs. So NAK to this change as-is; likewise for the addition of USER() to other ldr* macros in copy_from_user.S and the addition of USER() str* macros in copy_to_user.S. If we want to handle memory errors on some kaccesses, we need a new EX_TYPE_* separate from the usual EX_TYPE_KACESS_ERR_ZERO that means "handle memory errors, but treat other faults as fatal". That should come with a rationale and explanation of why it's actually useful. [...] > diff --git a/arch/arm64/mm/extable.c b/arch/arm64/mm/extable.c > index 478e639f8680..28ec35e3d210 100644 > --- a/arch/arm64/mm/extable.c > +++ b/arch/arm64/mm/extable.c > @@ -85,10 +85,10 @@ bool fixup_exception_mc(struct pt_regs *regs) > if (!ex) > return false; > > - /* > - * This is not complete, More Machine check safe extable type can > - * be processed here. > - */ > + switch (ex->type) { > + case EX_TYPE_UACCESS_ERR_ZERO: > + return ex_handler_uaccess_err_zero(ex, regs); > + } Please fold this part into the prior patch, and start ogf with *only* handling errors on accesses already marked with EX_TYPE_UACCESS_ERR_ZERO. I think that change would be relatively uncontroversial, and it would be much easier to build atop that. Thanks, Mark.