Skip to content

Commit

Permalink
fs/fcb: Clean fcb_append inconsistency
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
kasjer committed Sep 10, 2024
1 parent 2fdc30a commit 36015a0
Showing 1 changed file with 9 additions and 6 deletions.
15 changes: 9 additions & 6 deletions fs/fcb/src/fcb_append.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -124,9 +126,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_elem_off + entry_len_in_flash;
active->fe_data_off = append_loc->fe_data_off;
active->fe_data_len = len;

Expand Down

0 comments on commit 36015a0

Please sign in to comment.