From 2585b7beea85804df355ffa97c83db3a7c0e4d92 Mon Sep 17 00:00:00 2001
From: Jerzy Kasenberg <jerzy.kasenberg@codecoup.pl>
Date: Wed, 7 Aug 2024 13:57:17 +0200
Subject: [PATCH] fs/fcb: Clean fcb_append inconsistency

Variable cnt and len were reused leading to some inconsistencies.

1.
cnt was number of bytes needed to store len but it was later
changed to store number of bytes in flash to store len (that
can be larger when flash alignment restrictions are applied).
Later it was used to actually write len filed to flash, however
if alignment was not 1 or 2 some random data would be written to
flash (starting from tmp_str but extending to other values on stack).
No there is separate variable to keep track how many bytes in
flash are needed.

2.
calculating whether entry will fit in current sector,
fcb_disk_area was not aligned correctly that could lead to
problems for flash that has alignment that is higher then 8.

3.
append_loc was partially updated but fe_data_len was never filled
leaving structure in inconsistent state.

4.
active->fe_data_len could have incorrect value (rounded up to alignment)
This was small inconsistency that probably would not result in any
problem

Signed-off-by: Jerzy Kasenberg <jerzy.kasenberg@codecoup.pl>
---
 fs/fcb/src/fcb_append.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/fs/fcb/src/fcb_append.c b/fs/fcb/src/fcb_append.c
index 4b8d39e5bc..29eedb70ee 100644
--- a/fs/fcb/src/fcb_append.c
+++ b/fs/fcb/src/fcb_append.c
@@ -86,24 +86,26 @@ fcb_append(struct fcb *fcb, uint16_t len, struct fcb_entry *append_loc)
     struct flash_area *fa;
     uint8_t tmp_str[2];
     int cnt;
+    int len_bytes_in_flash;
+    int entry_len_in_flash;
     int rc;
 
     cnt = fcb_put_len(tmp_str, len);
     if (cnt < 0) {
         return cnt;
     }
-    cnt = fcb_len_in_flash(fcb, cnt);
-    len = fcb_len_in_flash(fcb, len) + fcb_len_in_flash(fcb, FCB_CRC_SZ);
+    len_bytes_in_flash = fcb_len_in_flash(fcb, cnt);
+    entry_len_in_flash = fcb_entry_total_len(fcb, len);
 
     rc = os_mutex_pend(&fcb->f_mtx, OS_WAIT_FOREVER);
     if (rc && rc != OS_NOT_STARTED) {
         return FCB_ERR_ARGS;
     }
     active = &fcb->f_active;
-    if (active->fe_elem_off + len + cnt > active->fe_area->fa_size) {
+    if (active->fe_elem_off + entry_len_in_flash > active->fe_area->fa_size) {
         fa = fcb_new_area(fcb, fcb->f_scratch_cnt);
         if (!fa || (fa->fa_size <
-            sizeof(struct fcb_disk_area) + len + cnt)) {
+                    fcb_len_in_flash(fcb, sizeof(struct fcb_disk_area)) + entry_len_in_flash)) {
             rc = FCB_ERR_NOSPACE;
             goto err;
         }
@@ -123,9 +125,10 @@ fcb_append(struct fcb *fcb, uint16_t len, struct fcb_entry *append_loc)
     }
     append_loc->fe_area = active->fe_area;
     append_loc->fe_elem_off = active->fe_elem_off;
-    append_loc->fe_data_off = active->fe_elem_off + cnt;
+    append_loc->fe_data_off = active->fe_elem_off + len_bytes_in_flash;
+    append_loc->fe_data_len = len;
 
-    active->fe_elem_off = append_loc->fe_data_off + len;
+    active->fe_elem_off = append_loc->fe_data_off + entry_len_in_flash;
     active->fe_data_off = append_loc->fe_data_off;
     active->fe_data_len = len;