linux-kernel - Re: [RFC PATCH] Drop some asm from copy_user

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20150514093630.GA29125@pd.tnic>
Date:	Thu, 14 May 2015 11:36:30 +0200
From:	Borislav Petkov <bp@...en8.de>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Ingo Molnar <mingo@...nel.org>, "H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Andy Lutomirski <luto@...capital.net>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	lkml <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH] Drop some asm from copy_user_64.S

On Wed, May 13, 2015 at 09:02:41AM -0700, Linus Torvalds wrote:
> The nice thing about using "rep movsb" for the user copy is that not
> only is it fairly close to optimal (for non-constant sizes) on newer
> Intel CPU's, but the fixup is also trivial. So we really should inline
> it. Just look at it: the copy_user_enhanced_fast_string function is
> literally just three 2-byte instructions right now:
> 
>     mov    %edx,%ecx
>     rep movsb
>     xor    %eax,%eax
> 
> and the rest is just the exception table thing.

Yeah, so I thought about it for a while and yeah, those labels there
would be a problem. Because you have this:

      mov    %edx,%ecx
1:    rep movsb
      xor    %eax,%eax

and _ASM_EXTABLE adds the .fixup section entry in the form of relative
offsets.

So I *think* it would work if we make the REP;STOSB case the default
one, i.e. those insns get issued during build. Then the labels will be
fine and all is good.

When they have to get patched probably with a CALL to the other
variants, the label 1: above (or rather the fixup entry) will point to
the newly patched instruction which, if it faults, might get fixed up
erroneously.

Hmm, let me give it a try - I'll have a better idea after I've done it.

> (And yes, there's the STAC/CLAC thing around it, but I think that
> should just be moved into _copy_from/to_user() too, since *all* of the
> copy_user_generic() cases need it).

Yeah.

> Yeah, yeah, we'd still do the double call thing for the more complex
> cases of the unrolled copy loop or the "movsq + tail" cases, but those
> are at least big enough that it makes sense. And they are presumably
> getting less common anyway.

Right, so we can avoid the first CALL if I inline copy_user_generic()
which practically inlines the alternative directly.

Lemme play with it a little...

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/