awokd@danwin1210.me has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38056 )
Change subject: [WIP] vc/amd/agesa/f16kb/Proc/GNB: Fix out-of-bounds read ......................................................................
[WIP] vc/amd/agesa/f16kb/Proc/GNB: Fix out-of-bounds read
Incorrect values read from a different memory region will cause incorrect computations. VceFlags array size should be 4 based on similar code in f15 branch, and because f16kb/Proc/GNB/Modules/GnbInitKB/GnbF1TableKB.c only loads 4 values for VceFlags in DefaultPpF1ArrayKB. Leaving it at 5 creates Coverity CID 1241878.
Change-Id: I0242c0634e66616018e6df04ac6f1505b82a630f Signed-off-by: Joe Moore awokd@danwin1210.me Found-by: Coverity CID 1241878 --- M src/vendorcode/amd/agesa/f16kb/Proc/GNB/Common/GnbF1Table.h 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/38056/1
diff --git a/src/vendorcode/amd/agesa/f16kb/Proc/GNB/Common/GnbF1Table.h b/src/vendorcode/amd/agesa/f16kb/Proc/GNB/Common/GnbF1Table.h index add5509..90df07c 100644 --- a/src/vendorcode/amd/agesa/f16kb/Proc/GNB/Common/GnbF1Table.h +++ b/src/vendorcode/amd/agesa/f16kb/Proc/GNB/Common/GnbF1Table.h @@ -66,7 +66,7 @@ UINT32 PP_FUSE_ARRAY_V2_fld11; UINT32 PP_FUSE_ARRAY_V2_fld12; BOOLEAN PP_FUSE_ARRAY_V2_fld13; - UINT8 VceFlags[5]; ///< VCE Flags + UINT8 VceFlags[4]; ///< VCE Flags UINT8 VceMclk; ///< MCLK for VCE UINT8 PP_FUSE_ARRAY_V2_fld16[4]; UINT8 EclkDid[5]; ///< Eclk DID
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38056 )
Change subject: [WIP] vc/amd/agesa/f16kb/Proc/GNB: Fix out-of-bounds read ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38056/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38056/1//COMMIT_MSG@15 PS1, Line 15: Where does the out-of-bounds read happen?
Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38056 )
Change subject: [WIP] vc/amd/agesa/f16kb/Proc/GNB: Fix out-of-bounds read ......................................................................
Patch Set 1: Code-Review+2
Display is working fine on fam16h ASUS AM1I-A with Athlon-5370 APU installed (pci1002,9830.rom, iGPU HD-8400 / R3-Series)
Hello Mike Banon, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38056
to look at the new patch set (#2).
Change subject: [WIP] vc/amd/agesa/f16kb/Proc/GNB: Fix out-of-bounds read ......................................................................
[WIP] vc/amd/agesa/f16kb/Proc/GNB: Fix out-of-bounds read
Incorrect values read from a different memory region will cause incorrect computations. VceFlags array size should be 4 based on similar code in f15 branch, and because f16kb/Proc/GNB/Modules/GnbInitKB/GnbF1TableKB.c only loads 4 values for VceFlags in DefaultPpF1ArrayKB. Leaving it at 5 results in an out-of-bounds read of PP_FUSE_ARRAY_V2_fld16 in line 901 of f16kb/Proc/GNB/Modules/GnbGfxIntTableV3/GfxPwrPlayTable.c when Index reaches 4.
Change-Id: I0242c0634e66616018e6df04ac6f1505b82a630f Signed-off-by: Joe Moore awokd@danwin1210.me Found-by: Coverity CID 1241878 --- M src/vendorcode/amd/agesa/f16kb/Proc/GNB/Common/GnbF1Table.h 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/38056/2
Hello Mike Banon, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38056
to look at the new patch set (#3).
Change subject: vc/amd/agesa/f16kb/Proc/GNB: Fix out-of-bounds read ......................................................................
vc/amd/agesa/f16kb/Proc/GNB: Fix out-of-bounds read
Incorrect values read from a different memory region will cause incorrect computations. VceFlags array size should be 4 based on similar code in f15 branch, and because f16kb/Proc/GNB/Modules/GnbInitKB/GnbF1TableKB.c only loads 4 values for VceFlags in DefaultPpF1ArrayKB. Leaving it at 5 results in an out-of-bounds read of PP_FUSE_ARRAY_V2_fld16 in line 901 of f16kb/Proc/GNB/Modules/GnbGfxIntTableV3/GfxPwrPlayTable.c when Index reaches 4.
Change-Id: I0242c0634e66616018e6df04ac6f1505b82a630f Signed-off-by: Joe Moore awokd@danwin1210.me Found-by: Coverity CID 1241878 --- M src/vendorcode/amd/agesa/f16kb/Proc/GNB/Common/GnbF1Table.h 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/38056/3
awokd@danwin1210.me has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38056 )
Change subject: vc/amd/agesa/f16kb/Proc/GNB: Fix out-of-bounds read ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38056/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38056/1//COMMIT_MSG@15 PS1, Line 15:
Where does the out-of-bounds read happen?
Updated commit message with: Leaving it at 5 results in an out-of-bounds read of PP_FUSE_ARRAY_V2_fld16 in line 901 of f16kb/Proc/GNB/Modules/GnbGfxIntTableV3/GfxPwrPlayTable.c when Index reaches 4.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38056 )
Change subject: vc/amd/agesa/f16kb/Proc/GNB: Fix out-of-bounds read ......................................................................
Patch Set 3: Code-Review+1
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38056 )
Change subject: vc/amd/agesa/f16kb/Proc/GNB: Fix out-of-bounds read ......................................................................
vc/amd/agesa/f16kb/Proc/GNB: Fix out-of-bounds read
Incorrect values read from a different memory region will cause incorrect computations. VceFlags array size should be 4 based on similar code in f15 branch, and because f16kb/Proc/GNB/Modules/GnbInitKB/GnbF1TableKB.c only loads 4 values for VceFlags in DefaultPpF1ArrayKB. Leaving it at 5 results in an out-of-bounds read of PP_FUSE_ARRAY_V2_fld16 in line 901 of f16kb/Proc/GNB/Modules/GnbGfxIntTableV3/GfxPwrPlayTable.c when Index reaches 4.
Change-Id: I0242c0634e66616018e6df04ac6f1505b82a630f Signed-off-by: Joe Moore awokd@danwin1210.me Found-by: Coverity CID 1241878 Reviewed-on: https://review.coreboot.org/c/coreboot/+/38056 Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Mike Banon mikebdp2@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/vendorcode/amd/agesa/f16kb/Proc/GNB/Common/GnbF1Table.h 1 file changed, 1 insertion(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Mike Banon: Looks good to me, approved
diff --git a/src/vendorcode/amd/agesa/f16kb/Proc/GNB/Common/GnbF1Table.h b/src/vendorcode/amd/agesa/f16kb/Proc/GNB/Common/GnbF1Table.h index add5509..90df07c 100644 --- a/src/vendorcode/amd/agesa/f16kb/Proc/GNB/Common/GnbF1Table.h +++ b/src/vendorcode/amd/agesa/f16kb/Proc/GNB/Common/GnbF1Table.h @@ -66,7 +66,7 @@ UINT32 PP_FUSE_ARRAY_V2_fld11; UINT32 PP_FUSE_ARRAY_V2_fld12; BOOLEAN PP_FUSE_ARRAY_V2_fld13; - UINT8 VceFlags[5]; ///< VCE Flags + UINT8 VceFlags[4]; ///< VCE Flags UINT8 VceMclk; ///< MCLK for VCE UINT8 PP_FUSE_ARRAY_V2_fld16[4]; UINT8 EclkDid[5]; ///< Eclk DID
-
Mike Banon (Code Review) -
Name of user not set (Code Review)
-
Nico Huber (Code Review)
-
Paul Menzel (Code Review)