linux-kernel - Re: [PATCH v2] x86, mem: move memmove to out of line assembler

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: <202209271143.7B7D15D@keescook>
Date:   Tue, 27 Sep 2022 12:23:57 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Nick Desaulniers <ndesaulniers@...gle.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        "H . Peter Anvin" <hpa@...or.com>,
        Peter Zijlstra <peterz@...radead.org>,
        linux-kernel@...r.kernel.org,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        llvm@...ts.linux.dev, Andy Lutomirski <luto@...nel.org>
Subject: Re: [PATCH v2] x86, mem: move memmove to out of line assembler

On Tue, Sep 27, 2022 at 10:28:39AM -0700, Nick Desaulniers wrote:
> In addition to having complex constraints that can't always be resolved,
> the clobber list seems to be missing %bx and %dx, and possibly %cl. By
> using numbered operands rather than symbolic operands, the constraints
> are quite obnoxious to refactor.
> [...]
> -		/*
> -		 * Move data from 2 bytes to 3 bytes.
> -		 */
> -		".p2align 4\n\t"
> -		"9:\n\t"
> -		"cmp $2, %0\n\t"
> -		"jb 10f\n\t"
> -		"movw 0*2(%1), %%dx\n\t"
> -		"movw -1*2(%1, %0), %%bx\n\t"
> -		"movw %%dx, 0*2(%2)\n\t"
> -		"movw %%bx, -1*2(%2, %0)\n\t"
> -		"jmp 11f\n\t"
> [...]
> +.set tmp0, %edi
> [...]
> +	/*
> +	 * Move data from 2 bytes to 3 bytes.
> +	 */
> +.p2align 4
> +.L4_byteswap:
> +	cmpl	$2, n
> +	jb	.Lbyteswap
> +	movw	0*2(src), %di
> +	movw	-1*2(src, n), %bx
> +	movw	%dx, 0*2(dest)
> +	movw	%bx, -1*2(dest, n)
> +	jmp	.Ldone

Found it (need to use %di instead of %dx). With this changed, the kunit
test passes again:

diff --git a/arch/x86/lib/memmove_32.S b/arch/x86/lib/memmove_32.S
index 73314a391a72..9e33c9a1c595 100644
--- a/arch/x86/lib/memmove_32.S
+++ b/arch/x86/lib/memmove_32.S
@@ -179,7 +179,7 @@ SYM_FUNC_START(memmove)
 	jb	.Lbyteswap
 	movw	0*2(src), %di
 	movw	-1*2(src, n), %bx
-	movw	%dx, 0*2(dest)
+	movw	%di, 0*2(dest)
 	movw	%bx, -1*2(dest, n)
 	jmp	.Ldone

-Kees

-- 
Kees Cook