linux-kernel - Re: Question Regarding ERMS 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: <20170305195432.6occvwaujq3l4ejl@pd.tnic>
Date:   Sun, 5 Mar 2017 20:54:33 +0100
From:   Borislav Petkov <bp@...e.de>
To:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Logan Gunthorpe <logang@...tatee.com>
Cc:     Peter Anvin <hpa@...or.com>, Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Tony Luck <tony.luck@...el.com>,
        Al Viro <viro@...iv.linux.org.uk>,
        the arch/x86 maintainers <x86@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Question Regarding ERMS memcpy

On Sun, Mar 05, 2017 at 11:19:42AM -0800, Linus Torvalds wrote:
> Actually, the "fromio/toio" code should never use regular memcpy().
> There used to be devices that literally broke on 64-bit accesses due
> to broken PCI crud.
> 
> We seem to have broken this *really* long ago, though.

I wonder why nothing blew up or failed strangely by now...

> On x86-64 we used to have a special __inline_memcpy() that copies our
> historical

It is still there in arch/x86/include/asm/string_64.h.

The comment "Only used for special circumstances." over it is grossly
understating it.

> 32-bit thing, and was used for memcpy_fromio() and memcpy_toio(). That
> was then undone by commit 6175ddf06b61 ("x86: Clean up mem*io
> functions")
> 
> That commit says
> 
>    "Iomem has no special significance on x86"
> 
> but that's not strictly true. iomem is in the same address space and
> uses the same access instructions as regular memory, but iomem _is_
> special.
> 
> And I think it's a bug that we use "memcpy()" on it. Not because of
> any gcc issues, but simply because our own memcpy() optimizations are
> not appropriate for iomem.
> 
> For example, "rep movsb" really is the right thing to use on normal
> memory on modern CPU's.

So Logan's box is a SNB and it doesn't have the ERMS optimizations. Are
you saying, regardless, we should let gcc put REP; MOVSB for smaller
sizes?

Because gcc does generate a REP; MOVSB there when it puts its own
memcpy, see mail upthread. (Even though that is wrong to do on iomem.)

> But it is *not* the right thing to use on IO memory, because the CPU
> only does the magic cacheline access optimizations on cacheable
> memory!

Ah, yes.

> So I think we should re-introduce that old "__inline_memcpy()" as that
> special "safe memcpy" thing. Not just for KMEMCHECK, and not just for
> 64-bit.

Logan, wanna give that a try, see if it takes care of your issue?

Oh, and along with the revert we would need a big fat warning explaining
why we need that special memcpy for IO memory.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--