LongSoft / UEFITool

UEFI firmware image viewer and editor
BSD 2-Clause "Simplified" License
4.47k stars 632 forks source link

Seg fault with qt6 + new_engine #336

Closed 4e4o closed 1 year ago

4e4o commented 1 year ago

With qt6 i have segfault here According to c++ strict aliasing rules type casting from byte array to char16_t is UB. In most cases it Ok, but as you can see not always. In x86 case you must ensure that char16_t* points to address aligned to 2. Here is my opened issue on qt bug tracker https://bugreports.qt.io/browse/QTBUG-110741

Screenshot_20230131_041551

NikolajSchlej commented 1 year ago

Nice find, C++ remains to be fun. The underlying data in unaligned by design, so we'll have to work around that by making copies or use different API calls.

NikolajSchlej commented 1 year ago

Also, I bet there's a metric shitton of other UB cases, makes sense to start using UBSAN to catch them.

NikolajSchlej commented 1 year ago

Could you share the exact environment, compiler version and input file, so I could reproduce the crash locally?

4e4o commented 1 year ago

Could you share the exact environment, compiler version and input file, so I could reproduce the crash locally?

Sure.

fedora 37 x86_64 uname -a Linux mobile 6.1.7-200.fc37.x86_64 #1 SMP PREEMPT_DYNAMIC Wed Jan 18 17:11:49 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux checked clang and gcc, same behaviour clang version 15.0.7 (Fedora 15.0.7-1.fc37) gcc version 12.2.1 20221121 (Red Hat 12.2.1-4) (GCC)

With qt5 this crash is not happens. To force qt5 usage i replace qt6 with qt5 in CMakeLists.txt.

You can simply run minimal example from here. Also fw.bin attached.
01966.bin.zip

NikolajSchlej commented 1 year ago

Reproduced, and added a Clang Qt6 build with all sanitizers enabled to CI/CD, so anybody else could reproduce it too (on Linux, at least).

0x55b3d9106661: note: pointer points here
 00 00 00  24 56 53 53 ff ff ff ff  5a fe 00 00 00 00 00 00  ff ff ff ff ff ff ff ff  ff ff ff ff ff
              ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:542:13 in 
/home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:542:57: runtime error: load of misaligned address 0x55b3d9106661 for type 'const UINT32' (aka 'const unsigned int'), which requires 4 byte alignment
0x55b3d9106661: note: pointer points here
 00 00 00  24 56 53 53 ff ff ff ff  5a fe 00 00 00 00 00 00  ff ff ff ff ff ff ff ff  ff ff ff ff ff
              ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:542:57 in 
/home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:542:107: runtime error: load of misaligned address 0x55b3d9106661 for type 'const UINT32' (aka 'const unsigned int'), which requires 4 byte alignment
0x55b3d9106661: note: pointer points here
 00 00 00  24 56 53 53 ff ff ff ff  5a fe 00 00 00 00 00 00  ff ff ff ff ff ff ff ff  ff ff ff ff ff
              ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:542:107 in 
/home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:555:18: runtime error: load of misaligned address 0x55b3d9106661 for type 'const UINT32' (aka 'const unsigned int'), which requires 4 byte alignment
0x55b3d9106661: note: pointer points here
 00 00 00  24 56 53 53 ff ff ff ff  5a fe 00 00 00 00 00 00  ff ff ff ff ff ff ff ff  ff ff ff ff ff
              ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:555:18 in 
/home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:555:80: runtime error: load of misaligned address 0x55b3d9106661 for type 'const UINT32' (aka 'const unsigned int'), which requires 4 byte alignment
0x55b3d9106661: note: pointer points here
 00 00 00  24 56 53 53 ff ff ff ff  5a fe 00 00 00 00 00 00  ff ff ff ff ff ff ff ff  ff ff ff ff ff
              ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:555:80 in 
/home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:572:18: runtime error: load of misaligned address 0x55b3d9106661 for type 'const UINT32' (aka 'const unsigned int'), which requires 4 byte alignment
0x55b3d9106661: note: pointer points here
 00 00 00  24 56 53 53 ff ff ff ff  5a fe 00 00 00 00 00 00  ff ff ff ff ff ff ff ff  ff ff ff ff ff
              ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:572:18 in 
/home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:581:18: runtime error: load of misaligned address 0x55b3d9106661 for type 'const UINT32' (aka 'const unsigned int'), which requires 4 byte alignment
0x55b3d9106661: note: pointer points here
 00 00 00  24 56 53 53 ff ff ff ff  5a fe 00 00 00 00 00 00  ff ff ff ff ff ff ff ff  ff ff ff ff ff
              ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:581:18 in 
/home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:581:69: runtime error: load of misaligned address 0x55b3d9106661 for type 'const UINT32' (aka 'const unsigned int'), which requires 4 byte alignment
0x55b3d9106661: note: pointer points here
 00 00 00  24 56 53 53 ff ff ff ff  5a fe 00 00 00 00 00 00  ff ff ff ff ff ff ff ff  ff ff ff ff ff
              ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:581:69 in 
