[nrf fromtree] fs: nvs: Add CRC-32 to protect data

Allow to protect the data part of each NVS item with a 32-bit CRC.
This uses 4 more bytes per NVS item.

Signed-off-by: Adrien Ricciardi <[email protected]>
(cherry picked from commit b76d630)
Signed-off-by: Andrzej Głąbek <[email protected]>

Author: RICCIARDI-Adrien

subsys/fs/nvs/Kconfig

@@ -29,6 +29,14 @@ config NVS_LOOKUP_CACHE_SIZE
 	  Number of entries in Non-volatile Storage lookup cache.
 	  It is recommended that it be a power of 2.
 
+config NVS_DATA_CRC
+	bool "Non-volatile Storage CRC protection on the data"
+	help
+	  Enable a CRC-32 on the data part of each NVS element.
+	  The ATE CRC is not impacted by this feature and stays the same.
+	  The CRC-32 is transparently stored at the end of the data field,
+	  in the NVS data section, so 4 more bytes are needed per NVS element.
+
 module = NVS
 module-str = nvs
 source "subsys/logging/Kconfig.template.log_config"

subsys/fs/nvs/nvs.c

@@ -175,11 +175,44 @@ static int nvs_flash_ate_wrt(struct nvs_fs *fs, const struct nvs_ate *entry)
 }
 
 /* data write */
