luciusDXL / TheForceEngine

Modern "Jedi Engine" replacement supporting Dark Forces, mods, and in the future Outlaws.
https://TheForceEngine.github.io
GNU General Public License v2.0
992 stars 72 forks source link

Stack Smashing in Linux Endeavour OS #327

Closed orderorder closed 1 year ago

orderorder commented 1 year ago

[Startup] TFE_System::init [Display] Fullscreen enabled. [Display] Vertical Sync enabled. [RenderBackend] OpenGL Device Tier: 3 [Startup] TFE_AudioSystem::init [Audio] Attempting to initialize Audio System using API 'Linux ALSA'. [Audio] Default Audio Output ID: 0. [Audio] Device Count: 10. [Audio] Device: 'default', ID: 0, Output Channel Count: 64. [Audio] Device: 'hw:HDA ATI HDMI,3', ID: 1, Output Channel Count: 8. [Audio] Device: 'hw:HDA ATI HDMI,7', ID: 2, Output Channel Count: 8. [Audio] Device: 'hw:HDA ATI HDMI,8', ID: 3, Output Channel Count: 8. [Audio] Device: 'hw:HDA ATI HDMI,9', ID: 4, Output Channel Count: 8. [Audio] Device: 'hw:HDA ATI HDMI,10', ID: 5, Output Channel Count: 8. [Audio] Device: 'hw:HDA ATI HDMI,11', ID: 6, Output Channel Count: 2. [Audio] Device: 'hw:HD-Audio Generic,0', ID: 7, Output Channel Count: 6. [Audio] Device: 'hw:HD-Audio Generic,1', ID: 8, Output Channel Count: 2. [Audio] Device: 'hw:HD-Audio Generic,2', ID: 9, Output Channel Count: 0. [Audio] Audio initialized using API 'Linux ALSA'. [Audio] Starting up audio stream for device 'default', id 0. [Startup] TFE_MidiPlayer::init [Startup] TFE_Polygon::init [Startup] TFE_Image::init [Startup] TFE_FrontEndUI::init [MemoryRegion] Allocated new memory block in region 'game' - new size is 1 blocks, total size is '8388608' [MemoryRegion] Allocated new memory block in region 'level' - new size is 1 blocks, total size is '8388608' [Error : a11y] Couldn't find font file at ; falling back to default font stack smashing detected : terminated [Error : CrashHandler] Received Signal 6 errno 10149 code 0 [Error : CrashHandler] Backtrace 9: [Error : CrashHandler] 000 ./theforceengine(+0x1ac419) [0x56484ebaa419] [Error : CrashHandler] 001 /usr/lib/libc.so.6(+0x3e710) [0x7f7d9ba3e710] [Error : CrashHandler] 002 /usr/lib/libc.so.6(+0x8e83c) [0x7f7d9ba8e83c] [Error : CrashHandler] 003 /usr/lib/libc.so.6(raise+0x18) [0x7f7d9ba3e668] [Error : CrashHandler] 004 /usr/lib/libc.so.6(abort+0xd7) [0x7f7d9ba264b8] [Error : CrashHandler] 005 /usr/lib/libc.so.6(+0x27390) [0x7f7d9ba27390] [Error : CrashHandler] 006 /usr/lib/libc.so.6(+0x11f17b) [0x7f7d9bb1f17b] [Error : CrashHandler] 007 /usr/lib/libc.so.6(+0x120486) [0x7f7d9bb20486] [Error : CrashHandler] 008 ./theforceengine(+0x1c447) [0x56484ea1a447] zsh: IOT instruction (core dumped) ./theforceengine

devusb commented 1 year ago

I get the same error on NixOS.

classilla commented 1 year ago

Same on Fedora.

classilla commented 1 year ago

Rebuilding with debug symbols, I get a completely bogus stack trace:

(gdb) bt
#0  0x0000000000000000 in ?? ()
classilla commented 1 year ago

This gets the system up and running, so it appears to be something in accessibility (adding @kevinfoley ).

