Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VFS/LittleFS: Incorrect cache size validation causes error when default cache size /lookaside buffer size usage is expected #82918

Closed
de-nordic opened this issue Dec 12, 2024 · 0 comments · Fixed by #82880
Assignees
Labels
bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug Regression Something, which was working, does not anymore
Milestone

Comments

@de-nordic
Copy link
Collaborator

Describe the bug

Commit 09574e6 has introduced cache and lookaside buffer validation that is supposed to error out in case when cache size is not aligned to device block size.
The problem with validation is that is directly tries to compare LittleFS instance stored sizes which may have value of 0 at this point; the value 0 in instance is possible as LittleFS initialization contains mechanism that defaults the instance value to system wide setting passed as Kconfig value.
The problem here is that during validation, the instance value is cached into test variable and corrected if 0:

lfs_size_t cache_size = lcp->cache_size;
if (cache_size == 0) {
cache_size = CONFIG_FS_LITTLEFS_CACHE_SIZE;
}
lfs_size_t lookahead_size = lcp->lookahead_size;
if (lookahead_size == 0) {
lookahead_size = CONFIG_FS_LITTLEFS_LOOKAHEAD_SIZE;
}

but check for validity once again tests against not yet modified instance value:

if (lcp->cache_size < new_cache_size) {
LOG_ERR("Configured cache size is too small: %d < %d", lcp->cache_size,
new_cache_size);
return -ENOMEM;
}
lcp->cache_size = new_cache_size;
if (lcp->lookahead_size < new_lookahead_size) {
LOG_ERR("Configured lookahead size is too small: %d < %d",
lcp->lookahead_size, new_lookahead_size);
return -ENOMEM;
}
lcp->lookahead_size = new_lookahead_size;

since the new_cache_size is calculated from block_size it will always be > 0; it has already been established that instance value can be 0, to pick default, but it has not yet been corrected, but the cache_size is already corrected in lines 780-790.
So the test here should check if cache_size/lookahead_size are requesting at least what is calculated from block size.

To Reproduce
It is enough to set cache-size parameter in DTS LittleFS instance or set cache_size of littlefs_cfg, while defining LittleFS data for fs_mount, to 0 to immediate see initialization error.

Expected behavior
Error should only appear in case when set cache size, either directly or default, is not enough to cover device block size.

Impact
Default LittleFS cache and lookaside buffer does not work.

Environment (please complete the following information):

  • OS: Any
  • Toolchain Zephyr sdk 0.16.3
  • Commit SHA ae7e852

Additional context
Affects 4.0.0.
No proper tests for default values to detect the problem.

@de-nordic de-nordic added the bug The issue is a bug, or the PR is fixing a bug label Dec 12, 2024
@de-nordic de-nordic added this to the v4.1.0 milestone Dec 12, 2024
@de-nordic de-nordic added Regression Something, which was working, does not anymore priority: medium Medium impact/importance bug labels Dec 12, 2024
@de-nordic de-nordic self-assigned this Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug Regression Something, which was working, does not anymore
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant