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 lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 94823C46CD2 for ; Tue, 30 Jan 2024 12:02:42 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4TPP1Y1xdnz3cYn for ; Tue, 30 Jan 2024 23:02:41 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=arm.com (client-ip=217.140.110.172; helo=foss.arm.com; envelope-from=mark.rutland@arm.com; receiver=lists.ozlabs.org) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lists.ozlabs.org (Postfix) with ESMTP id 4TPP0z4Kkdz3bqQ for ; Tue, 30 Jan 2024 23:02:08 +1100 (AEDT) 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 3DDFDDA7; Tue, 30 Jan 2024 04:02:19 -0800 (PST) Received: from FVFF77S0Q05N (unknown [10.57.48.92]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 57FCA3F762; Tue, 30 Jan 2024 04:01:31 -0800 (PST) Date: Tue, 30 Jan 2024 12:01:26 +0000 From: Mark Rutland To: Tong Tiangen 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> <23795738-b86e-7709-bc2b-5abba2e77b68@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <23795738-b86e-7709-bc2b-5abba2e77b68@huawei.com> X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: wangkefeng.wang@huawei.com, Catalin Marinas , Dave Hansen , linux-mm@kvack.org, Andrey Ryabinin , Alexander Potapenko , kasan-dev@googlegroups.com, "H. Peter Anvin" , Vincenzo Frascino , Will Deacon , x86@kernel.org, "Aneesh Kumar K.V" , Ingo Molnar , linux-arm-kernel@lists.infradead.org, "Naveen N. Rao" , Nicholas Piggin , Borislav Petkov , Alexander Viro , Thomas Gleixner , Dmitry Vyukov , Andrey Konovalov , linuxppc-dev@lists.ozlabs.org, Guohanjun , linux-kernel@vger.kernel.org, James Morse , Andrew Morton , Robin Murphy Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Tue, Jan 30, 2024 at 07:14:35PM +0800, Tong Tiangen wrote: > 在 2024/1/30 1:43, Mark Rutland 写道: > > On Mon, Jan 29, 2024 at 09:46:49PM +0800, Tong Tiangen wrote: > > 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. > > I think this is better than the panic kernel, because the real bugs > belongs to the user process. Even if the wrong pointer is > transferred, the page corresponding to the wrong pointer has a memroy > error. I think you have misunderstood my point; I'm talking about the case of a bad kernel pointer *without* a memory error. For example, consider some buggy code such as: void __user *uptr = some_valid_user_pointer; void *kptr = NULL; // or any other bad pointer ret = copy_to_user(uptr, kptr, size); if (ret) return -EFAULT; Before this patch, when copy_to_user() attempted to load from NULL it would fault, there would be no fixup handler for the LDR, and the kernel would die(), reporting the bad kernel access. After this patch (which adds fixup handlers to all the LDR*s in copy_to_user()), the fault (which is *not* a memory error) would be handled by the fixup handler, and copy_to_user() would return an error without *any* indication of the horrible kernel bug. This will hide kernel bugs, which will make those harder to identify and fix, and will also potentially make it easier to exploit the kernel: if the user somehow gains control of the kernel pointer, they can rely on the fixup handler returning an error, and can scan through memory rather than dying as soon as they pas a bad pointer. > In addition, the panic information contains necessary information > for users to check. There is no panic() in the case I am describing. > > 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. > > This makes sense. Add kaccess types that can be processed properly. > > > > > [...] > > > > > 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. > > OK, the two patches will be merged in the next release. Thanks. Mark. 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 6990AC46CD2 for ; Tue, 30 Jan 2024 12:01:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=QsnKHiB8VZsPlhnZBjJizNXGreBh40iS9grxYOxQits=; b=HKCMXtBsm3O+2+ kO3rS8yLyAYI5DcIL0X2JpqMlDG+eIS16W/KcfqEmTrqlZZCql6VgXicQJtaBh47qbzN4hEwO6v0D 7gbMOJc13vocvRNxcWuwppAwpVCaxeahVUw/nA0OZjIkg8hITZemDHrMrLu5Qj2vhA198qcSEpr/E b/lOmQPdSuHqL8oPgEiItnkRqG6OKrI1SrLLR+6Vaup5juvT9OMZx/Ki3Cq3hxhR/QFZVi7DpI6Sq Ary4tVU1pbjOyXszqlU8Tthti2MRra/KPvExjGitluYWhWypC5izms+rYXWtKmx1KfodfHBhv9H2f SsqVuTOhtmGq5Vq0MShw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rUmo8-0000000Galf-0RiZ; Tue, 30 Jan 2024 12:01:40 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rUmo5-0000000GakP-3ip5 for linux-arm-kernel@lists.infradead.org; Tue, 30 Jan 2024 12:01:39 +0000 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 3DDFDDA7; Tue, 30 Jan 2024 04:02:19 -0800 (PST) Received: from FVFF77S0Q05N (unknown [10.57.48.92]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 57FCA3F762; Tue, 30 Jan 2024 04:01:31 -0800 (PST) Date: Tue, 30 Jan 2024 12:01:26 +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> <23795738-b86e-7709-bc2b-5abba2e77b68@huawei.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <23795738-b86e-7709-bc2b-5abba2e77b68@huawei.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240130_040138_041827_5DC2104D X-CRM114-Status: GOOD ( 32.28 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org T24gVHVlLCBKYW4gMzAsIDIwMjQgYXQgMDc6MTQ6MzVQTSArMDgwMCwgVG9uZyBUaWFuZ2VuIHdy b3RlOgo+IOWcqCAyMDI0LzEvMzAgMTo0MywgTWFyayBSdXRsYW5kIOWGmemBkzoKPiA+IE9uIE1v biwgSmFuIDI5LCAyMDI0IGF0IDA5OjQ2OjQ5UE0gKzA4MDAsIFRvbmcgVGlhbmdlbiB3cm90ZToK PiA+IEZ1cnRoZXIsIHRoaXMgY2hhbmdlIHdpbGwgYWxzbyBzaWxlbnRseSBmaXh1cCB1bmV4cGVj dGVkIGtlcm5lbCBmYXVsdHMgaWYgd2UKPiA+IHBhc3MgYmFkIGtlcm5lbCBwb2ludGVycyB0byBj b3B5X3t0byxmcm9tfV91c2VyLCB3aGljaCB3aWxsIGhpZGUgcmVhbCBidWdzLgo+IAo+IEkgdGhp bmsgdGhpcyBpcyBiZXR0ZXIgdGhhbiB0aGUgcGFuaWMga2VybmVsLCBiZWNhdXNlIHRoZSByZWFs IGJ1Z3MKPiBiZWxvbmdzIHRvIHRoZSB1c2VyIHByb2Nlc3MuIEV2ZW4gaWYgdGhlIHdyb25nIHBv aW50ZXIgaXMKPiB0cmFuc2ZlcnJlZCwgdGhlIHBhZ2UgY29ycmVzcG9uZGluZyB0byB0aGUgd3Jv bmcgcG9pbnRlciBoYXMgYSBtZW1yb3kKPiBlcnJvci4KCkkgdGhpbmsgeW91IGhhdmUgbWlzdW5k ZXJzdG9vZCBteSBwb2ludDsgSSdtIHRhbGtpbmcgYWJvdXQgdGhlIGNhc2Ugb2YgYSBiYWQKa2Vy bmVsIHBvaW50ZXIgKndpdGhvdXQqIGEgbWVtb3J5IGVycm9yLgoKRm9yIGV4YW1wbGUsIGNvbnNp ZGVyIHNvbWUgYnVnZ3kgY29kZSBzdWNoIGFzOgoKCXZvaWQgX191c2VyICp1cHRyID0gc29tZV92 YWxpZF91c2VyX3BvaW50ZXI7Cgl2b2lkICprcHRyID0gTlVMTDsgLy8gb3IgYW55IG90aGVyIGJh ZCBwb2ludGVyCgoJcmV0ID0gY29weV90b191c2VyKHVwdHIsIGtwdHIsIHNpemUpOwoJaWYgKHJl dCkKCQlyZXR1cm4gLUVGQVVMVDsKCkJlZm9yZSB0aGlzIHBhdGNoLCB3aGVuIGNvcHlfdG9fdXNl cigpIGF0dGVtcHRlZCB0byBsb2FkIGZyb20gTlVMTCBpdCB3b3VsZApmYXVsdCwgdGhlcmUgd291 bGQgYmUgbm8gZml4dXAgaGFuZGxlciBmb3IgdGhlIExEUiwgYW5kIHRoZSBrZXJuZWwgd291bGQg ZGllKCksCnJlcG9ydGluZyB0aGUgYmFkIGtlcm5lbCBhY2Nlc3MuCgpBZnRlciB0aGlzIHBhdGNo ICh3aGljaCBhZGRzIGZpeHVwIGhhbmRsZXJzIHRvIGFsbCB0aGUgTERSKnMgaW4KY29weV90b191 c2VyKCkpLCB0aGUgZmF1bHQgKHdoaWNoIGlzICpub3QqIGEgbWVtb3J5IGVycm9yKSB3b3VsZCBi ZSBoYW5kbGVkIGJ5CnRoZSBmaXh1cCBoYW5kbGVyLCBhbmQgY29weV90b191c2VyKCkgd291bGQg cmV0dXJuIGFuIGVycm9yIHdpdGhvdXQgKmFueSoKaW5kaWNhdGlvbiBvZiB0aGUgaG9ycmlibGUg a2VybmVsIGJ1Zy4KClRoaXMgd2lsbCBoaWRlIGtlcm5lbCBidWdzLCB3aGljaCB3aWxsIG1ha2Ug dGhvc2UgaGFyZGVyIHRvIGlkZW50aWZ5IGFuZCBmaXgsCmFuZCB3aWxsIGFsc28gcG90ZW50aWFs bHkgbWFrZSBpdCBlYXNpZXIgdG8gZXhwbG9pdCB0aGUga2VybmVsOiBpZiB0aGUgdXNlcgpzb21l aG93IGdhaW5zIGNvbnRyb2wgb2YgdGhlIGtlcm5lbCBwb2ludGVyLCB0aGV5IGNhbiByZWx5IG9u IHRoZSBmaXh1cCBoYW5kbGVyCnJldHVybmluZyBhbiBlcnJvciwgYW5kIGNhbiBzY2FuIHRocm91 Z2ggbWVtb3J5IHJhdGhlciB0aGFuIGR5aW5nIGFzIHNvb24gYXMKdGhleSBwYXMgYSBiYWQgcG9p bnRlci4KCj4gSW4gYWRkaXRpb24sIHRoZSBwYW5pYyBpbmZvcm1hdGlvbiBjb250YWlucyBuZWNl c3NhcnkgaW5mb3JtYXRpb24KPiBmb3IgdXNlcnMgdG8gY2hlY2suCgpUaGVyZSBpcyBubyBwYW5p YygpIGluIHRoZSBjYXNlIEkgYW0gZGVzY3JpYmluZy4KCj4gPiBTbyBOQUsgdG8gdGhpcyBjaGFu Z2UgYXMtaXM7IGxpa2V3aXNlIGZvciB0aGUgYWRkaXRpb24gb2YgVVNFUigpIHRvIG90aGVyIGxk cioKPiA+IG1hY3JvcyBpbiBjb3B5X2Zyb21fdXNlci5TIGFuZCB0aGUgYWRkaXRpb24gb2YgVVNF UigpIHN0ciogbWFjcm9zIGluCj4gPiBjb3B5X3RvX3VzZXIuUy4KPiA+IAo+ID4gSWYgd2Ugd2Fu dCB0byBoYW5kbGUgbWVtb3J5IGVycm9ycyBvbiBzb21lIGthY2Nlc3Nlcywgd2UgbmVlZCBhIG5l dyBFWF9UWVBFXyoKPiA+IHNlcGFyYXRlIGZyb20gdGhlIHVzdWFsIEVYX1RZUEVfS0FDRVNTX0VS Ul9aRVJPIHRoYXQgbWVhbnMgImhhbmRsZSBtZW1vcnkKPiA+IGVycm9ycywgYnV0IHRyZWF0IG90 aGVyIGZhdWx0cyBhcyBmYXRhbCIuIFRoYXQgc2hvdWxkIGNvbWUgd2l0aCBhIHJhdGlvbmFsZSBh bmQKPiA+IGV4cGxhbmF0aW9uIG9mIHdoeSBpdCdzIGFjdHVhbGx5IHVzZWZ1bC4KPiAKPiBUaGlz IG1ha2VzIHNlbnNlLiBBZGQga2FjY2VzcyB0eXBlcyB0aGF0IGNhbiBiZSBwcm9jZXNzZWQgcHJv cGVybHkuCj4gCj4gPiAKPiA+IFsuLi5dCj4gPiAKPiA+ID4gZGlmZiAtLWdpdCBhL2FyY2gvYXJt NjQvbW0vZXh0YWJsZS5jIGIvYXJjaC9hcm02NC9tbS9leHRhYmxlLmMKPiA+ID4gaW5kZXggNDc4 ZTYzOWY4NjgwLi4yOGVjMzVlM2QyMTAgMTAwNjQ0Cj4gPiA+IC0tLSBhL2FyY2gvYXJtNjQvbW0v ZXh0YWJsZS5jCj4gPiA+ICsrKyBiL2FyY2gvYXJtNjQvbW0vZXh0YWJsZS5jCj4gPiA+IEBAIC04 NSwxMCArODUsMTAgQEAgYm9vbCBmaXh1cF9leGNlcHRpb25fbWMoc3RydWN0IHB0X3JlZ3MgKnJl Z3MpCj4gPiA+ICAgCWlmICghZXgpCj4gPiA+ICAgCQlyZXR1cm4gZmFsc2U7Cj4gPiA+IC0JLyoK PiA+ID4gLQkgKiBUaGlzIGlzIG5vdCBjb21wbGV0ZSwgTW9yZSBNYWNoaW5lIGNoZWNrIHNhZmUg ZXh0YWJsZSB0eXBlIGNhbgo+ID4gPiAtCSAqIGJlIHByb2Nlc3NlZCBoZXJlLgo+ID4gPiAtCSAq Lwo+ID4gPiArCXN3aXRjaCAoZXgtPnR5cGUpIHsKPiA+ID4gKwljYXNlIEVYX1RZUEVfVUFDQ0VT U19FUlJfWkVSTzoKPiA+ID4gKwkJcmV0dXJuIGV4X2hhbmRsZXJfdWFjY2Vzc19lcnJfemVybyhl eCwgcmVncyk7Cj4gPiA+ICsJfQo+ID4gCj4gPiBQbGVhc2UgZm9sZCB0aGlzIHBhcnQgaW50byB0 aGUgcHJpb3IgcGF0Y2gsIGFuZCBzdGFydCBvZ2Ygd2l0aCAqb25seSogaGFuZGxpbmcKPiA+IGVy cm9ycyBvbiBhY2Nlc3NlcyBhbHJlYWR5IG1hcmtlZCB3aXRoIEVYX1RZUEVfVUFDQ0VTU19FUlJf WkVSTy4gSSB0aGluayB0aGF0Cj4gPiBjaGFuZ2Ugd291bGQgYmUgcmVsYXRpdmVseSB1bmNvbnRy b3ZlcnNpYWwsIGFuZCBpdCB3b3VsZCBiZSBtdWNoIGVhc2llciB0bwo+ID4gYnVpbGQgYXRvcCB0 aGF0Lgo+IAo+IE9LLCB0aGUgdHdvIHBhdGNoZXMgd2lsbCBiZSBtZXJnZWQgaW4gdGhlIG5leHQg cmVsZWFzZS4KClRoYW5rcy4KCk1hcmsuCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fXwpsaW51eC1hcm0ta2VybmVsIG1haWxpbmcgbGlzdApsaW51eC1hcm0t a2VybmVsQGxpc3RzLmluZnJhZGVhZC5vcmcKaHR0cDovL2xpc3RzLmluZnJhZGVhZC5vcmcvbWFp bG1hbi9saXN0aW5mby9saW51eC1hcm0ta2VybmVsCg== 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 38FB0C47DDF for ; Tue, 30 Jan 2024 12:01:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AB8B96B0075; Tue, 30 Jan 2024 07:01:38 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id A41836B007D; Tue, 30 Jan 2024 07:01:38 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8BA926B007E; Tue, 30 Jan 2024 07:01:38 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 735346B0075 for ; Tue, 30 Jan 2024 07:01:38 -0500 (EST) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 4AD5FA1925 for ; Tue, 30 Jan 2024 12:01:38 +0000 (UTC) X-FDA: 81735837876.20.C1335FF Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf16.hostedemail.com (Postfix) with ESMTP id 70C0618000C for ; Tue, 30 Jan 2024 12:01:36 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf16.hostedemail.com: domain of mark.rutland@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=mark.rutland@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1706616096; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=n0WbJbv6fzoimQXx65ShQdRhm3qiZXY+n9VOiK6k8oY=; b=yi9DtWcmsPaBl7a8Z8ptjgJdhDbpv5F+xFat9D4RxqkFvCrACnmBoEwR3popIz6Q9B5tD5 HtxlxgUppvTN9I2dtWQBMIn7Qw6g5LfXNiGpQEMCWkpYlyAs4bI8/Si0tnkYJC538vOADM TSPQBFr9XllRf1kSNOtqy9LY6wYyR1U= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf16.hostedemail.com: domain of mark.rutland@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=mark.rutland@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706616096; a=rsa-sha256; cv=none; b=J1MgnwG+nYa2/uLvN4OouowrFeNB5s4WH3qM63wZAo2P6+GUu/uk9r7zaZ9mzlp9wPyGQJ 2lQrNDwuB5/FJ5T7uDSwDnarlV1HqvCTM5mr8a681R62nUTKZoTLZX4xu5O96i9ucd0RTo u3e5O+PBjOSF8qi8joUV5vXBSiNqImM= 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 3DDFDDA7; Tue, 30 Jan 2024 04:02:19 -0800 (PST) Received: from FVFF77S0Q05N (unknown [10.57.48.92]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 57FCA3F762; Tue, 30 Jan 2024 04:01:31 -0800 (PST) Date: Tue, 30 Jan 2024 12:01:26 +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> <23795738-b86e-7709-bc2b-5abba2e77b68@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <23795738-b86e-7709-bc2b-5abba2e77b68@huawei.com> X-Rspam-User: X-Stat-Signature: z4dn7xzkaakuujtcaim49jnkcrmzgg1k X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 70C0618000C X-HE-Tag: 1706616096-767853 X-HE-Meta: U2FsdGVkX1+5AUMAdnvkCB/7I8iYoRXRYsvjymyiuQHWiTq5g95NybHTrdnA3xyeq8FiQG50YqISSR0SyYLyxst+QLBRKRzfFWXljpMmY2lkL3v4QjpgCBnEtzCONywfxNTwcgITjVUMu6Bs32WrCmOMAh17lVKH6aMpV64lCqIlmaPUf7pKznEzA+veSnDn2ylb/lyeD4Pl+ro3HYpP1MplztL0yaK5dYDwgX8+S3Ku9fPS+hwISGurhwOtxVsI2qjJudssX/gkQsdTegvO7ZAvsig0Z8ILCRt1+CHwAz7GVDODJDSmVFREtIMKS4cN+Toh0O/WeODWdg74gQYYz5uACXrOs16TDqmgBACOu1bgkwXQBmz1AvTbpwiOAqumOrIP7aLdhOjI9sIdB76z/7CMJ6WGKHfXCABDqWdT9u/ZlZYautWPvhz3l/uUAb0xYpE1fh0GsZfuAwKNVOhB9KMXeauqw5Ldzg1ErslFM7fCZGljeVXnvNb27sTaD9Wi0m8IWtV1eEVhlBC/IqtiOTWkwHmwCBolhnkv0CD+dtI1J3+EZxwMfNlXUA16T/bPL7AlXFa9hwrjUAxT45FUchLBjPBiiU5VRem7vR1GDerio51pJ/xE8gRPY++JKwsqy68syXUfY/UUcwmmAM2rZOX2a5vcLqUwWND9cHHIHJ7jdZDWNRfCXTuR4kz8eLyO4Fe/fRWBfO+LTT+CjWcdSbbZzt+l25bjSBpThUEyERvnhgJT8u6P9E6xaIKIIyvJV4obMZd8AC7sXArCdPPSgMQ8hI9Z2Ianb50soFMFRxLW5IJtfFiHMdPdvQ+DL44qNjiDUstKsFAH2FK8zqLRZipdogdcJQTkF8sN+1u5HreUUAIQpsQ/QxcH/WpZCXvObGbVtLbLvHdMPu1Bt332D+FK3sKwm2YnNrB1oaY132Z5wc36S0E5k38lYTkI1ac9J+Bjc8WSyA2gggvphH2 Waq9VsSE y62jq/WvTiGW93+8yPr+NdFZiXSWHIrp87cJ2GeBHA2toqYtH84oqamvQ+UnwxKr1F7SdLbSs7EXb+TdwMefuZtDg0G72w0fzSPwM/hL6Bk5LKRC+OJ0UAxwb/S/++jMMhfBi1zZG7RFb6O6m3wkg4WK/22ks253Ja3WZBAonzchPBqTfBGkg0NC4iv8hHVLhy0KgXsKB9JLegBk= 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 Tue, Jan 30, 2024 at 07:14:35PM +0800, Tong Tiangen wrote: > 在 2024/1/30 1:43, Mark Rutland 写道: > > On Mon, Jan 29, 2024 at 09:46:49PM +0800, Tong Tiangen wrote: > > 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. > > I think this is better than the panic kernel, because the real bugs > belongs to the user process. Even if the wrong pointer is > transferred, the page corresponding to the wrong pointer has a memroy > error. I think you have misunderstood my point; I'm talking about the case of a bad kernel pointer *without* a memory error. For example, consider some buggy code such as: void __user *uptr = some_valid_user_pointer; void *kptr = NULL; // or any other bad pointer ret = copy_to_user(uptr, kptr, size); if (ret) return -EFAULT; Before this patch, when copy_to_user() attempted to load from NULL it would fault, there would be no fixup handler for the LDR, and the kernel would die(), reporting the bad kernel access. After this patch (which adds fixup handlers to all the LDR*s in copy_to_user()), the fault (which is *not* a memory error) would be handled by the fixup handler, and copy_to_user() would return an error without *any* indication of the horrible kernel bug. This will hide kernel bugs, which will make those harder to identify and fix, and will also potentially make it easier to exploit the kernel: if the user somehow gains control of the kernel pointer, they can rely on the fixup handler returning an error, and can scan through memory rather than dying as soon as they pas a bad pointer. > In addition, the panic information contains necessary information > for users to check. There is no panic() in the case I am describing. > > 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. > > This makes sense. Add kaccess types that can be processed properly. > > > > > [...] > > > > > 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. > > OK, the two patches will be merged in the next release. Thanks. Mark.