diff --git a/TheForceEngine/TFE_A11y/accessibility.cpp b/TheForceEngine/TFE_A11y/accessibility.cpp
index b448a4b..7d772af 100644
--- a/TheForceEngine/TFE_A11y/accessibility.cpp
+++ b/TheForceEngine/TFE_A11y/accessibility.cpp
@@ -86,16 +86,17 @@ namespace TFE_A11Y  // a11y is industry slang for accessibility
        static std::vector<Caption> s_exampleCaptions;
        static std::map<string, ImFont*> s_fontMap;
        static ImFont* s_currentCaptionFont;
        static string s_pendingFontPath;

        // Initialize the Accessibility system. Only call this once on application launch.
        void init()
        {
+return;
                assert(s_status == CC_NOT_LOADED);
                CCMD("showCaption", enqueueCaption, 1, "Display a test caption. Example: showCaption \"Hello, world\"");
                CVAR_BOOL(s_logSFXNames, "d_logSFXNames", CVFLAG_DO_NOT_SERIALIZE, "If enabled, log the name of each sound effect that plays.");

                findCaptionFiles();
                findFontFiles();
        }
kevinfoley commented 1 year ago

@classilla Thanks. Please test the possible fix by maxz at https://github.com/luciusDXL/TheForceEngine/pull/331 . I don't have a Linux dev environment set up right now, but I'll work on setting one up.

classilla commented 1 year ago

I have that already applied, but that doesn't fix the crash here.

kevinfoley commented 1 year ago

@classilla Thanks for clarifying. I'll try to get a Fedora dev environment set up in the next few days so that I can troubleshoot this thoroughly. In the meantime, your workaround of simply skipping A11y initialization will work fine as long as you don't try to go into the Accessibility settings menu.

mlauss2 commented 1 year ago

One thing I noticed in accessibility.cpp::143:

            sprintf(programFontsDir, "Fonts/");
            TFE_Paths::mapSystemPath(programFontsDir);

mapSystemPath() expects a path to a file, not a directory. This of course works on windows since a) it's a noop, and b) windows will keep opening files at where you put the binary/what you set as root in visual studio I suppose.

I'll try to find a solution.

mlauss2 commented 1 year ago

The crash is here, at TFE_A11y/accessibility.cpp::216:

TFE_Settings::getA11ySettings()->lastFontPath = path;

It crashes at the string to string assignment. Don't yet know why, on the surface it looks innocent...

mlauss2 commented 1 year ago

This here fixes startup for me:

diff --git a/TheForceEngine/TFE_A11y/accessibility.cpp b/TheForceEngine/TFE_A11y/accessibility.cpp
index 71980ef5..3037b10d 100644
--- a/TheForceEngine/TFE_A11y/accessibility.cpp
+++ b/TheForceEngine/TFE_A11y/accessibility.cpp
@@ -208,12 +208,16 @@ namespace TFE_A11Y  // a11y is industry slang for accessibility
                }
                assert(s_currentCaptionFont != nullptr);

-               char name[256];
-               FileUtil::getFileNameFromPath(path.c_str(), name);
+               char name[TFE_MAX_PATH];
+               char pp[TFE_MAX_PATH];
+               memset(name, 0, TFE_MAX_PATH);
+               memset(pp, 0, TFE_MAX_PATH);
+               strncpy(pp, path.c_str(), TFE_MAX_PATH);
+               FileUtil::getFileNameFromPath(pp, name);
                s_currentFontFile.name = string(name);
                s_currentFontFile.path = path;

-               TFE_Settings::getA11ySettings()->lastFontPath = path;
+               TFE_Settings::getA11ySettings()->lastFontPath = string(pp);
        }
mlauss2 commented 1 year ago

Ok it's a simple buffer overrun: getFileNameFromPath() clears "name" with TFE_MAX_PATH null chars. Fixed thusly:

--- a/TheForceEngine/TFE_A11y/accessibility.cpp
+++ b/TheForceEngine/TFE_A11y/accessibility.cpp
@@ -208,7 +208,7 @@ namespace TFE_A11Y  // a11y is industry slang for accessibility
                }
                assert(s_currentCaptionFont != nullptr);

-               char name[256];
+               char name[TFE_MAX_PATH];
                FileUtil::getFileNameFromPath(path.c_str(), name);
                s_currentFontFile.name = string(name);
                s_currentFontFile.path = path;
orderorder commented 1 year ago

That fix work for me

luciusDXL commented 1 year ago

This should be fixed in master now. Thanks everyone for looking into the issue.