GCC untraceable stack corruption

[8infy]

Hi everyone, I'm currently testing syscalls on my OS and have encountered an insane bug.

Heres the syscall handler code:

Code: Select all

auto g_logger = log();

void SyscallDispatcher::dispatch(RegisterState registers)
{
    static size_t index = 0;
    (void)registers;
    
    g_logger << "\rsyscalled: " << ++index;
}

Here I have some dumb test code with a global variable. After this code gets called 4 times, it ends up corrupting the stack and the syscall pop gs instruction fails with a page fault


You would probably say like yeah sure your logger causes the stack corruption after trying to log a number >4. But nope. Changing abosolutely random pieces of this code fixes the stack corruption.
For example:
- adding << "\n" after ++index
- adding random code after g_logger <<, like if (index == 0xFFFFFFFF) index = 0;
- replacing g_logger with log()
- probably a million more things

I think that the actual reason is that GCC decides to do some clever optimization and does something weird with the stack, if that's so how do I make sure this doesn't happen?
I don't actually need this g_logger and it works without it but how many more potential cases of this possibly breaking are there. Also I should mention that dispatch() gets called from a function written in assembly:

Code: Select all

extern "C" void syscall();
asm(".globl syscall\n"                                                                                             
    "syscall:\n"                                                                                                   
    "    pushl $0\n"                                                                                               
    "    pusha\n"                                                                                                  
    "    pushl %ds\n"                                                                                              
    "    pushl %es\n"                                                                                              
    "    pushl %fs\n"                                                                                              
    "    pushl %gs\n"                                                                                              
    "    pushl %ss\n"                                                                                              
    "    mov $0x10, %ax\n"                                                                                         
    "    mov %ax, %ds\n"                                                                                           
    "    mov %ax, %es\n"                                                                                           
    "    cld\n"                                                                                                    
    "    call _ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE\n"                                        
    "    add $0x4, %esp \n"                                                                                        
    "    popl %gs\n"                                                                                               
    "    popl %fs\n"                                                                                               
    "    popl %es\n"                                                                                               
    "    popl %ds\n"                                                                                               
    "    popa\n"                                                                                                   
    "    add $0x4, %esp\n"                                                                                         
    "    iret\n");

I have many copies of this function for each IRQ and exception etc so im sure its fine.

Also if it's a stack corruption why on earth does it only break after the 4th digit...
And if I do static index = 4 it breaks after logging 5

Please help me I really wanna know whats up here

UPD: Heres the dissasembly of the function (broken version)

Code: Select all

00000050 < _ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE > :
50 : 83 ec 0c                sub    $0xc, % esp
53 : 83 3d 04 00 00 00 02    cmpl   $0x2, 0x4
5a : 74 64                   je     c0 < _ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE + 0x70>
5c : 8b 15 00 00 00 00       mov    0x0, % edx
62 : 8b 02                   mov(% edx), % eax
64 : 8b 00                   mov(% eax), % eax
66 : 3d 00 00 00 00          cmp    $0x0, % eax
6b : 75 73                   jne    e0 < _ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE + 0x90>
6d : b9 73 00 00 00          mov    $0x73, % ecx
72 : b8 0d 00 00 00          mov    $0xd, % eax
77 : ba 00 00 00 00          mov    $0x0, % edx
7c : eb 08                   jmp    86 < _ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE + 0x36 >
7e:   66 90                   xchg % ax, % ax
80 : 89 c8                   mov % ecx, % eax
82 : 0f b6 4a 01             movzbl 0x1(% edx), % ecx
86 : 83 c2 01                add    $0x1, % edx
89 : e6 e9                   out % al, $0xe9
8b : 84 c9                   test % cl, % cl
8d : 75 f1                   jne    80 < _ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE + 0x30 >
8f : b8 00 00 00 00          mov    $0x0, % eax
94 : 8b 0d 0c 00 00 00       mov    0xc, % ecx
9a : 89 44 24 10             mov % eax, 0x10(% esp)
9e : 8d 51 01                lea    0x1(% ecx), % edx
a1 : 89 15 0c 00 00 00       mov % edx, 0xc
a7 : 89 54 24 14             mov % edx, 0x14(% esp)
ab : 83 c4 0c                add    $0xc, % esp
ae : e9 fc ff ff ff          jmp    af <_ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE + 0x5f>
b3 : 8d b4 26 00 00 00 00    lea    0x0(% esi, % eiz, 1), % esi
ba : 8d b6 00 00 00 00       lea    0x0(% esi), % esi
c0 : 83 ec 08                sub    $0x8, % esp
c3 : 68 00 00 00 00          push   $0x0
c8 : 68 00 00 00 00          push   $0x0
cd : e8 fc ff ff ff          call   ce <_ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE + 0x7e>
d2 : 83 c4 10                add    $0x10, % esp
d5 : eb bd                   jmp    94 < _ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE + 0x44 >
d7 : 8d b4 26 00 00 00 00    lea    0x0(% esi, % eiz, 1), % esi
de : 66 90                   xchg % ax, % ax
e0 : 83 ec 08                sub    $0x8, % esp
e3 : 68 00 00 00 00          push   $0x0
e8 : 52                      push % edx
e9 : ff d0                   call * %eax
eb : b8 00 00 00 00          mov    $0x0, % eax
f0 : 83 c4 10                add    $0x10, % esp
f3 : eb 9f                   jmp    94 < _ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE + 0x44 >

