Add missing allocation failure check

[kleisauke]

This was discovered by libvips in libvips/libvips#5129, which adds a fuzzer that deterministically injects allocation failures.

Details

AddressSanitizer:DEADLYSIGNAL
=================================================================
==9==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000020 (pc 0x7fa653c8d7fc bp 0x7ffdee5f7400 sp 0x7ffdee5f73d0 T0)
==9==The signal is caused by a WRITE memory access.
==9==Hint: address points to the zero page.
SCARINESS: 10 (null-deref)
    #0 0x7fa653c8d7fc in __pthread_mutex_init (/lib/x86_64-linux-gnu/libc.so.6+0x9f7fc) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #1 0x5576ed9a774c in _cmsInitMutexPrimitive /src/lcms/build/../src/lcms2_internal.h:342:12
    #2 0x5576ed9a774c in defMtxCreate /src/lcms/build/../src/cmserr.c:593:5
    #3 0x5576ed9aa07c in cmsCreateProfilePlaceholder /src/lcms/build/../src/cmsio0.c:625:22
    #4 0x5576ed9f41f3 in cmsCreateRGBProfileTHR /src/lcms/build/../src/cmsvirt.c:113:12
    #5 0x5576ed9f5acc in cmsCreateLab4ProfileTHR /src/lcms/build/../src/cmsvirt.c:535:16
    #6 0x5576ed1747d7 in vips_icc_import_build /src/libvips/build/../libvips/colour/icc_transform.c:817:22
    #7 0x5576ecf46dc9 in vips_object_build /src/libvips/build/../libvips/iofuncs/object.c:372:6
    #8 0x5576ecf22b53 in LLVMFuzzerTestOneInput /src/libvips/build/../fuzz/vips_fuzzer.cc:445:8
    #9 0x5576ecdbe2cd in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:619:13
    #10 0x5576ecdbd905 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:516:7
    #11 0x5576ecdbfc42 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__Fuzzer::vector<fuzzer::SizedFile, std::__Fuzzer::allocator<fuzzer::SizedFile>>&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:834:7
    #12 0x5576ecdbff48 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, std::__Fuzzer::allocator<fuzzer::SizedFile>>&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:872:3
    #13 0x5576ecdaede5 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:917:6
    #14 0x5576ecddaa42 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #15 0x7fa653c181c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #16 0x7fa653c1828a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #17 0x5576ecda2134 in _start (/out/vips_oom_fuzzer+0x965134)

[kleisauke]

I also pushed commit 383e13a to this PR, as Icc->UsrMutex might be NULL when malloc() fails.

Details

AddressSanitizer:DEADLYSIGNAL
=================================================================
==9==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000010 (pc 0x7fca1168dff4 bp 0x7ffc668a0310 sp 0x7ffc668a02f8 T0)
==9==The signal is caused by a READ memory access.
==9==Hint: address points to the zero page.
SCARINESS: 10 (null-deref)
    #0 0x7fca1168dff4 in __pthread_mutex_lock (/lib/x86_64-linux-gnu/libc.so.6+0x9fff4) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #1 0x5654a3986c80 in _cmsLockPrimitive /src/lcms/build/../src/lcms2_internal.h:332:12
    #2 0x5654a3986c80 in defMtxLock /src/lcms/build/../src/cmserr.c:609:12
    #3 0x5654a3990f9b in cmsWriteTag /src/lcms/build/../src/cmsio0.c:1922:10
    #4 0x5654a39d3ccd in SetTextTags /src/lcms/build/../src/cmsvirt.c:47:10
    #5 0x5654a39d36a3 in cmsCreateRGBProfileTHR /src/lcms/build/../src/cmsvirt.c:141:10
    #6 0x5654a39d4f1c in cmsCreateLab4ProfileTHR /src/lcms/build/../src/cmsvirt.c:535:16
    #7 0x5654a32a5517 in vips_icc_import_build /src/libvips/build/../libvips/colour/icc_transform.c:817:22
    #8 0x5654a308bb29 in vips_object_build /src/libvips/build/../libvips/iofuncs/object.c:372:6
    #9 0x5654a30678b3 in LLVMFuzzerTestOneInput /src/libvips/build/../fuzz/vips_fuzzer.cc:445:8
    #10 0x5654a2f0302d in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:619:13
    #11 0x5654a2f02665 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:516:7
    #12 0x5654a2f049a2 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__Fuzzer::vector<fuzzer::SizedFile, std::__Fuzzer::allocator<fuzzer::SizedFile>>&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:834:7
    #13 0x5654a2f04ca8 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, std::__Fuzzer::allocator<fuzzer::SizedFile>>&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:872:3
    #14 0x5654a2ef3b45 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:917:6
    #15 0x5654a2f1f7a2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #16 0x7fca116181c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #17 0x7fca1161828a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #18 0x5654a2ee6e94 in _start (/out/vips_oom_fuzzer+0x8d7e94)

[mm2]

Thanks. I am adding your simple check for the alloc case, although this will never happen in real world. The other changes are modifying behavior and/or functionality, so I’ll stick only with the allocation check. fb34320
Thank you!

[kleisauke]

Great, thanks! Indeed, it would likely never happen in the real world.

FWIW, the reason for commit 383e13a was that adding a NULL check here:

Little-CMS/src/cmsio0.c

Lines 624 to 625 in fb34320

	// Create a mutex if the user provided proper plugin. NULL otherwise
	Icc ->UsrMutex = _cmsCreateMutex(ContextID);

is not possible, because cmsPluginMutex can be overridden with a no-op implementation. That said, you're right that such a change would still affect behavior: if malloc() fails, it would silently cause the mutex plugin itself to become a no-op as well.