lkml.org 
[lkml]   [2008]   [Jul]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: KVM overflows the stack
    Dave Hansen wrote:
    > On Wed, 2008-07-16 at 14:44 -0700, Dave Hansen wrote:
    >
    >> On a suggestion of Anthony's, I tried a defconfig kernel.
    >>
    >> It is now bombing out on an assertion in the lapic code:
    >>
    >> http://sr71.net/~dave/linux/2.6.26-oops1.txt
    >>
    >
    > I think I found it!!!
    >
    > $ (objdump -d kvm.ko ; objdump -d kvm-intel.ko ) | egrep 'sub.*0x...,.*esp|>:' | egrep sub -B1
    > 00001a90 <kvm_vcpu_ioctl>:
    > 1a9a: 81 ec 60 06 00 00 sub $0x660,%esp
    > --
    > 00004e90 <kvm_arch_vcpu_ioctl>:
    > 4e9d: 81 ec 6c 08 00 00 sub $0x86c,%esp
    > --
    > 00005900 <kvm_arch_vm_ioctl>:
    > 5903: 81 ec 34 05 00 00 sub $0x534,%esp
    > --
    > 0000d4f0 <paging64_prefetch_page>:
    > d4f8: 81 ec 1c 01 00 00 sub $0x11c,%esp
    > --
    > 0000dfd0 <paging32_prefetch_page>:
    > dfd8: 81 ec 1c 01 00 00 sub $0x11c,%esp
    > --
    > 0000f390 <kvm_pv_mmu_op>:
    > f3a1: 81 ec 28 02 00 00 sub $0x228,%esp
    >
    > We're simply overflowing the stack. I changed all of the large on-stack
    > allocations to 'static', and it actually boots now. I know 'static'
    > isn't safe, but it was good for a quick test.
    >
    >

    Yes! It's obvious, once you know it...

    > A 'make stackcheck' confirms this:
    >
    > dave@nimitz:~/kernels/linux-2.6.git$ make checkstack
    > objdump -d vmlinux $(find . -name '*.ko') | \
    > perl /home/dave/kernels/linux-2.6.git-t61/scripts/checkstack.pl i386
    > 0x000042d3 kvm_arch_vcpu_ioctl [kvm]: 2148
    > 0x000012e3 kvm_vcpu_ioctl [kvm]: 1620
    > 0x00004a83 kvm_arch_vm_ioctl [kvm]: 1332
    > 0x00009a26 airo_get_aplist [airo]: 1140
    > 0x00009b76 airo_get_aplist [airo]: 1140
    > 0x00009c82 airo_get_aplist [airo]: 1140
    > ...
    >
    > In other words, kvm has the top 3 stack users in my kernel. As you can
    > see from my trace above, these things also get called with super-long
    > stacks already. Man. That sucked to find.
    >
    > Avi, how would you like this fixed? I'd be happy to prepare some
    > patches. Do you have a particular approach that you think we should
    > use? Just make the big objects dynamically allocated?
    >

    Yes, things like kvm_lapic_state are way too big to be on the stack.
    There's an additional problem here, that apparently your gcc (which
    version?) doesn't fold objects in a switch statement into the same stack
    slot:

    switch (...) {
    case x: {
    struct medium a;
    ...
    }
    case y:
    struct medium b;
    ...
    }
    };

    These could be solved either by stack allocation, or by moving into
    functions marked noinline. Whichever is easier.

    --
    I have a truly marvellous patch that fixes the bug which this
    signature is too narrow to contain.



    \
     
     \ /
      Last update: 2008-07-17 07:55    [W:0.024 / U:36.396 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site