linux-kernel - Re: [PATCH 01/11] x86: Use asm goto to implement better modify_and

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:	Wed, 18 Sep 2013 13:44:31 -0500
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Ingo Molnar <mingo@...nel.org>, Andi Kleen <ak@...ux.intel.com>,
	Peter Anvin <hpa@...or.com>,
	Mike Galbraith <bitbucket@...ine.de>,
	Thomas Gleixner <tglx@...utronix.de>,
	Arjan van de Ven <arjan@...ux.intel.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>
Subject: Re: [PATCH 01/11] x86: Use asm goto to implement better
 modify_and_test() functions

On Tue, Sep 17, 2013 at 4:10 AM, Peter Zijlstra <peterz@...radead.org> wrote:
> Linus suggested using asm goto to get rid of the typical SETcc + TEST
> instruction pair -- which also clobbers an extra register -- for our
> typical modify_and_test() functions.

Thinking about this, we actually have another place in x86 low-level
code where "asm goto" makes a lot of sense: exception handling for
__put_user_asm().

I'd love to use it for __get_user_asm() too, but it doesn't work there
because "asm goto" cannot have outputs (and a get_user obviously needs
an output - the value it gets). But for put_user(), it seems to be a
very good match.

The attached patch is ENTIRELY untested, but I did check some of the
generated assembly language. And the output is absolutely beautiful,
because now gcc sees the error case directly, so the straight-line
code is just he single "mov" instruction, no tests, no nothing. The
exception case will just jump to the local label directly.

Of course, the STAC/CLAC noise is there, and we really should try to
come up with a better model for that (so that the code that uses
__put_user() because it wants to do many of them in one go after
having done one access_ok() check) but that's a separate issue.

hpa, comments? Are you looking at perhaps moving the stac/clac
instructions out? With this, "filldir()" ends up lookng something like

   ...
   data32 xchg %ax,%ax  # stac
   mov    %rcx,0x8(%rax)
   data32 xchg %ax,%ax  # clac
   mov    0x10(%rbx),%r13
   data32 xchg %ax,%ax  # stac
   mov    %r8,0x0(%r13)
   data32 xchg %ax,%ax  # clac
   data32 xchg %ax,%ax  # stac
   mov    %r12w,0x10(%r13)
   data32 xchg %ax,%ax  # clac
   ...

which is a bit sad, since the code really is almost perfect aside from
the tons of extra nops/stac/clac instructions...

                        Linus

Download attachment "patch.diff" of type "application/octet-stream" (1436 bytes)