Working dissasembly (added << "\n" at the end)

Code: Select all

00000050 <_ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE>:
  50:   83 ec 0c                sub    $0xc,%esp
  53:   83 3d 04 00 00 00 02    cmpl   $0x2,0x4
  5a:   0f 84 80 00 00 00       je     e0 <_ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE+0x90>
  60:   8b 15 00 00 00 00       mov    0x0,%edx
  66:   8b 02                   mov    (%edx),%eax
  68:   8b 00                   mov    (%eax),%eax
  6a:   3d 00 00 00 00          cmp    $0x0,%eax
  6f:   0f 85 cb 00 00 00       jne    140 <_ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE+0xf0>
  75:   b9 73 00 00 00          mov    $0x73,%ecx
  7a:   b8 0d 00 00 00          mov    $0xd,%eax
  7f:   ba 00 00 00 00          mov    $0x0,%edx
  84:   eb 10                   jmp    96 <_ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE+0x46>
  86:   8d b4 26 00 00 00 00    lea    0x0(%esi,%eiz,1),%esi
  8d:   8d 76 00                lea    0x0(%esi),%esi
  90:   89 c8                   mov    %ecx,%eax
  92:   0f b6 4a 01             movzbl 0x1(%edx),%ecx
  96:   83 c2 01                add    $0x1,%edx
  99:   e6 e9                   out    %al,$0xe9
  9b:   84 c9                   test   %cl,%cl
  9d:   75 f1                   jne    90 <_ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE+0x40>
  9f:   b8 00 00 00 00          mov    $0x0,%eax
  a4:   8b 0d 0c 00 00 00       mov    0xc,%ecx
  aa:   83 ec 08                sub    $0x8,%esp
  ad:   8d 51 01                lea    0x1(%ecx),%edx
  b0:   52                      push   %edx
  b1:   50                      push   %eax
  b2:   89 15 0c 00 00 00       mov    %edx,0xc
  b8:   e8 fc ff ff ff          call   b9 <_ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE+0x69>
  bd:   83 c4 10                add    $0x10,%esp
  c0:   83 78 04 02             cmpl   $0x2,0x4(%eax)
  c4:   74 3a                   je     100 <_ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE+0xb0>
  c6:   8b 10                   mov    (%eax),%edx
  c8:   8b 02                   mov    (%edx),%eax
  ca:   8b 00                   mov    (%eax),%eax
  cc:   3d 00 00 00 00          cmp    $0x0,%eax
  d1:   75 4d                   jne    120 <_ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE+0xd0>
  d3:   b8 0a 00 00 00          mov    $0xa,%eax
  d8:   e6 e9                   out    %al,$0xe9
  da:   83 c4 0c                add    $0xc,%esp
  dd:   c3                      ret
  de:   66 90                   xchg   %ax,%ax
  e0:   83 ec 08                sub    $0x8,%esp
  e3:   68 00 00 00 00          push   $0x0
  e8:   68 00 00 00 00          push   $0x0
  ed:   e8 fc ff ff ff          call   ee <_ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE+0x9e>
  f2:   83 c4 10                add    $0x10,%esp
  f5:   eb ad                   jmp    a4 <_ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE+0x54>
  f7:   8d b4 26 00 00 00 00    lea    0x0(%esi,%eiz,1),%esi
  fe:   66 90                   xchg   %ax,%ax
 100:   c7 44 24 14 0d 00 00    movl   $0xd,0x14(%esp)
 107:   00
 108:   89 44 24 10             mov    %eax,0x10(%esp)
 10c:   83 c4 0c                add    $0xc,%esp
 10f:   e9 fc ff ff ff          jmp    110 <_ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE+0xc0>
 114:   8d b4 26 00 00 00 00    lea    0x0(%esi,%eiz,1),%esi
 11b:   8d 74 26 00             lea    0x0(%esi,%eiz,1),%esi
 11f:   90                      nop
 120:   c7 44 24 14 0d 00 00    movl   $0xd,0x14(%esp)
 127:   00
 128:   89 54 24 10             mov    %edx,0x10(%esp)
 12c:   83 c4 0c                add    $0xc,%esp
 12f:   ff e0                   jmp    *%eax
 131:   8d b4 26 00 00 00 00    lea    0x0(%esi,%eiz,1),%esi
 138:   8d b4 26 00 00 00 00    lea    0x0(%esi,%eiz,1),%esi
 13f:   90                      nop
 140:   83 ec 08                sub    $0x8,%esp
 143:   68 00 00 00 00          push   $0x0
 148:   52                      push   %edx
 149:   ff d0                   call   *%eax
 14b:   b8 00 00 00 00          mov    $0x0,%eax
 150:   83 c4 10                add    $0x10,%esp
 153:   e9 4c ff ff ff          jmp    a4 <_ZN6kernel17SyscallDispatcher8dispatchENS_13RegisterStateE+0x54>

