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]

Date:	Thu, 25 Feb 2016 09:56:24 +0100
From:	Ingo Molnar <mingo@...nel.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	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()


* Linus Torvalds <torvalds@...ux-foundation.org> wrote:

> On Wed, Feb 24, 2016 at 11:27 AM, Tony Luck <tony.luck@...il.com> wrote:
> >
> > This isn't just about NVDIMMs. It is about uncorrected errors in any
> > type of memory.
> >
> > The copy_from_user() case I'd like to fix is when there is an uncorrected in 
> > memory. This can happen to regular DDR3/DDR4 memory just as it can happen to 
> > NVDIMM. When a user process directly reads the location with the uncorrected 
> > error the h/w triggers a machine check and if the error is marked as 
> > recoverable the kernel will SIGBUS the process. See mm/memory-failure.c
> 
> But the way to fix that is absolutely *not* to introduce some new concept.
> 
> You'd just extend the _existing_ get_user() and put_user() functions to return a 
> different error code for a memory error. Instead of -EFAULT, maybe they can 
> return -EMEMERR or something.
> 
> I do not see how it could possibly ever make sense to introduce a new name and 
> then make old users use that new name instead.

So if we do that we should first fix these hard coded assumptions about uaccess 
return codes:

  triton:~/tip> git grep -E '== -EFAULT' | wc -l
  44

But yes, your suggestion sounds a lot cleaner and a lot more powerful overall.

AFAICS we'll still need this exception table extension commit:

  548acf19234d x86/mm: Expand the exception table logic to allow new handling options

to propagate the error code to actual uaccess methods, right?

Alternatively we could change the exception handling protocol and change _all_ 
uaccess methods all at once, but I think that would be crazy complex and risky. 
I'd rather change key uaccess methods step by step.

So if everyone agrees then I'll keep this commit queued up. The followup patches 
will have to be reworked according to Linus's suggestions.

Thanks,

	Ingo