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

From: Kees Cook
Date: Tue Sep 27 2022 - 15:24:05 EST


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