lkml.org 
[lkml]   [1999]   [Nov]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[patch] string.h speedup, cld-2.3.30-A1

x86 GCC and string.h generates lots of 'cld' instructions for string /
memory copies. My 2.3.30-pre2 kernel contains 973 cld instructions, often
in hot paths. Most of these cld's are unnecessery, and just add a 6 cycle
overhead and pollute the icache. This is a problem which has been long
known, but not addressed due to various subtle dangers related to the
change.

'cld' is used so frequently on x86 CPUs because untrusted code is free to
do a 'std' to change the direction of memory/string copies. This flag is
subtle because it only affects string/memory copies, and can lead to
security problems (and has lead to at least one exploit in the distant
past) which go unnoticed during normal use. (normal code doesnt set std
explicitly before calling the kernel)

but all Linux entry and 'untrusted exit' points already do a 'cld', so the
change would be safe, conceptually.

To be 100% safe, i tried to find an 'indirect' way to ensure that no
entry/exit point go unnoticed, and the key is segment registers. Whenever
we change to another protection domain, we must expect '%ds' and '%es' to
get changed by untrusted code. So the 'cld change' can be validated by
checking all places that store/restore %ds. If any such place is missed
then it's unsafe already, because untrusted code can change %ds and trick
the kernel into either crashing or writing to the wrong area.

x86 places that save/restore %ds are easy to find (rgrep -rl '%ds' .):

./include/asm-i386/hw_irq.h
./drivers/char/nvram.c
./drivers/scsi/in2000.h
./arch/i386/boot/bootsect.S
./arch/i386/boot/setup.S
./arch/i386/boot/compressed/head.S
./arch/i386/boot/video.S
./arch/i386/kernel/entry.S
./arch/i386/kernel/head.S
./arch/i386/kernel/process.c
./arch/i386/kernel/trampoline.S
./arch/i386/kernel/apm.c
./arch/i386/kernel/pci-pc.c
./arch/i386/kernel/apm.c.orig

a few of these are trivially not to be protected (boot code does a final
cld, reboot code never returns), the remaining ones are:

./include/asm-i386/hw_irq.h
./arch/i386/kernel/entry.S
./arch/i386/kernel/apm.c
./arch/i386/kernel/pci-pc.c

pci-pc.c is the part dealing with PCI BIOS callbacks, apm.c is calling the
APM BIOS, entry.S has our main entry points (syscalls, traps, faults).
hw_irq.h has the macros for IRQ entry points. All these entry/exit points
are sufficiently protected by a 'cld' after applying my patch. pci-pc.c
had to be fixed to do a cld after calling the BIOS.

string.h's only function which does an 'std' (memmove) has to have a
matching 'cld'.

future exit/entry points will have to do a 'cld' when returning from
untrusted domains, just like they have to restore segment registers.

a the same kernel after applying the attached patch contains 284 'cld's
and is 4k smaller. (the 4k savings are not an error, they are due to
alignment gains, the 600 clds themselves caused a 4k kernel code bloat)


much of the remaining 284 cld's is still unjustified, because '*a = *b;'
type of structure copies are inlined by GCC. (GCC generates a cld because
user-space has to be prepared to be interrupted by uncooperative signal
handlers and the like). So i added a cpy(x,y) macro which copies one
structure into another via memcpy. We may not want to do it this way
though, if it's too ugly. I have not found any way to prevent GCC from
using it's own memcpy function in the structure copy case.

the patch works just fine here, and i'm reasonably sure that no entry/exit
point is missing. (i ran testcode for some time which does 'std' and 'cld'
and thus tests the IRQ handler entry code and the syscall entry code. No
problems whatsoever, as expected.)

