daid / EmptyEpsilon

Open source bridge simulator. Build with the SeriousProton engine.
https://daid.github.io/EmptyEpsilon/
GNU General Public License v2.0
529 stars 180 forks source link

Linux server crashes when clients try to connect #1999

Closed jc1bert closed 1 year ago

jc1bert commented 1 year ago

Summary

When trying to connect to a server running on EmptyEpsilon's native linux version, the program segfaults when any client (windows or linux) try to join.

Steps to reproduce

  1. Create a server on linux native version
  2. Try to join with any client
  3. Server segfaults with this error message:
    /usr/include/c++/12.2.1/bits/stl_vector.h:1123: std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::operator[](size_type) [with _Tp = unsigned char; _Alloc = std::allocator<unsigned char>; reference = unsigned char&; size_type = long unsigned int]: Assertion '__n < this->size()' failed.
    [1]    21901 IOT instruction (core dumped)  EmptyEpsilon

System infos

daid commented 1 year ago

Odd, I'll see if I can reproduce, any chance you can manage a stack trace?

aBlueShadow commented 1 year ago

I can't confirm it on MX Linux(Debian based), even when provoking issues by connecting different versions. (Avoided version check by using version number 0 for the server).

I ran the server on master, and was able to successfully connect from 20221029 installed from the .deb (on the same computer) and from a 20230612 build on Android.

Is the issue the same when starting the server on another machine?

jc1bert commented 1 year ago

@aBlueShadow I tried on 3 different computers, and the server crashes on every one of them. Unfortunately, they are all running ArchLinux so it might be a distro related issue. I may have time to fire a VM later this week to try with another distro.

@daid Here is the stack trace:

Process 16663 (EmptyEpsilon) of user 1000 dumped core.
Stack trace of thread 16663:
#0  0x00007f56eea7a26c n/a (libc.so.6 + 0x8926c)
#1  0x00007f56eea2aa08 raise (libc.so.6 + 0x39a08)
#2  0x00007f56eea13538 abort (libc.so.6 + 0x22538)
#3  0x00007f56eecdd3b2 _ZSt21__glibcxx_assert_failPKciS0_S0_ (libstdc++.so.6 + 0xdd3b2)
#4  0x0000565123f665c9 n/a (EmptyEpsilon + 0x43c5c9)
#5  0x0000565123eff704 n/a (EmptyEpsilon + 0x3d5704)
#6  0x0000565123bbddb6 n/a (EmptyEpsilon + 0x93db6)
#7  0x00007f56eea14850 n/a (libc.so.6 + 0x23850)
#8  0x00007f56eea1490a __libc_start_main (libc.so.6 + 0x2390a)
#9  0x0000565123bc5695 n/a (EmptyEpsilon + 0x9b695)

and the backtrace:

(gdb) bt
#0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
#1  0x00007ffff789f2d3 in __pthread_kill_internal (signo=6, threadid=<optimized out>) at pthread_kill.c:78
#2  0x00007ffff784fa08 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#3  0x00007ffff7838538 in __GI_abort () at abort.c:79
#4  0x00007ffff7add3b2 in std::__glibcxx_assert_fail (file=file@entry=0x555555a0f6d0 "/usr/include/c++/12.2.1/bits/stl_vector.h", line=line@entry=1123, 
    function=function@entry=0x555555a10a38 "std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::operator[](size_type) [with _Tp = unsigned char; _Alloc = std::allocator<unsigned char>; reference = unsigned char&; size_type = long unsi"..., condition=condition@entry=0x555555a0f019 "__n < this->size()") at /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/debug.cc:61
#5  0x000055555582ed8a in std::vector<unsigned char, std::allocator<unsigned char> >::operator[] (__n=<optimized out>, this=0x7fffffffcad0) at /usr/include/c++/12.2.1/bits/stl_vector.h:1123
#6  sp::io::DataBuffer::read (s=..., this=<optimized out>) at /usr/src/debug/emptyepsilon/SeriousProton/src/io/dataBuffer.h:190
#7  sp::io::DataBuffer::operator>> (data=..., this=<optimized out>, this=<optimized out>, data=...) at /usr/src/debug/emptyepsilon/SeriousProton/src/io/dataBuffer.h:222
#8  GameServer::update (this=0x55555722d280) at /usr/src/debug/emptyepsilon/SeriousProton/src/multiplayer_server.cpp:248
#9  0x000055555580b67c in Engine::runMainLoop (this=<optimized out>) at /usr/src/debug/emptyepsilon/SeriousProton/src/engine.cpp:163
#10 0x00005555555ee758 in main (argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/emptyepsilon/EmptyEpsilon/src/main.cpp:399

with ArchLinux's packaged version (2022.10.28).

The temporary workaround is to run the server on the windows version through wine.

daid commented 1 year ago

That is odd, it points at this line causing the crash: https://github.com/daid/SeriousProton/blob/master/src/io/dataBuffer.h#L190C55-L190C55 With read_index being beyond the size of the buffer. But the check above that should make that impossible.

I see the problem. What I do is indeed technically "wrong", but only crashes if it's build as debug when asserts in std::vector are enabled.

If the string length is zero, the indexing there is indeed beyond the end of the buffer. But as I'm taking zero bytes from that position I'm not actually accessing the buffer out of bounds.

daid commented 1 year ago

Note that as a client, there is still a slight chance of crashes due to the same bug.

jc1bert commented 1 year ago

I just built v2022.10.28 and v2023.06.17 after patching dataBuffer.h. Server is not crashing anymore when a client connects. Thank you for this fast solve !