alexcekay opened a new pull request, #19811: URL: https://github.com/apache/nuttx/pull/19811
## Summary cromfs_read()'s fast path decompresses a block directly into the caller's buffer whenever a read reaches a block at its start and the caller has room for the whole decompressed block, bypassing the per-file decompression cache (ff_buffer). It nonetheless marked that block as cached by setting ff_offset, without ever writing ff_buffer itself. A later read of the same block that fell onto the slow path trusted that false cache tag, skipped decompression, and copied from ff_buffer without it ever having been populated for that block. A repeated identical fast-path read of the same block hit the same false tag and skipped decompression entirely, leaving the caller's buffer untouched and returning whatever was already there. Fixed by having the fast path only read the cache, never populate it: reuse ff_buffer when a prior slow-path read already cached the same block, otherwise decompress straight into the caller's buffer without touching ff_offset/ff_buffer. ## Detection - We detected this in PX4 mavftp server which serves files from the CROMFS. It was only detected now as the radio link was quite lossy, which caused the ground station to request re-reads for the same file offset a second time due to loss on the link. - This caused a re-read on thus an access to the "poisoned" cache, which in-turn lead to the read not changing the user buffer at all, which caused the previous data delivered via mavftp to be delivered again. ## Impact - This does not have a functional impact on existing users, CROMFS behavior is fixed with this change. - The fast-path cache will not be used anymore which may have a performance impact, but as this caching was not safe it's the better trade-off. As it was not reported yet I assume the cache path was never hit by other users. ## Testing - Tested in the context of PX4 using a STM32H743VIH6 - Test setup is a generated ROMFS (https://gist.github.com/alexcekay/03a240112dbdaf44e4ad374ffa12142b) which contains 2 compressed blocks (512 byte uncompressed each), 2 non-compressed blocks (512 byte uncompressed each) and one tail block (77 byte uncompresssed) - Test is a PX4 systemcmd that was executed (https://gist.github.com/alexcekay/404f42702edf5535bb563d24e49f2e0c). - Description of individual tests: - `file_size`: Use `stat` to check that CROMFS reports correct file size - `full_read`: Read the whole CROMFS file into one buffer and check that it contains the expected data - `chunked_read`: Read file with different chunk sizes and check the chunks contain the expected data - `seek_and_reread`: Seek to various offsets and do a chunked read. Check that chunks contain the expected data - `regression_double_visit`: Read using the fast path, afterwards using the slow path and afterwards using the fast path again. Checks that the fast path correctly uses the cache set by the slow path - `regression_repeated_fast_path`: Read the same block twice using the fast path. Ensure that this does not cause cache problems - `eof_handling`: Read and seek near EOF **Test result without fix:** ``` nsh> cromfs_test [file_size] PASS [full_read] PASS [chunked_read] PASS ERROR [cromfs_test] cromfs: mismatch at file offset 2560: got 0x61 expected 0x6f ERROR [cromfs_test] cromfs: mismatch after seek to 2560 [seek_and_reread] FAIL ERROR [cromfs_test] cromfs: mismatch at file offset 2560: got 0xec expected 0x6f [regression_double_visit] FAIL ERROR [cromfs_test] cromfs: mismatch at file offset 2560: got 0xa5 expected 0x6f ERROR [cromfs_test] cromfs: REGRESSION - repeated fast-path read of the same block returned stale/unwritten data instead of de [regression_repeated_fast_path] FAIL [eof_handling] PASS ERROR [cromfs_test] cromfs_test FAILED ``` **Test result with fix:** ``` nsh> cromfs_test [file_size] PASS [full_read] PASS [chunked_read] PASS [seek_and_reread] PASS [regression_double_visit] PASS [regression_repeated_fast_path] PASS [eof_handling] PASS INFO [cromfs_test] cromfs_test PASSED ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
