I guess we could check the name_len here or add a CRC to check uffs_FileInfo.

But why the info is invalid ? we are building a valid tree node and the data suppose to be valid.

My immediate workaround was to add a length sanity check to the core CRC routine. It might be good to cap the length at whatever the maximum size expected for this routine (max page size?) so at least it always falls through in reasonable time.

As far as why the info is invalid, that is what I am trying to figure out. I am still having an issue with getting UFFS up and running for the first time on my target. Files above a certain size are not appearing in the directory. I think it is related (i.e. bad FileInfo). However, this is probably a problem with my low level flash drivers, not UFFS. I'm still investigating.

To add additional robustness I think the assumption should be made that any page read can result in bad data, even though it may have passed integrity check before. In this case, a page is read from external flash, it is checked to be marked as a bad, however the info-> data is then unconditionally used (even if the page is detected as corrupt). Here, it's probably my faulty flash driver. However, an unintended write or cosmic event could give a corrupt page read at any time. Yes, I think a CRC should validate ALL file metadata in the absence of a page wide ECC or CRC check, including the FileInfo. I think a CRC here would be a good solution.

There are only a few spots in the code where uffs_FlashPageRead() is called, and in some of those spots the metadata is used unconditionally regardless of the page error reporting:
URET _BuildValidTreeNode(()
uffs_BufFlush_Exist_With_BlockRecover()
uffs_BufGetEx()
process_pending_recover()

Additional error handling should be added in these spots to mitigate a bad page read or a maliciously corrupted page.

An update to this post:

Ricky, thanks for your help in troubleshooting the problem!

The core problem was that the max number of pages per block UFFS supports out of the box is 64, and UFFS was aliasing to the wrong page looking for the FileInfo header. This can be rectified by modifying the uffs_TagStoreSt to increase the pade_id bit-field while reducing the reserved field to make room. Additionally the line TAG_PAGE_ID(tag) = (u8)(i & 0xFF); needs to be updated.

To this, I have the following feature requests:

• A runtime check to see if UFFS is being configured with more pages per block than it supports, and if so error out.
• The line TAG_PAGE_ID(tag) = (u8)(i & 0xFF); should be updated to TAG_PAGE_ID(tag) = i; in the latest code release, and the FIX ME comment deleted.
• A CRC should be added to the FileInfo page to verify the integrity of the metadata.
• Put a LEN sanity check on the core CRC routine. Cap the max length to 16K or 64K for instance.
• There are a few spots in the code where a page read is done, but the contents of the page are used unconditionally (i.e. file metadata), even if the page reports as bad (bad ECC or bad CRC). In these places, the assumption is made that the page will be good based on a recent prior good page read of the data. However, I don't think this is an assumption that should be relied upon. I think the code should assume that a bad page read can happen at any time even if it was recently good.

Chris