lkml.org 
[lkml]   [2008]   [Aug]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: Kernel oops with 2.6.26, padlock and ipsec: probably problem with fpu state changes
    On Fri, Aug 08, 2008 at 03:36:54AM -0700, Wolfgang Walter wrote:
    > Am Donnerstag, 7. August 2008 18:23 schrieb Wolfgang Walter:
    > > But yesterday I tried the following patch on top of a vanilla 2.6.26:
    > >
    > > =======================================================
    > > diff -ur ../linux-2.6.26/drivers/crypto/padlock-aes.c
    > > ./drivers/crypto/padlock-aes.c ---
    > > ../linux-2.6.26/drivers/crypto/padlock-aes.c 2008-07-15 11:29:32.000000000
    > > +0200 +++ ./drivers/crypto/padlock-aes.c 2008-08-07 17:46:55.000000000
    > > +0200 @@ -16,6 +16,7 @@
    > > #include <linux/interrupt.h>
    > > #include <linux/kernel.h>
    > > #include <asm/byteorder.h>
    > > +#include <asm/i387.h>
    > > #include "padlock.h"
    > >
    > > /* Control word. */
    > > @@ -144,9 +145,11 @@
    > > static inline void padlock_xcrypt(const u8 *input, u8 *output, void *key,
    > > void *control_word)
    > > {
    > > + kernel_fpu_begin();
    > > asm volatile (".byte 0xf3,0x0f,0xa7,0xc8" /* rep xcryptecb */
    > >
    > > : "+S"(input), "+D"(output)
    > > : "d"(control_word), "b"(key), "c"(1));
    > >
    > > + kernel_fpu_end();
    > > }
    > >
    > > static void aes_crypt_copy(const u8 *in, u8 *out, u32 *key, struct cword
    > > *cword) @@ -179,6 +182,7 @@
    > > return;
    > > }
    > >
    > > + kernel_fpu_begin();
    > > asm volatile ("test $1, %%cl;"
    > > "je 1f;"
    > > "lea -1(%%ecx), %%eax;"
    > > @@ -190,15 +194,18 @@
    > >
    > > : "+S"(input), "+D"(output)
    > > : "d"(control_word), "b"(key), "c"(count)
    > > : "ax");
    > >
    > > + kernel_fpu_end();
    > > }
    > >
    > > static inline u8 *padlock_xcrypt_cbc(const u8 *input, u8 *output, void
    > > *key, u8 *iv, void *control_word, u32 count)
    > > {
    > > /* rep xcryptcbc */
    > > + kernel_fpu_begin();
    > > asm volatile (".byte 0xf3,0x0f,0xa7,0xd0"
    > >
    > > : "+S" (input), "+D" (output), "+a" (iv)
    > > : "d" (control_word), "b" (key), "c" (count));
    > >
    > > + kernel_fpu_end();
    > > return iv;
    > > }
    > >
    > > =============================================================
    > >
    > > I found that kernel_fpu_begin(); kernel_fpu_begin(); is used with
    > > MMX and/or SSE:
    > >
    > > include/asm/xor_32.h
    > > drivers/md/raid6mmx.c
    > > drivers/md/raid6sse1.c
    > > drivers/md/raid6sse2.c
    > >
    > >
    > > With this change I its a little bit more stable, I needed more then 5
    > > minutes to crash the kernel (repeated it several times). If I read
    > > the code correctly this disables preemption for the time the padlock cmd
    > > is executing.
    >
    > Forget that - I booted the wrong kernel.
    >
    > I now really test this modification and it seems to be stable. The router now
    > runs for 45 minutes without oops.

    Ok. Thanks. I think I can explain the failure:

    These padlock instructions though don't use/touch SSE registers, but it behaves
    similar to other SSE instructions. For example, it might cause DNA faults
    when cr0.ts is set. While this is a spurious DNA trap, it might cause
    problems with the recent fpu code changes.

    This is the code sequence that is probably causing this problem:

    a) new app is getting exec'd and it is somewhere in between
    start_thread() and flush_old_exec() in the load_xyz_binary()

    b) At pont "a", task's fpu state (like TS_USEDFPU, used_math() etc) is
    cleared.

    c) Now we get an interrupt/softirq which starts using these encrypt/decrypt
    routines in the network stack. This generates a math fault (as
    cr0.ts is '1') which sets TS_USEDFPU and restores the math that is
    in the task's xstate.

    d) Return to exec code path, which does start_thread() which does
    free_thread_xstate() and sets xstate pointer to NULL while
    the TS_USEDFPU is still set.

    e) At the next context switch from the new exec'd task to another task,
    we have a scenarios where TS_USEDFPU is set but xstate pointer is null.
    This can cause an oops during unlazy_fpu() in __switch_to()

    Now:

    1) This should happen with or with out pre-emption. Viro also encountered
    similar problem with out CONFIG_PREEMPT.

    2) kernel_fpu_begin() and kernel_fpu_end() will fix this problem, because
    kernel_fpu_begin() will manually do a clts() and won't run in to the
    situation of setting TS_USEDFPU in step "c" above.

    3) This was working before the fpu changes, because its a spurious
    math fault which doesn't corrupt any fpu/sse registers and the task's
    math state was always in an allocated state.

    I propose to go with wolf's patch which add's kernel_fpu_begin() and
    kernel_fpu_end() around these instructions. This is the correct fix
    (unless there is something wrong in my above understanding).

    thanks,
    suresh


    \
     
     \ /
      Last update: 2008-08-08 20:57    [W:3.413 / U:0.632 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site