| 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
| ||
|
Date: Sat, 14 Sep 2019 18:15:37 +0300
From: Alexey Dobriyan <adobriyan@...il.com>
To: Borislav Petkov <bp@...en8.de>
Cc: tglx@...utronix.de, mingo@...hat.com, hpa@...or.com,
linux-kernel@...r.kernel.org, x86@...r.kernel.org,
linux@...musvillemoes.dk, torvalds@...ux-foundation.org
Subject: Re: [PATCH] x86_64: new and improved memset()
On Sat, Sep 14, 2019 at 01:37:17PM +0200, Borislav Petkov wrote:
> On Sat, Sep 14, 2019 at 01:33:45PM +0300, Alexey Dobriyan wrote:
> > --- a/arch/x86/include/asm/string_64.h
> > +++ b/arch/x86/include/asm/string_64.h
> > @@ -15,7 +15,111 @@ extern void *memcpy(void *to, const void *from, size_t len);
> > extern void *__memcpy(void *to, const void *from, size_t len);
> >
> > #define __HAVE_ARCH_MEMSET
> > +#if defined(_ARCH_X86_BOOT) || defined(CONFIG_FORTIFY_SOURCE)
> > void *memset(void *s, int c, size_t n);
> > +#else
> > +#include <asm/alternative.h>
> > +#include <asm/cpufeatures.h>
> > +
> > +/* Internal, do not use. */
> > +static __always_inline void memset0(void *s, size_t n)
> > +{
> > + /* Internal, do not use. */
> > + void _memset0_mov(void);
> > + void _memset0_rep_stosq(void);
> > + void memset0_mov(void);
> > + void memset0_rep_stosq(void);
> > + void memset0_rep_stosb(void);
> > +
> > + if (__builtin_constant_p(n) && n == 0) {
> > + } else if (__builtin_constant_p(n) && n == 1) {
> > + *(uint8_t *)s = 0;
> > + } else if (__builtin_constant_p(n) && n == 2) {
> > + *(uint16_t *)s = 0;
> > + } else if (__builtin_constant_p(n) && n == 4) {
> > + *(uint32_t *)s = 0;
> > + } else if (__builtin_constant_p(n) && n == 6) {
> > + *(uint32_t *)s = 0;
> > + *(uint16_t *)(s + 4) = 0;
> > + } else if (__builtin_constant_p(n) && n == 8) {
> > + *(uint64_t *)s = 0;
> > + } else if (__builtin_constant_p(n) && (n & 7) == 0) {
> > + alternative_call_2(
> > + _memset0_mov,
> > + _memset0_rep_stosq, X86_FEATURE_REP_GOOD,
> > + memset0_rep_stosb, X86_FEATURE_ERMS,
> > + ASM_OUTPUT2("=D" (s), "=c" (n)),
> > + "D" (s), "c" (n)
> > + : "rax", "cc", "memory"
> > + );
> > + } else {
> > + alternative_call_2(
> > + memset0_mov,
> > + memset0_rep_stosq, X86_FEATURE_REP_GOOD,
> > + memset0_rep_stosb, X86_FEATURE_ERMS,
> > + ASM_OUTPUT2("=D" (s), "=c" (n)),
> > + "D" (s), "c" (n)
> > + : "rax", "rsi", "cc", "memory"
> > + );
> > + }
> > +}
> > +
> > +/* Internal, do not use. */
> > +static __always_inline void memsetx(void *s, int c, size_t n)
> > +{
> > + /* Internal, do not use. */
> > + void _memsetx_mov(void);
> > + void _memsetx_rep_stosq(void);
> > + void memsetx_mov(void);
> > + void memsetx_rep_stosq(void);
> > + void memsetx_rep_stosb(void);
> > +
> > + const uint64_t ccc = (uint8_t)c * 0x0101010101010101ULL;
> > +
> > + if (__builtin_constant_p(n) && n == 0) {
> > + } else if (__builtin_constant_p(n) && n == 1) {
> > + *(uint8_t *)s = ccc;
> > + } else if (__builtin_constant_p(n) && n == 2) {
> > + *(uint16_t *)s = ccc;
> > + } else if (__builtin_constant_p(n) && n == 4) {
> > + *(uint32_t *)s = ccc;
> > + } else if (__builtin_constant_p(n) && n == 8) {
> > + *(uint64_t *)s = ccc;
> > + } else if (__builtin_constant_p(n) && (n & 7) == 0) {
> > + alternative_call_2(
> > + _memsetx_mov,
> > + _memsetx_rep_stosq, X86_FEATURE_REP_GOOD,
> > + memsetx_rep_stosb, X86_FEATURE_ERMS,
> > + ASM_OUTPUT2("=D" (s), "=c" (n)),
> > + "D" (s), "c" (n), "a" (ccc)
> > + : "cc", "memory"
> > + );
> > + } else {
> > + alternative_call_2(
> > + memsetx_mov,
> > + memsetx_rep_stosq, X86_FEATURE_REP_GOOD,
> > + memsetx_rep_stosb, X86_FEATURE_ERMS,
> > + ASM_OUTPUT2("=D" (s), "=c" (n)),
> > + "D" (s), "c" (n), "a" (ccc)
> > + : "rsi", "cc", "memory"
> > + );
> > + }
> > +}
> > +
> > +static __always_inline void *memset(void *s, int c, size_t n)
> > +{
> > + if (__builtin_constant_p(c)) {
> > + if (c == 0) {
> > + memset0(s, n);
> > + } else {
> > + memsetx(s, c, n);
> > + }
> > + return s;
> > + } else {
> > + return __builtin_memset(s, c, n);
> > + }
> > +}
>
> I'm willing to take something like that only when such complexity is
> justified by numbers. I.e., I'm much more inclined to capping it under
> 32 and 64 byte sizes and keeping it simple.
OK. Those small lengths were indeed annoying.
> > +ENTRY(_memset0_mov)
> > + xor eax, eax
> > +.globl _memsetx_mov
> > +_memsetx_mov:
> > + add rcx, rdi
> > + cmp rdi, rcx
> > + je 1f
> > +2:
> > + mov [rdi], rax
> > + add rdi, 8
> > + cmp rdi, rcx
> > + jne 2b
> > +1:
> > + ret
> > +ENDPROC(_memset0_mov)
> > +ENDPROC(_memsetx_mov)
> > +EXPORT_SYMBOL(_memset0_mov)
> > +EXPORT_SYMBOL(_memsetx_mov)
> > +
> > +ENTRY(memset0_mov)
> > + xor eax, eax
> > +.globl memsetx_mov
> > +memsetx_mov:
> > + lea rsi, [rdi + rcx]
> > + cmp rdi, rsi
> > + je 1f
> > +2:
> > + mov [rdi], al
> > + add rdi, 1
> > + cmp rdi, rsi
> > + jne 2b
> > +1:
> > + ret
>
> Say what now? Intel syntax? You must be joking...
It is the best thing in the x86 assembler universe.
> > +ENDPROC(memset0_mov)
> > +ENDPROC(memsetx_mov)
> > +EXPORT_SYMBOL(memset0_mov)
> > +EXPORT_SYMBOL(memsetx_mov)
>
> Too many exported symbols.
Those are technical exports. memset() remains the only developer-visible
interface.
> Again, I'll much more prefer a cleaner,
> smaller solution than one where readability suffers greatly at the
> expense of *maybe* getting a bit better performance.
Readability is red herring, I for one find AT&T syntax unreadable.
Powered by blists - more mailing lists