awokd@danwin1210.me has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36192 )
Change subject: vc/amd/agesa: Fix out of bounds read ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/36192/2/src/vendorcode/amd/agesa/f1... File src/vendorcode/amd/agesa/f12/Proc/Mem/Tech/mtthrcSeedTrain.c:
https://review.coreboot.org/c/coreboot/+/36192/2/src/vendorcode/amd/agesa/f1... PS2, Line 329: if ((RxEn >= NBPtr->MinRxEnSeedGross) && (RxEn <= NBPtr->MaxRxEnSeedTotal)) {
Sources had one static initializer for right-hand side of comparison, so those could get treated as […]
Ack
https://review.coreboot.org/c/coreboot/+/36192/2/src/vendorcode/amd/agesa/f1... PS2, Line 337: OutOfRange[ByteLane] = TRUE;
This path is why I was wondering if the intent with 'PassTestRxEnDly[ByteLane] = RxOrig[ByteLane];' […]
Further reading shows the passing results are intended to be the only ones considered, so the contents should not matter in a failed case like this. However, it should be initialized regardless. Objection to my moving 'PassTestRxEnDly[ByteLane] = RxOrig[ByteLane];' here?
https://review.coreboot.org/c/coreboot/+/36192/2/src/vendorcode/amd/agesa/f1... PS2, Line 359: MemClkCycle = (UINT8) (PassTestRxEnDly[ByteLane] >> 5);
Is the file run through preprocessor before coverity analysis? IDS_HDT_CONSOLE() may just have empty […]
Ack