drivers: fix CFI MTD array bounds checks#11558
Conversation
Generated-by: OpenAI Codex Signed-off-by: Old-Ding <35417409+Old-Ding@users.noreply.github.com>
|
👋 感谢您对 RT-Thread 的贡献！Thank you for your contribution to RT-Thread! 为确保代码符合 RT-Thread 的编码规范，请在你的仓库中执行以下步骤运行代码格式化工作流（如果格式化CI运行失败）。 🛠 操作步骤 | Steps
完成后，提交将自动更新至 如有问题欢迎联系我们，再次感谢您的贡献！💐 |
📌 Code Review Assignment🏷️ Tag: componentsReviewers: @Maihuanyi Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2026-07-12 03:06 CST)
📝 Review Instructions
|
|
|
The sector-table fix looks correct. For erase regions, moving the bound before the fixup/reverse path also makes sense because cfi_reverse_geometry() consumes query->erase_region_info using query->num_erase_regions before the main region loop. One question: if the device reports more erase regions than the locally configured CFI_QUERY_ERASE_REGIONS_MAX, should probe continue with a truncated geometry, or should it fail? Clamping avoids the out-of-bounds access, but it can also silently describe only part of the flash layout. If partial geometry is acceptable here, the patch looks reasonable; otherwise this should probably return an error after logging. |
A truncated erase-region table leaves the full flash size registered with only part of its sector geometry. Fail probing instead of exposing a device whose erase address lookup can select the wrong sector. Signed-off-by: Old-Ding <35417409+Old-Ding@users.noreply.github.com>
Signed-off-by: Old-Ding <35417409+Old-Ding@users.noreply.github.com>
2 participants
Summary
erase_region_infotable before fixup/reverse logic consumes them.sect_count == RT_ARRAY_SIZE(fdev->sect)as full before writingfdev->sect[sect_count]andfdev->protect[sect_count].Details
query->num_erase_regionscomes from the CFI query data, but the localerase_region_infoarray can be configured withCFI_QUERY_ERASE_REGIONS_MAX. Check the count immediately after detection so later fixup/reverse helpers cannot index beyond the table.The probe now fails when the reported geometry cannot fit. Truncating would leave
fdev->sizedescribing the full flash whilesect[]andsect_countdescribe only the retained regions, allowing erase address lookup to select an incorrect sector.The sector table check currently allows
sect_count == RT_ARRAY_SIZE(fdev->sect), then writes through that index. This changes the guard to>=so the arrays are treated as full before the out-of-bounds write.Testing
git diff --checkb63da3b16passed format/license, static analysis, package tests, the full unit-test and BSP matrices, memory reports, and CLA; only expected helper jobs were skipped.