-- mingo
--- linux/include/asm-i386/string.h.orig Sat Nov 27 05:44:12 1999
+++ linux/include/asm-i386/string.h Sat Nov 27 06:26:17 1999
@@ -32,7 +32,6 @@
{
int d0, d1, d2;
__asm__ __volatile__(
- "cld\n"
"1:\tlodsb\n\t"
"stosb\n\t"
"testb %%al,%%al\n\t"
@@ -47,7 +46,6 @@
{
int d0, d1, d2, d3;
__asm__ __volatile__(
- "cld\n"
"1:\tdecl %2\n\t"
"js 2f\n\t"
"lodsb\n\t"
@@ -67,7 +65,6 @@
{
int d0, d1, d2, d3;
__asm__ __volatile__(
- "cld\n\t"
"repne\n\t"
"scasb\n\t"
"decl %1\n"
@@ -85,7 +82,6 @@
{
int d0, d1, d2, d3;
__asm__ __volatile__(
- "cld\n\t"
"repne\n\t"
"scasb\n\t"
"decl %1\n\t"
@@ -110,7 +106,6 @@
int d0, d1;
register int __res;
__asm__ __volatile__(
- "cld\n"
"1:\tlodsb\n\t"
"scasb\n\t"
"jne 2f\n\t"
@@ -132,7 +127,6 @@
register int __res;
int d0, d1, d2;
__asm__ __volatile__(
- "cld\n"
"1:\tdecl %3\n\t"
"js 2f\n\t"
"lodsb\n\t"
@@ -156,7 +150,6 @@
int d0;
register char * __res;
__asm__ __volatile__(
- "cld\n\t"
"movb %%al,%%ah\n"
"1:\tlodsb\n\t"
"cmpb %%ah,%%al\n\t"
@@ -176,7 +169,6 @@
int d0, d1;
register char * __res;
__asm__ __volatile__(
- "cld\n\t"
"movb %%al,%%ah\n"
"1:\tlodsb\n\t"
"cmpb %%ah,%%al\n\t"
@@ -194,7 +186,6 @@
int d0;
register int __res;
__asm__ __volatile__(
- "cld\n\t"
"repne\n\t"
"scasb\n\t"
"notl %0\n\t"
@@ -207,7 +198,6 @@
{
int d0, d1, d2;
__asm__ __volatile__(
- "cld\n\t"
"rep ; movsl\n\t"
"testb $2,%b4\n\t"
"je 1f\n\t"
@@ -273,7 +263,6 @@
}
#define COMMON(x) \
__asm__ __volatile__( \
- "cld\n\t" \
"rep ; movsl" \
x \
: "=&c" (d0), "=&D" (d1), "=&S" (d2) \
@@ -343,13 +332,14 @@

#endif

+#define cpy(x,y) memcpy(x, y, sizeof(*(x)))
+
#define __HAVE_ARCH_MEMMOVE
extern inline void * memmove(void * dest,const void * src, size_t n)
{
int d0, d1, d2;
if (dest<src)
__asm__ __volatile__(
- "cld\n\t"
"rep\n\t"
"movsb"
: "=&c" (d0), "=&S" (d1), "=&D" (d2)
@@ -379,7 +369,6 @@
if (!count)
return NULL;
__asm__ __volatile__(
- "cld\n\t"
"repne\n\t"
"scasb\n\t"
"je 1f\n\t"
@@ -393,7 +382,6 @@
{
int d0, d1;
__asm__ __volatile__(
- "cld\n\t"
"rep\n\t"
"stosb"
: "=&c" (d0), "=&D" (d1)
@@ -414,7 +402,6 @@
{
int d0, d1;
__asm__ __volatile__(
- "cld\n\t"
"rep ; stosl\n\t"
"testb $2,%b3\n\t"
"je 1f\n\t"
@@ -475,7 +462,7 @@
return s;
}
#define COMMON(x) \
-__asm__ __volatile__("cld\n\t" \
+__asm__ __volatile__( \
"rep ; stosl" \
x \
: "=&c" (d0), "=&D" (d1) \
@@ -518,8 +505,7 @@
{
if (!size)
return addr;
- __asm__("cld
- repnz; scasb
+ __asm__("repnz; scasb
jnz 1f
dec %%edi
1: "
--- linux/include/asm-i386/string-486.h.orig Sat Nov 27 05:45:53 1999
+++ linux/include/asm-i386/string-486.h Sat Nov 27 05:58:05 1999
@@ -187,7 +187,6 @@
int d0, d1;
register char * __res;
__asm__ __volatile__(
- "cld\n\t"
"movb %%al,%%ah\n"
"1:\tlodsb\n\t"
"cmpb %%ah,%%al\n\t"
@@ -206,7 +205,6 @@
int d0, d1;
register char * __res;
__asm__ __volatile__(
- "cld\n\t"
"movl %6,%%edi\n\t"
"repne\n\t"
"scasb\n\t"
@@ -234,7 +232,6 @@
int d0, d1;
register char * __res;
__asm__ __volatile__(
- "cld\n\t"
"movl %6,%%edi\n\t"
"repne\n\t"
"scasb\n\t"
@@ -263,7 +260,6 @@
int d0, d1;
register char * __res;
__asm__ __volatile__(
- "cld\n\t"
"movl %6,%%edi\n\t"
"repne\n\t"
"scasb\n\t"
@@ -296,7 +292,6 @@
int d0, d1;
register char * __res;
__asm__ __volatile__(
- "cld\n\t" \
"movl %6,%%edi\n\t"
"repne\n\t"
"scasb\n\t"
@@ -378,7 +373,6 @@
"1:\txorl %0,%0\n\t"
"movl $-1,%%ecx\n\t"
"xorl %%eax,%%eax\n\t"
- "cld\n\t"
"movl %4,%%edi\n\t"
"repne\n\t"
"scasb\n\t"
@@ -474,7 +468,6 @@
int d0, d1, d2;
register void *tmp = (void *)to;
__asm__ __volatile__ (
- "cld\n\t"
"shrl $1,%%ecx\n\t"
"jnc 1f\n\t"
"movsb\n"
@@ -554,7 +547,6 @@
register void *tmp = (void *)dest;
if (dest<src)
__asm__ __volatile__ (
- "cld\n\t"
"rep\n\t"
"movsb"
:"=&c" (d0), "=&S" (d1), "=&D" (d2)
@@ -577,7 +569,6 @@
int d0, d1, d2;
register int __res;
__asm__ __volatile__(
- "cld\n\t"
"repe\n\t"
"cmpsb\n\t"
"je 1f\n\t"
@@ -597,7 +588,6 @@
if (!count)
return NULL;
__asm__ __volatile__(
- "cld\n\t"
"repne\n\t"
"scasb\n\t"
"je 1f\n\t"
@@ -753,8 +743,7 @@
{
if (!size)
return addr;
- __asm__("cld
- repnz; scasb
+ __asm__("repnz; scasb
jnz 1f
dec %%edi
1: "
--- linux/include/asm-i386/io.h.orig Sat Nov 27 05:46:43 1999
+++ linux/include/asm-i386/io.h Sat Nov 27 06:26:27 1999
@@ -71,12 +71,12 @@

#define __INS(s) \
extern inline void ins##s(unsigned short port, void * addr, unsigned long count) \
-{ __asm__ __volatile__ ("cld ; rep ; ins" #s \
+{ __asm__ __volatile__ ("rep ; ins" #s \
: "=D" (addr), "=c" (count) : "d" (port),"0" (addr),"1" (count)); }

#define __OUTS(s) \
extern inline void outs##s(unsigned short port, const void * addr, unsigned long count) \
-{ __asm__ __volatile__ ("cld ; rep ; outs" #s \
+{ __asm__ __volatile__ ("rep ; outs" #s \
: "=S" (addr), "=c" (count) : "d" (port),"0" (addr),"1" (count)); }

#define RETURN_TYPE unsigned char
--- linux/include/asm-i386/bitops.h.orig Sat Nov 27 05:46:55 1999
+++ linux/include/asm-i386/bitops.h Sat Nov 27 05:47:09 1999
@@ -132,8 +132,7 @@

if (!size)
return 0;
- __asm__("cld\n\t"
- "movl $-1,%%eax\n\t"
+ __asm__("movl $-1,%%eax\n\t"
"xorl %%edx,%%edx\n\t"
"repe; scasl\n\t"
"je 1f\n\t"
--- linux/arch/i386/kernel/apm.c.orig Sat Nov 27 05:47:49 1999
+++ linux/arch/i386/kernel/apm.c Sat Nov 27 05:52:58 1999
@@ -380,7 +380,7 @@
__asm__ __volatile__(APM_DO_ZERO_SEGS
"pushl %%edi\n\t"
"pushl %%ebp\n\t"
- "lcall %%cs:" SYMBOL_NAME_STR(apm_bios_entry) "\n\t"
+ "lcall %%cs:" SYMBOL_NAME_STR(apm_bios_entry) "; cld\n\t"
"setc %%al\n\t"
"popl %%ebp\n\t"
"popl %%edi\n\t"
@@ -413,7 +413,7 @@
__asm__ __volatile__(APM_DO_ZERO_SEGS
"pushl %%edi\n\t"
"pushl %%ebp\n\t"
- "lcall %%cs:" SYMBOL_NAME_STR(apm_bios_entry) "\n\t"
+ "lcall %%cs:" SYMBOL_NAME_STR(apm_bios_entry)"cld;\n\t"
"setc %%bl\n\t"
"popl %%ebp\n\t"
"popl %%edi\n\t"
--- linux/arch/i386/kernel/pci-pc.c.orig Sat Nov 27 05:49:53 1999
+++ linux/arch/i386/kernel/pci-pc.c Sat Nov 27 05:51:44 1999
@@ -342,7 +342,7 @@
unsigned long flags;

__save_flags(flags); __cli();
- __asm__("lcall (%%edi)"
+ __asm__("lcall (%%edi); cld"
: "=a" (return_code),
"=b" (address),
"=c" (length),
@@ -383,7 +383,7 @@

__save_flags(flags); __cli();
__asm__(
- "lcall (%%edi)\n\t"
+ "lcall (%%edi); cld\n\t"
"jc 1f\n\t"
"xor %%ah, %%ah\n"
"1:"
@@ -427,7 +427,7 @@
unsigned short bx;
unsigned short ret;

- __asm__("lcall (%%edi)\n\t"
+ __asm__("lcall (%%edi); cld\n\t"
"jc 1f\n\t"
"xor %%ah, %%ah\n"
"1:"
@@ -448,7 +448,7 @@
unsigned long ret;
unsigned long bx = (dev->bus->number << 8) | dev->devfn;

- __asm__("lcall (%%esi)\n\t"
+ __asm__("lcall (%%esi); cld\n\t"
"jc 1f\n\t"
"xor %%ah, %%ah\n"
"1:"
@@ -466,7 +466,7 @@
unsigned long ret;
unsigned long bx = (dev->bus->number << 8) | dev->devfn;

- __asm__("lcall (%%esi)\n\t"
+ __asm__("lcall (%%esi); cld\n\t"
"jc 1f\n\t"
"xor %%ah, %%ah\n"
"1:"
@@ -484,7 +484,7 @@
unsigned long ret;
unsigned long bx = (dev->bus->number << 8) | dev->devfn;

- __asm__("lcall (%%esi)\n\t"
+ __asm__("lcall (%%esi); cld\n\t"
"jc 1f\n\t"
"xor %%ah, %%ah\n"
"1:"
@@ -502,7 +502,7 @@
unsigned long ret;
unsigned long bx = (dev->bus->number << 8) | dev->devfn;

- __asm__("lcall (%%esi)\n\t"
+ __asm__("lcall (%%esi); cld\n\t"
"jc 1f\n\t"
"xor %%ah, %%ah\n"
"1:"
@@ -520,7 +520,7 @@
unsigned long ret;
unsigned long bx = (dev->bus->number << 8) | dev->devfn;

- __asm__("lcall (%%esi)\n\t"
+ __asm__("lcall (%%esi); cld\n\t"
"jc 1f\n\t"
"xor %%ah, %%ah\n"
"1:"
@@ -538,7 +538,7 @@
unsigned long ret;
unsigned long bx = (dev->bus->number << 8) | dev->devfn;

- __asm__("lcall (%%esi)\n\t"
+ __asm__("lcall (%%esi); cld\n\t"
"jc 1f\n\t"
"xor %%ah, %%ah\n"
"1:"
@@ -702,7 +702,7 @@
__asm__("push %%es\n\t"
"push %%ds\n\t"
"pop %%es\n\t"
- "lcall (%%esi)\n\t"
+ "lcall (%%esi); cld\n\t"
"pop %%es\n\t"
"jc 1f\n\t"
"xor %%ah, %%ah\n"
--- linux/arch/i386/kernel/process.c.orig Sat Nov 27 06:00:12 1999
+++ linux/arch/i386/kernel/process.c Sat Nov 27 06:26:25 1999
@@ -462,7 +462,7 @@
struct pt_regs * childregs;

childregs = ((struct pt_regs *) (THREAD_SIZE + (unsigned long) p)) - 1;
- *childregs = *regs;
+ cpy(childregs, regs);
childregs->eax = 0;
childregs->esp = esp;

@@ -475,7 +475,7 @@
savesegment(gs,p->thread.gs);

unlazy_fpu(current);
- p->thread.i387 = current->thread.i387;
+ cpy(&p->thread.i387, &current->thread.i387);

return 0;
}
\
 
 \ /
  Last update: 2005-03-22 13:55    [from the cache]
©2003-2011 Jasper Spaans