search

zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.08k stars 6.19k forks source link

Make CRC for NVS data entries as a default `y` selection #75524

Open omkar3141 opened 4 days ago

omkar3141 commented 4 days ago

Is your enhancement proposal related to a problem? Please describe. NVS backed now supports CRC for data entries as well https://github.com/zephyrproject-rtos/zephyr/pull/73436

Describe the solution you'd like NVS backend now has CRC enabled for metadata (as per older implementation), but CRC for actual data is disabled by default. It now makes sense to enable CRC for data entries as a default, instead of leaving this to user (end app developers would have high change of forgetting this).

Also, in general, it could be considered as a good practice that NVS does not blindly load corrupted data in the application states and prevent misbehavior that would be difficult to debug in the field. If someone does not need this CRC protection for data entries, then they need to make conscious choice to disable this manually.

Thus, recommending, https://github.com/RICCIARDI-Adrien/zephyr/blob/9184e09100ba00f813e707190f6d23d262bb8dc6/subsys/fs/nvs/Kconfig#L32 to be set to y as a default.

Laczen commented 4 days ago

@omkar3141 thank you for creating this RFC. Sadly this cannot be done as it would break the NVS system for existing users.

I am also not convinced that this is the best approach in general: It is straight forward to add a crc to critical settings data by correctly using the settings system. This allows better control and conserves flash space. Other systems e.g. littlefs also don't provide protection of the data, only the metadata is protected.