On Thu, 5 Sept 2024 at 13:09, Michal Simek michal.simek@amd.com wrote:
On 9/5/24 09:38, Sughosh Ganu wrote:
On Thu, 5 Sept 2024 at 13:03, Michal Simek michal.simek@amd.com wrote:
On 9/5/24 08:24, Sughosh Ganu wrote:
On Wed, 4 Sept 2024 at 17:30, Michal Simek michal.simek@amd.com wrote:
On 8/30/24 13:40, Sughosh Ganu wrote:
The following set of patches are miscellaneous fixes and some hardening of the FWU update logic.
Sughosh Ganu (6): fwu: v2: perform some checks before reading metadata fwu: v2: try reading both copies of metadata fwu: v1: do a version check for the metadata fwu: check all images for transitioning out of Trial State fwu: add dependency checks for selecting FWU metadata version fwu: do not allow capsule processing on exceeding Trial Counter threshold
include/fwu.h | 11 ++++++ lib/fwu_updates/Kconfig | 1 + lib/fwu_updates/fwu.c | 31 +++++++++++++++- lib/fwu_updates/fwu_v1.c | 18 +++++++-- lib/fwu_updates/fwu_v2.c | 80 ++++++++++++++++++++++------------------ 5 files changed, 99 insertions(+), 42 deletions(-)I found one more thing.
I did this change
diff --git a/tools/mkfwumdata.c b/tools/mkfwumdata.c index fbc2067bc12d..dab9530e499c 100644 --- a/tools/mkfwumdata.c +++ b/tools/mkfwumdata.c @@ -293,7 +293,7 @@ static void fwu_fill_version_specific_mdata(struct fwu_mdata_object *mobj) struct fwu_mdata *mdata = mobj->mdata;
mdata->metadata_size = mobj->size;
mdata->desc_offset = sizeof(struct fwu_mdata);
mdata->desc_offset = 0x21; sizeof(struct fwu_mdata); for (i = 0; i < MAX_BANKS_V2; i++) mdata->bank_state[i] = i < mobj->banks ?to break desc_offset location. I generated mdata structure and write it to primary mdata partition.
This has been spotted by (my debug) which is the reason why I did it.
if (g_mdata.desc_offset != FWU_IMG_DESC_OFFSET) { printf("mdata offset is not 0x20\n"); return false; }But I got one more message below which
mdata offset is not 0x20 Both FWU metadata copies are valid but do not match. Restoring the secondary partition from the primary
The reason why this logic is in place is to handle the scenario whereby, during the metadata update, the primary copy gets updated correctly, but due to some reason (like a power cut), the secondary copy does not -- this is because the primary copy gets updated first. In such a scenario, the secondary copy of metadata can be then restored from the primary copy. In the scenario where the primary copy is corrupted, it will show up in the crc check, and will get restored from the secondary copy.
But in this case - CRC of primary is correct but data inside is wrong because offset is at 0x21 not at 0x20.
How will this event play out ? Am I missing something ? When the metadata is getting updated, the offset will be written as 0x20, and not 0x21. And in case the metadata gets corrupted, the CRC check would fail and the primary partition would get restored from the secondary (assuming that the secondary copy is correct).
It is not after update. It is initial state with incorrect mdata structure.
Well, if the metadata partition is being written with incorrect data, not sure how we combat that (or even if we should). After all, the spec clearly states that the metadata cannot be protected against malicious writes. The logic that you pointed out earlier is to handle the scenario where the primary partition got updated, but the secondary did not.
-sughosh