littlefs-project / littlefs

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

Critical Logic Bug in lfs_file_close_ Memory Release #1037

Closed qnix-code closed 3 weeks ago

qnix-code commented 3 weeks ago

There is an obvious logic bug in the lfs_fileclose function regarding memory release.

In this function, the current code for memory release has a major flaw. The lfs_fileclose function is supposed to be a method for cleaning up resources. Even if there are external dependencies, it is still a bug that the current code only attempts to free file->cache.buffer when !file->cfg->buffer. This inconsistent and potentially incomplete memory release behavior can lead to memory leaks and incorrect resource management.

Here is the relevant code:

// clean up memory
if (!file->cfg->buffer) {
    lfs_free(file->cache.buffer);
}

This needs to be fixed to ensure proper and complete resource cleanup during the lfs_fileclose operation.

It should be noted that this bug still exists in the master branch of the current version.

geky commented 3 weeks ago

Hi @qnix-code, thanks for creating an issue

Note that this logic matches the logic used for allocation: https://github.com/littlefs-project/littlefs/blob/d01280e64934a09ba16cac60cf9d3a37e228bb66/lfs.c#L3153-L3162

file->cfg->buffer (via lfs_file_opencfg) allows users to optionally provide their own memory for the file's buffer (file->cache.buffer). This memory is up to the user to manage, and usually just stored in global memory, so attempting to free it would cause Bad Things™ to happen.

qnix-code commented 3 weeks ago

Hi @geky, thanks for reply. My mistake. I misunderstood. Initially, when I was debugging, I found that free would be passed NULL. In my implementation of the free function, I didn't check for NULL, which caused the program to crash.