eclipse-threadx / levelx

Eclipse ThreadX - LevelX Provides Flash Wear Leveling for FileX and Stand Alone purposes.
https://github.com/eclipse-threadx/rtos-docs/blob/main/rtos-docs/levelx/index.md
MIT License
102 stars 61 forks source link

Weak ECC check #44

Open stefano-zanotti opened 7 months ago

stefano-zanotti commented 7 months ago

The function _lx_nand_flash_256byte_ecc_check cannot properly detect single-bit errors in the ECC bytes themselves. Such single-bit errors result in a LX_NAND_ERROR_NOT_CORRECTED return result from the function.

Repro:

UCHAR buff[256];
memset(buff, 0xFF, 256);
UCHAR ecc[3];
lx_nand_flash_256byte_ecc_compute(buff, ecc); // this results in an ECC of {0xFF, 0xFF, 0xFF}
ecc[0] ^= 0x1;
UINT res = lx_nand_flash_256byte_ecc_check((UCHAR*)buff, (UCHAR*)ecc); // this returns LX_NAND_ERROR_NOT_CORRECTED

The same code, injecting an error in the data rather than the ECC:

UCHAR buff[256];
memset(buff, 0xFF, 256);
UCHAR ecc[3];
lx_nand_flash_256byte_ecc_compute(buff, ecc); // this results in an ECC of {0xFF, 0xFF, 0xFF}
buff[0] ^= 0x1; // <<-- CHANGED HERE
UINT res = lx_nand_flash_256byte_ecc_check((UCHAR*)buff, (UCHAR*)ecc); // this works correctly, and returns LX_NAND_ERROR_CORRECTED

I don't know the details of the ECC algorithm used here (whether the ECC is actually strong enough to reliably correct all 1-bit errors in the data or ECC itself, and to reliably detect all 2-bit errors), but maybe a simple fix would be the following? Here: https://github.com/eclipse-threadx/levelx/blob/24eb7d86926dea748e816dd43f5c5b88875776cc/common/src/lx_nand_flash_256byte_ecc_check.c#L135-L144 add a check:

/* Was a correctable error discovered in the ECC itself?  */
else if (error_count == 1) {
    /* Error in the ECC: nothing to actually correct in the data */
    return(LX_NAND_ERROR_CORRECTED);
}