littlefs-project / littlefs

A little fail-safe filesystem designed for microcontrollers
BSD 3-Clause "New" or "Revised" License
5.26k stars 805 forks source link

lfs_file_open does not assert *next pointer is NULL/a sane value. #617

Open geekbozu opened 3 years ago

geekbozu commented 3 years ago

I ran into an issue while working on a binding from LFS <-> BLE for the Infinitime Firmware for the PineTime smart watch where in release builds my file variables were not getting Zeroed on instantiation. When I close the file opened with a bad *next pointer it caused LFS to behave weird causing crashes, OOB accesses and infinite loops.

The quick and simple fix was to make sure my lfs_file_t variables are zero initialized.

I wonder if LFS should be ensuring that the *next pointer points to a valid value in lfs_file_open, instead of doing nothing with it as it currently seems to do.

Some more technical information.

Opening the file seems to work properly, It can be read/written to just fine. Until I call close. Inside close lfs_dir_commit gets called with the file handle, which then walked *next and proceeds to crash.

geky commented 2 years ago

Hi @geekbozu, thanks for pointing this out. You certainly shouldn't need to zero-initialize the lfs_file_t struct, so it sounds like this is a bug in littlefs.

Given that lfs_file_open always assigns the next point, I'm not sure how this could happen unless the mlist pointer in the lfs_t struct was already corrupted. Did you have any functions return errors before this crash happens?