-static int nvs_flash_data_wrt(struct nvs_fs *fs, const void *data, size_t len)
+static int nvs_flash_data_wrt(struct nvs_fs *fs, const void *data, size_t len, bool compute_crc)
 {
 	int rc;
 
-	rc = nvs_flash_al_wrt(fs, fs->data_wra, data, len);
+	/* Only add the CRC if required (ignore deletion requests, i.e. when len is 0) */
+	if (IS_ENABLED(CONFIG_NVS_DATA_CRC) && compute_crc && (len > 0)) {
+		size_t aligned_len, data_len = len;
+		uint8_t *data8 = (uint8_t *)data, buf[NVS_BLOCK_SIZE + NVS_DATA_CRC_SIZE], *pbuf;
+		uint32_t data_crc;
+
+		/* Write as much aligned data as possible, so the CRC can be concatenated at
+		 * the end of the unaligned data later
+		 */
+		aligned_len = len & ~(fs->flash_parameters->write_block_size - 1U);
+		rc = nvs_flash_al_wrt(fs, fs->data_wra, data8, aligned_len);
+		fs->data_wra += aligned_len;
+		if (rc) {
+			return rc;
+		}
+		data8 += aligned_len;
+		len -= aligned_len;
+
+		/* Create a buffer with the unaligned data if any */
+		pbuf = buf;
+		if (len) {
+			memcpy(pbuf, data8, len);
+			pbuf += len;
+		}
+
+		/* Append the CRC */
+		data_crc = crc32_ieee(data, data_len);
+		memcpy(pbuf, &data_crc, sizeof(data_crc));
+		len += sizeof(data_crc);
+
+		rc = nvs_flash_al_wrt(fs, fs->data_wra, buf, len);
+	} else {
+		rc = nvs_flash_al_wrt(fs, fs->data_wra, data, len);
+	}
 	fs->data_wra += nvs_al_size(fs, len);
 
 	return rc;
@@ -279,7 +312,10 @@ static int nvs_flash_block_move(struct nvs_fs *fs, uint32_t addr, size_t len)
 		if (rc) {
 			return rc;
 		}
-		rc = nvs_flash_data_wrt(fs, buf, bytes_to_copy);
+		/* Just rewrite the whole record, no need to recompute the CRC as the data
+		 * did not change
+		 */
+		rc = nvs_flash_data_wrt(fs, buf, bytes_to_copy, false);
 		if (rc) {
 			return rc;
 		}
@@ -348,7 +384,6 @@ static int nvs_ate_crc8_check(const struct nvs_ate *entry)
 /* nvs_ate_cmp_const compares an ATE to a constant value. returns 0 if
  * the whole ATE is equal to value, 1 if not equal.
  */
-
 static int nvs_ate_cmp_const(const struct nvs_ate *entry, uint8_t value)
 {
 	const uint8_t *data8 = (const uint8_t *)entry;
@@ -416,12 +451,19 @@ static int nvs_flash_wrt_entry(struct nvs_fs *fs, uint16_t id, const void *data,
 	entry.len = (uint16_t)len;
 	entry.part = 0xff;
 
-	nvs_ate_crc8_update(&entry);
-
-	rc = nvs_flash_data_wrt(fs, data, len);
+	rc = nvs_flash_data_wrt(fs, data, len, true);
 	if (rc) {
 		return rc;
 	}
+
+#ifdef CONFIG_NVS_DATA_CRC
+	/* No CRC has been added if this is a deletion write request */
+	if (len > 0) {
+		entry.len += NVS_DATA_CRC_SIZE;
+	}
+#endif
+	nvs_ate_crc8_update(&entry);
+
 	rc = nvs_flash_ate_wrt(fs, &entry);
 	if (rc) {
 		return rc;
@@ -1030,8 +1072,10 @@ ssize_t nvs_write(struct nvs_fs *fs, uint16_t id, const void *data, size_t len)
 	/* The maximum data size is sector size - 4 ate
 	 * where: 1 ate for data, 1 ate for sector close, 1 ate for gc done,
 	 * and 1 ate to always allow a delete.
+	 * Also take into account the data CRC that is appended at the end of the data field,
+	 * if any.
 	 */
-	if ((len > (fs->sector_size - 4 * ate_size)) ||
+	if ((len > (fs->sector_size - 4 * ate_size - NVS_DATA_CRC_SIZE)) ||
 	    ((len > 0) && (data == NULL))) {
 		return -EINVAL;
 	}
@@ -1080,10 +1124,10 @@ ssize_t nvs_write(struct nvs_fs *fs, uint16_t id, const void *data, size_t len)
 				 */
 				return 0;
 			}
-		} else if (len == wlk_ate.len) {
+		} else if (len + NVS_DATA_CRC_SIZE == wlk_ate.len) {
 			/* do not try to compare if lengths are not equal */
 			/* compare the data and if equal return 0 */
-			rc = nvs_flash_block_cmp(fs, rd_addr, data, len);
+			rc = nvs_flash_block_cmp(fs, rd_addr, data, len + NVS_DATA_CRC_SIZE);
 			if (rc <= 0) {
 				return rc;
 			}
@@ -1098,7 +1142,7 @@ ssize_t nvs_write(struct nvs_fs *fs, uint16_t id, const void *data, size_t len)
 	/* calculate required space if the entry contains data */
 	if (data_size) {
 		/* Leave space for delete ate */
-		required_space = data_size + ate_size;
+		required_space = data_size + ate_size + NVS_DATA_CRC_SIZE;
 	}
 
 	k_mutex_lock(&fs->nvs_lock, K_FOREVER);
@@ -1153,6 +1197,9 @@ ssize_t nvs_read_hist(struct nvs_fs *fs, uint16_t id, void *data, size_t len,
 	uint16_t cnt_his;
 	struct nvs_ate wlk_ate;
 	size_t ate_size;
+#ifdef CONFIG_NVS_DATA_CRC
+	uint32_t read_data_crc, computed_data_crc;
+#endif
 
 	if (!fs->ready) {
 		LOG_ERR("NVS not initialized");
@@ -1198,14 +1245,40 @@ ssize_t nvs_read_hist(struct nvs_fs *fs, uint16_t id, void *data, size_t len,
 		return -ENOENT;
 	}
 
+#ifdef CONFIG_NVS_DATA_CRC
+	/* When data CRC is enabled, there should be at least the CRC stored in the data field */
+	if (wlk_ate.len < NVS_DATA_CRC_SIZE) {
+		return -ENOENT;
+	}
+#endif
+
 	rd_addr &= ADDR_SECT_MASK;
 	rd_addr += wlk_ate.offset;
-	rc = nvs_flash_rd(fs, rd_addr, data, MIN(len, wlk_ate.len));
+	rc = nvs_flash_rd(fs, rd_addr, data, MIN(len, wlk_ate.len - NVS_DATA_CRC_SIZE));
 	if (rc) {
 		goto err;
 	}
 
-	return wlk_ate.len;
+	/* Check data CRC (only if the whole element data has been read) */
+#ifdef CONFIG_NVS_DATA_CRC
+	if (len >= (wlk_ate.len - NVS_DATA_CRC_SIZE)) {
+		rd_addr += wlk_ate.len - NVS_DATA_CRC_SIZE;
+		rc = nvs_flash_rd(fs, rd_addr, &read_data_crc, sizeof(read_data_crc));
+		if (rc) {
+			goto err;
+		}
+
+		computed_data_crc = crc32_ieee(data, wlk_ate.len - NVS_DATA_CRC_SIZE);
+		if (read_data_crc != computed_data_crc) {
+			LOG_ERR("Invalid data CRC: read_data_crc=0x%08X, computed_data_crc=0x%08X",
+				read_data_crc, computed_data_crc);
+			rc = -EIO;
+			goto err;
+		}
+	}
+#endif
+
+	return wlk_ate.len - NVS_DATA_CRC_SIZE;
 
 err:
 	return rc;

subsys/fs/nvs/nvs_priv.h

@@ -30,6 +30,15 @@ extern "C" {
 
 #define NVS_LOOKUP_CACHE_NO_ADDR 0xFFFFFFFF
 
+/*
+ * Allow to use the NVS_DATA_CRC_SIZE macro in computations whether data CRC is enabled or not
+ */
+#ifdef CONFIG_NVS_DATA_CRC
+#define NVS_DATA_CRC_SIZE 4 /* CRC-32 size in bytes */
+#else
+#define NVS_DATA_CRC_SIZE 0
+#endif
+
 /* Allocation Table Entry */
 struct nvs_ate {
 	uint16_t id;	/* data id */