/home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:590:18: runtime error: load of misaligned address 0x55b3d9106661 for type 'const UINT32' (aka 'const unsigned int'), which requires 4 byte alignment
0x55b3d9106661: note: pointer points here
 00 00 00  24 56 53 53 ff ff ff ff  5a fe 00 00 00 00 00 00  ff ff ff ff ff ff ff ff  ff ff ff ff ff
              ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:590:18 in 
/home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:607:18: runtime error: load of misaligned address 0x55b3d9106661 for type 'const UINT32' (aka 'const unsigned int'), which requires 4 byte alignment
0x55b3d9106661: note: pointer points here
 00 00 00  24 56 53 53 ff ff ff ff  5a fe 00 00 00 00 00 00  ff ff ff ff ff ff ff ff  ff ff ff ff ff
              ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:607:18 in 
/home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:607:71: runtime error: load of misaligned address 0x55b3d9106661 for type 'const UINT32' (aka 'const unsigned int'), which requires 4 byte alignment
0x55b3d9106661: note: pointer points here
 00 00 00  24 56 53 53 ff ff ff ff  5a fe 00 00 00 00 00 00  ff ff ff ff ff ff ff ff  ff ff ff ff ff
              ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:607:71 in 
/home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:633:18: runtime error: load of misaligned address 0x55b3d9106661 for type 'const UINT32' (aka 'const unsigned int'), which requires 4 byte alignment
0x55b3d9106661: note: pointer points here
 00 00 00  24 56 53 53 ff ff ff ff  5a fe 00 00 00 00 00 00  ff ff ff ff ff ff ff ff  ff ff ff ff ff
              ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:633:18 in 
/home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:641:18: runtime error: load of misaligned address 0x55b3d9106661 for type 'const UINT32' (aka 'const unsigned int'), which requires 4 byte alignment
0x55b3d9106661: note: pointer points here
 00 00 00  24 56 53 53 ff ff ff ff  5a fe 00 00 00 00 00 00  ff ff ff ff ff ff ff ff  ff ff ff ff ff
              ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:641:18 in 
/home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:651:18: runtime error: load of misaligned address 0x55b3d9106661 for type 'const UINT32' (aka 'const unsigned int'), which requires 4 byte alignment
0x55b3d9106661: note: pointer points here
 00 00 00  24 56 53 53 ff ff ff ff  5a fe 00 00 00 00 00 00  ff ff ff ff ff ff ff ff  ff ff ff ff ff
              ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:651:18 in 
/home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:662:18: runtime error: load of misaligned address 0x55b3d9106661 for type 'const UINT32' (aka 'const unsigned int'), which requires 4 byte alignment
0x55b3d9106661: note: pointer points here
 00 00 00  24 56 53 53 ff ff ff ff  5a fe 00 00 00 00 00 00  ff ff ff ff ff ff ff ff  ff ff ff ff ff
              ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:662:18 in 
/home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:675:18: runtime error: load of misaligned address 0x55b3d9106661 for type 'const UINT32' (aka 'const unsigned int'), which requires 4 byte alignment
0x55b3d9106661: note: pointer points here
 00 00 00  24 56 53 53 ff ff ff ff  5a fe 00 00 00 00 00 00  ff ff ff ff ff ff ff ff  ff ff ff ff ff
              ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:675:18 in 
UndefinedBehaviorSanitizer:DEADLYSIGNAL
==4477==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address (pc 0x7f117b7d82e9 bp 0x7ffe778d17e0 sp 0x7ffe778d09d8 T4477)
==4477==The signal is caused by a READ memory access.
==4477==Hint: this fault was caused by a dereference of a high value address (see register values below).  Disassemble the provided pc to learn which register was used.
    #0 0x7f117b7d82e9 in QtPrivate::qustrlen(char16_t const*) (/lib/x86_64-linux-gnu/libQt6Core.so.6+0x2302e9) (BuildId: 10c2c7ccc13f5d4a41be5530fed7514a09239f8d)
    #1 0x7f117b7e4017 in QString::fromUtf16(char16_t const*, long long) (/lib/x86_64-linux-gnu/libQt6Core.so.6+0x23c017) (BuildId: 10c2c7ccc13f5d4a41be5530fed7514a09239f8d)
    #2 0x55b3d65a3662 in NvramParser::parseVssStoreBody(QModelIndex const&, unsigned char) /home/runner/work/UEFITool/UEFITool/common/nvramparser.cpp:1509:20

The fix will be fairly trivial: use already exiting readUnaligned function in more places.

NikolajSchlej commented 1 year ago

Should be fixed by https://github.com/LongSoft/UEFITool/commit/c9939e23ec7c758166ed020f5e410a67e846dc0c

4e4o commented 1 year ago

Yep, checked, no more sigsegv.