godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.61k stars 21.28k forks source link

Specific OGG (playable) file triggers stb_vorbis segmentation fault on import #40164

Closed kapes232 closed 4 years ago

kapes232 commented 4 years ago

Godot version: Godot 3.2.2 stable mono official

OS/device including version: Windows 10 [Versión 10.0.18363.900]

Issue description: Dropping an specific OGG loop in res folder after few seconds editor closes, restart editor on importing the asset closes again, deleting the asset starts ok. The OGG file plays correctly, (File is attached)

Steps to reproduce: Dropping again the file editor closes again.

Minimal reproduction project: Dropping OGG file in any project, the editor closes trying importing it.

Farty-McSty.zip

Calinou commented 4 years ago

Related to #38418. It seems like the bug wasn't fixed completely?

kapes232 commented 4 years ago

Related to #38418. It seems like the bug wasn't fixed completely?

In my case it has been solved converting from MP3 to OGG in latest version of Audacity. I have posted this issue to contribute in Godot's stability. I love Godot, and I want to contribute so that it gets better and better.

Thank you.

qarmin commented 4 years ago

Backtrace

thirdparty/misc/stb_vorbis.c:3646:32: runtime error: store to null pointer of type 'char'
handle_crash: Program crashed with signal 11
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[1] godots() [0x14429b0] (/mnt/Miecz/godot3.2/platform/x11/crash_handler_x11.cpp:54)
[2] /lib/x86_64-linux-gnu/libc.so.6(+0x46210) [0x7f3c60cb3210] (??:0)
[3] godots() [0x46dc2ea] (/mnt/Miecz/godot3.2/thirdparty/misc/stb_vorbis.c:3646 (discriminator 3))
[4] godots(stb_vorbis_open_memory+0x289) [0x46fc86d] (/mnt/Miecz/godot3.2/thirdparty/misc/stb_vorbis.c:5081)
[5] AudioStreamOGGVorbis::set_data(PoolVector<unsigned char> const&) (/mnt/Miecz/godot3.2/modules/stb_vorbis/audio_stream_ogg_vorbis.cpp:189)
[6] ResourceImporterOGGVorbis::import(String const&, String const&, Map<StringName, Variant, Comparator<StringName>, DefaultAllocator> const&, List<String, DefaultAllocator>*, List<String, DefaultAllocator>*, Variant*) (/mnt/Miecz/godot3.2/modules/stb_vorbis/resource_importer_ogg_vorbis.cpp:102)
[7] EditorFileSystem::_reimport_file(String const&) (/mnt/Miecz/godot3.2/editor/editor_file_system.cpp:1797)
[8] EditorFileSystem::reimport_files(Vector<String> const&) (/mnt/Miecz/godot3.2/editor/editor_file_system.cpp:1993 (discriminator 3))
[9] EditorFileSystem::_update_scan_actions() (/mnt/Miecz/godot3.2/editor/editor_file_system.cpp:591)
[10] EditorFileSystem::_notification(int) (/mnt/Miecz/godot3.2/editor/editor_file_system.cpp:1174)
[11] EditorFileSystem::_notificationv(int, bool) (/mnt/Miecz/godot3.2/editor/editor_file_system.h:108 (discriminator 14))
[12] Object::notification(int, bool) (/mnt/Miecz/godot3.2/core/object.cpp:934)
[13] SceneTree::_notify_group_pause(StringName const&, int) (/mnt/Miecz/godot3.2/scene/main/scene_tree.cpp:985)
[14] SceneTree::idle(float) (/mnt/Miecz/godot3.2/scene/main/scene_tree.cpp:525 (discriminator 3))
[15] Main::iteration() (/mnt/Miecz/godot3.2/main/main.cpp:2099)
[16] OS_X11::run() (/mnt/Miecz/godot3.2/platform/x11/os_x11.cpp:3233)
[17] godots(main+0x340) [0x143a4e6] (/mnt/Miecz/godot3.2/platform/x11/godot_x11.cpp:57)
[18] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3) [0x7f3c60c940b3] (??:0)
[19] godots(_start+0x2e) [0x143a0ee] (??:?)
-- END OF BACKTRACE --
Przerwane (zrzut pamięci)
projekt2502 commented 4 years ago

I also had this problem, the project worked in 3.2.1, but it didn't work in 3.2.2 because of the OGG file.

akien-mga commented 4 years ago

@RandomShaper More alignment woes in stb_vorbis.c I guess...

Crashes here on line 3646: https://github.com/godotengine/godot/blob/624eff4633bfb847a08e2354d57db429c51d0900/thirdparty/misc/stb_vorbis.c#L3637-L3649

f->comment_list[i] = (char*)setup_malloc(f, sizeof(char) * (len+1)); returns 0x0, while len is 4757.

akien-mga commented 4 years ago

Nope it's not alignment this time, the problem is that we use a small alloc_try size initially (1024), which is meant to be increased in powers of two as-needed when stb_vorbis fails decoding the memory data: https://github.com/godotengine/godot/blob/624eff4633bfb847a08e2354d57db429c51d0900/modules/stb_vorbis/audio_stream_ogg_vorbis.cpp#L163-L183

But https://github.com/nothings/stb/pull/804 added support for parsing OGG comments and does not handle mem alloc in a safe way, so if the buffer is too small, it just happily triggers a segfault without giving us a chance to retry with a bigger alloc size.

Increasing the alloc size is a workaround (the OP's OGG has a comment_list of size 4757, so increasing alloc_try to 8192 solves it, but it's really a stb_vorbis bug that it crashes in the first place, so I'll report it there.

Edit: Upstream issue: https://github.com/nothings/stb/issues/988