linux-kernel - Re: [PATCH v13] x86, mce: Add memcpy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+55aFyx1ziygg9EtQzUB5JDt0akaJ-TZvHKbZ3Wrho-_yUmgA@mail.gmail.com>
Date:	Thu, 25 Feb 2016 12:39:07 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	"Luck, Tony" <tony.luck@...el.com>
Cc:	Ingo Molnar <mingo@...nel.org>, Tony Luck <tony.luck@...il.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Borislav Petkov <bp@...en8.de>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"H. Peter Anvin" <hpa@...or.com>, Andy Lutomirski <luto@...nel.org>
Subject: Re: [PATCH v13] x86, mce: Add memcpy_trap()

On Thu, Feb 25, 2016 at 11:33 AM, Luck, Tony <tony.luck@...el.com> 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