From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751409AbcBYUjK (ORCPT ); Thu, 25 Feb 2016 15:39:10 -0500 Received: from mail-io0-f193.google.com ([209.85.223.193]:34264 "EHLO mail-io0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750780AbcBYUjI (ORCPT ); Thu, 25 Feb 2016 15:39:08 -0500 MIME-Version: 1.0 In-Reply-To: <56cf571d44495b2cf@agluck-desk.sc.intel.com> References: <20160225085624.GA13833@gmail.com> <56cf571d44495b2cf@agluck-desk.sc.intel.com> Date: Thu, 25 Feb 2016 12:39:07 -0800 X-Google-Sender-Auth: FD6-dVa69JiOd4naEWmTSql1ajE Message-ID: Subject: Re: [PATCH v13] x86, mce: Add memcpy_trap() From: Linus Torvalds To: "Luck, Tony" Cc: Ingo Molnar , Tony Luck , Thomas Gleixner , Peter Zijlstra , Borislav Petkov , Linux Kernel Mailing List , Andrew Morton , "H. Peter Anvin" , Andy Lutomirski Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Feb 25, 2016 at 11:33 AM, Luck, Tony wrote: > For reference below is what I'd hoped to be able to do with > copy_from_user() [obviously needs to not just replace that > ALTERNATIVE_2 setup in _copy_from_user ... would have to invent > an ALTERNATIVE_3 to pick the new function for people willing > to sacrifice speed for recoverability] Why? I really don't see why you are so obsessed with replacing our current thing. Just replace the error number in the exception path slow handling. That's *all*. This should all be entirely about just the exception itself doing that memory failure accounting, and the actual copying code shouldn't be different or care about things AT ALL outside of the fact that we need to replace the -EFAULT with something else. I will keep on NAK'ing these insane "let's replace the copy code" patches. I'll do it forever if that is what it takes. And no, if the issue is that you want to replace "rep movs", then I will _still_ keep NAK'ing them. Because if the hardware gets it wrong, then there is absolutely no point in us special-casing one of the _last_ common memory operations in the whole system. The user copying is literally a drop in the ocean. User space does a lot more memcpy() on its own than we will *ever* copy data from user space. Now, if the patch starts looking more like - return -EFAULT; + return memory_access_fault(): where "memory_access_fault()" might do something like "current_thread_info()->trap_nr == X86_TRAP_MC ? -EBADMEM : -EFAULT", now *then* we'll be talking. But doing things like + if (r.trap_nr == X86_TRAP_MC) { + volatile void *fault_addr = (volatile void *)from + n - r.bytes_left; + phys_addr_t p = virt_to_phys(fault_addr); + + memory_failure(p >> PAGE_SHIFT, MCE_VECTOR, 0); + } in the copying code is insane, because dammit, that should be done by the codethat sets X86_TRAP_MC in the first place. And if there is hardware that raises a machine check without actually telling you why - including the address - then it's laugable to talk about "recoverability" and "hardening" and things like that. Then the hardware is just broken. Linus