[iansjack]

1. What happens if you disable optimization?

2. IMO it's a really, really bad idea to use inline assembler for large chunks of code like this. Why not just write the function in an assembly-code file?

[8infy]

1. What happens if you disable optimization?

For some reason at O0 GCC completely discards the inline assembly functions and they all point to 0x00000000

2. IMO it's a really, really bad idea to use inline assembler for large chunks of code like this. Why not just write the function in an assembly-code file?

Yes its horrible, im definitely moving them out now. They were inline as it seemed like a nicer approach at first but gcc doesnt want to cooperate so im gonna move them out.

UPD: Lmao just upgraded to gcc 10.1.0 and it's now working flawlessly... Still wanna know what happened before

[Octocontrabass]

You are passing RegisterState registers by value. Since it is a trivial type, it follows the C ABI rules, which specify that the called function may clobber the stack space used to store the parameters.

Pass a pointer to the data on the stack instead. That way, only the pointer gets clobbered.

[8infy]

You are passing RegisterState registers by value. Since it is a trivial type, it follows the C ABI rules, which specify that the called function may clobber the stack space used to store the parameters.

Pass a pointer to the data on the stack instead. That way, only the pointer gets clobbered.

Now I see, that makes sense, thanks a lot.

Do you know why when I compile it at -O0 it completely breaks as well? (whereas at 02 it works flawlessly) For example the extern "C" functions (defined in global inline assembly) end up being null pointers and some logging looks weird (haven't tested it further since I cant get past the initialization stage because the ISR class complains about someone trying to register a null handler.) I would expect it to be the opposite (e.g optimizations breaking the code) but its vise versa which confuses me even more. I will try to move all the global inline assembly functions out, I hate the GNU assembly syntax anyways, and see if it fixes it.

[Octocontrabass]

I think you would have to look at the generated assembly to see what's happening. I'm not sure if GCC has well-defined behavior when you use inline assembly to specify an entire function. The documentation suggests it's possible to make it work, but discourages its use.

[8infy]

I think you would have to look at the generated assembly to see what's happening. I'm not sure if GCC has well-defined behavior when you use inline assembly to specify an entire function. The documentation suggests it's possible to make it work, but discourages its use.

I see, thank you.

UPD: the weird logging issue was the global log object that I forgot to remove, and the null ISR was solved by moving the inline assembly to the bottom of the file, now it's magically not-null. (for some reason having inline assembly functions at the top of the file turns the first one into a nullptr. I've had to deal with something like this before when I was just starting out.)