Possible crash in fopen when out of memory (IDFGH-187)

[bernd-edlinger]

at components/newlib/locks.c in function lock_init_generic I see this:

        xSemaphoreHandle new_sem = xQueueCreateMutex(mutex_type);
        if (!new_sem) {
            abort(); /* No more semaphores available or OOM */
        }
        *lock = (_lock_t)new_sem;

This may happen at fopen as following callstack shows:

0x40083eb6: lock_init_generic at /home/ed/esp/esp-idf/components/newlib/./locks.c:79
0x40083fbf: _lock_init_recursive at /home/ed/esp/esp-idf/components/newlib/./locks.c:95
0x400954e0: __sfp at /home/jeroen/esp8266/esp32/newlib_xtensa-2.2.0-bin/newlib_xtensa-2.2.0/xtensa-esp32-elf/newlib/libc/stdio/../../../.././newlib/libc/stdio/findfp.c:145
0x400d3d94: _fopen_r at /home/jeroen/esp8266/esp32/newlib_xtensa-2.2.0-bin/newlib_xtensa-2.2.0/xtensa-esp32-elf/newlib/libc/stdio/../../../.././newlib/libc/stdio/fopen.c:138
0x400d3e54: fopen at /home/jeroen/esp8266/esp32/newlib_xtensa-2.2.0-bin/newlib_xtensa-2.2.0/xtensa-esp32-elf/newlib/libc/stdio/../../../.././newlib/libc/stdio/fopen.c:181
0x40157ecc: mbedtls_pk_load_file at /home/ed/esp/esp-idf/components/mbedtls/library/pkparse.c:81
0x4015d837: mbedtls_x509_crt_parse_file at /home/ed/esp/esp-idf/components/mbedtls/library/x509_crt.c:1093

[igrr]

Newlib does not have support for errors returned from lock routines, so if a lock can not be initialized, the only option is to abort. We can pre-allocate some (large) number of locks at application startup, would such solution work for you?

[bernd-edlinger]

No. My thought was that the port could reserve the necessary space for the fallback in the _lock_t structure, so that it is guaranteed to work out one way or the other.

[bernd-edlinger]

You could even squeeze it in the existing space of a _lock_t like this: (_lock_t)-1 unlocked, (lock_t)-2 locked.

[igrr]

The problem with reserving the space for the fallback structure is that newlib doesn't release the lock until the FILE is closed. So depending on what the application does, it can easily run out of the fallback structure(s). As I said, we can pre-allocate some (configurable) number of lock structures at startup, which will help in some usage patterns.

Reusing the _lock_t variable itself is unlikely to be a workable solution because a single uint32_t does not have enough space to hold something equivalent to a mutex. A spinlock might be an option, but spinlocks don't have event lists, so other tasks can't block on spinlocks. Spinlocks also don't support priority inheritance.

[bernd-edlinger]

It is only a theoretical concern so far, there is absolutely no pressure to do anything about it, especially if it only trades one failure mode against another failure mode.

Of course the best solution would be to patch newlib to have a result from the lock_init_generic function and make the fopen return NULL if it was not possible to create a mutex in the first place.

Another possibility would be to have the space for the mutex inside the _lock_t structure, so the FILE struct gets a bit longer but there is no more memory allocation necessary for the mutex itself.

[drmpf]

I have the same problem I mount the SD card, open two files, write 2K, close two files, unmount SD, end SPI after 3100 cycles I get 0x4008d454: invoke_abort at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/esp32/panic.c line 155 0x4008d685: abort at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/esp32/panic.c line 170 0x40085dcd: lock_init_generic at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/newlib/locks.c line 81 0x40085ee3: _lock_init_recursive at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/newlib/locks.c line 95 0x40145f7f: _fopen_r at ../../../.././newlib/libc/stdio/fopen.c line 138 0x40146021: fopen at ../../../.././newlib/libc/stdio/fopen.c line 181

and the program reboots. the second time though I get 0x4008d454: invoke_abort at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/esp32/panic.c line 155 0x4008d685: abort at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/esp32/panic.c line 170 0x4018bb9b: cxxabiv1::terminate(void ()()) at /builds/idf/crosstool-NG/.build/src/gcc-5.2.0/libstdc++-v3/libsupc++/eh_terminate.cc line 47 0x4018bbe2: std::terminate() at /builds/idf/crosstool-NG/.build/src/gcc-5.2.0/libstdc++-v3/libsupc++/eh_terminate.cc line 57 0x4017d79d: cxxabiv1::cxa_allocate_exception(std::size_t) at /builds/idf/crosstool-NG/.build/src/gcc-5.2.0/libstdc++-v3/libsupc++/eh_alloc.cc line 268 0x4017d4a4: operator new(unsigned int) at /builds/idf/crosstool-NG/.build/src/gcc-5.2.0/libstdc++-v3/libsupc++/new_op.cc line 54 0x400d9217: std::__shared_ptr ::__shared_ptr , VFSImpl&, char const, char const&>(std::_Sp_make_shared_tag, std::allocator const&, VFSImpl&, char const&&, char const*&) at c:\users\matthew\appdata\local\arduino15\packages\esp32\tools\xtensa-esp32-elf-gcc\1.22.0-80-g6c4433a-5.2.0\xtensa-esp32-elf\include\c++\5.2.0\ext/new_allocator.h line 104 again after ! 3100 cycles. Subsequent reboots give the second trace back each time.

My solution is to ditch the ESP32 SD library and replace it with https://github.com/greiman/SdFat using this library not only gives me timestamps but also (more importantly) it does not 'panic' Have tested SdFat upto 50,000 cycles with no problems.

[o-marshmallow]

Closing this issue. If the problem is still relevant, please open a new issue.