Kyösti Mälkki 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 2:
(6 comments)
https://review.coreboot.org/c/coreboot/+/36192/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36192/2//COMMIT_MSG@9 PS2, Line 9: ByteLane is incorrectly used unitialized from prior for statement. Might be bad coding style, not incorrect per-se. There is A potential out-of-bound read of RxOrig[MaxByteLanes].
https://review.coreboot.org/c/coreboot/+/36192/2//COMMIT_MSG@11 PS2, Line 11: PassTestRxEnDly at that index, so appears safe to delete. Rephrase perhaps: PassTestRxEnDly[MaxByteLanes] never appears as rvalue, all for loops have ByteLane < MaxByteLanes exit condition.
https://review.coreboot.org/c/coreboot/+/36192/2//COMMIT_MSG@14 PS2, Line 14: 'PassTestRxEnDly[ByteLane] = RxOrig[ByteLane];'. I did not understand this comment.
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)) { For coverity scan, this might evaluate as a static true statement.
https://review.coreboot.org/c/coreboot/+/36192/2/src/vendorcode/amd/agesa/f1... PS2, Line 337: OutOfRange[ByteLane] = TRUE; ... so this path of not setting PassTestRxEndDly might not ever be reached.
https://review.coreboot.org/c/coreboot/+/36192/2/src/vendorcode/amd/agesa/f1... PS2, Line 359: MemClkCycle = (UINT8) (PassTestRxEnDly[ByteLane] >> 5); PassTestRxEnDly could still be unset here?