[coreboot-gerrit] Change in coreboot[master]: vc/amd/agesa/f16kb/Proc/CPU: Avoid out-of-bounds read

[Name of user not set (Code Review)]

[e-mail] has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38466 )

Change subject: vc/amd/agesa/f16kb/Proc/CPU: Avoid out-of-bounds read
......................................................................

vc/amd/agesa/f16kb/Proc/CPU: Avoid out-of-bounds read

In an edge case where the cpuF16AddingMmioMap function is
called with a fully populated MmioRange array, MmioPair
could be incremented to its maximum value of 11, causing
an out-of-bounds read in cpuF16MmioMap.c:228 with the
comparison to MmioRange[MmioPair + i]. Add logic to avoid
this case, and similar as i is incremented.

Change-Id: I781bee51a57602f3d73f43ad0ad8466a01ef8736
Signed-off-by: Joe Moore 
Found-by: Coverity CID 1376955
---
M src/vendorcode/amd/agesa/f16kb/Proc/CPU/Family/0x16/cpuF16MmioMap.c
1 file changed, 20 insertions(+), 13 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/38466/1

diff --git a/src/vendorcode/amd/agesa/f16kb/Proc/CPU/Family/0x16/cpuF16MmioMap.c b/src/vendorcode/amd/agesa/f16kb/Proc/CPU/Family/0x16/cpuF16MmioMap.c
index 714f970..1023215 100644
--- a/src/vendorcode/amd/agesa/f16kb/Proc/CPU/Family/0x16/cpuF16MmioMap.c
+++ b/src/vendorcode/amd/agesa/f16kb/Proc/CPU/Family/0x16/cpuF16MmioMap.c
@@ -223,29 +223,36 @@
 //                                                                                       |          |
 //                                                                                   new base   new limit

+          if (MmioPair >= (MMIO_REG_PAIR_NUM - 1)) {
+            IDS_HDT_CONSOLE (MAIN_FLOW, " [ERROR] Not enough MMIO register pairs to hold the request.\n");
+            return AGESA_ERROR;
+          }
           MmioRange[MmioPair].Base = (MmioRange[MmioPair].Base <= NewMmioRange.Base) ? MmioRange[MmioPair].Base : NewMmioRange.Base;
           MmioRange[MmioPair].Modified = TRUE;
-          for (i = 1; NewMmioRange.Limit >= MmioRange[MmioPair + i].Base; i++) {
-            if ((NewMmioRange.Attribute.MmioPostedRange == MmioRange[MmioPair + i].Attribute.MmioPostedRange) &&
-                (NewMmioRange.Attribute.MmioReadableRange == MmioRange[MmioPair + i].Attribute.MmioReadableRange) &&
-                (NewMmioRange.Attribute.MmioWritableRange == MmioRange[MmioPair + i].Attribute.MmioWritableRange) &&
-                (NewMmioRange.Attribute.MmioSecuredRange == MmioRange[MmioPair + i].Attribute.MmioSecuredRange)) {
-              MmioRange[MmioPair].Limit = MmioRange[MmioPair + i].Limit;
-              MmioRange[MmioPair + i].Base = 0;
-              MmioRange[MmioPair + i].Limit = F16_MMIO_ALIGN;
-              MmioRange[MmioPair + i].Attribute.MmioReadableRange = 0;
-              MmioRange[MmioPair + i].Attribute.MmioWritableRange = 0;
-              MmioRange[MmioPair + i].Modified = TRUE;
+          for (i = MmioPair + 1; i < (MMIO_REG_PAIR_NUM - 1); i++) {
+            if ((NewMmioRange.Attribute.MmioPostedRange == MmioRange[i].Attribute.MmioPostedRange) &&
+                (NewMmioRange.Attribute.MmioReadableRange == MmioRange[i].Attribute.MmioReadableRange) &&
+                (NewMmioRange.Attribute.MmioWritableRange == MmioRange[i].Attribute.MmioWritableRange) &&
+                (NewMmioRange.Attribute.MmioSecuredRange == MmioRange[i].Attribute.MmioSecuredRange) &&
+                (NewMmioRange.Limit >= MmioRange[i + 1].Base)) {
+              MmioRange[MmioPair].Limit = MmioRange[i].Limit;
+              MmioRange[i].Base = 0;
+              MmioRange[i].Limit = F16_MMIO_ALIGN;
+              MmioRange[i].Attribute.MmioReadableRange = 0;
+              MmioRange[i].Attribute.MmioWritableRange = 0;
+              MmioRange[i].Modified = TRUE;
               UnusedMmioPair++;
-            } else if (MmioRange[MmioPair + i].Attribute.MmioReadableRange != 0 || MmioRange[MmioPair + i].Attribute.MmioWritableRange != 0) {
+            } else if (MmioRange[i].Attribute.MmioReadableRange != 0 || MmioRange[i].Attribute.MmioWritableRange != 0) {
               // Overlapped MMIO regions with different attributes
               MmioRange[MmioPair].Limit = (MmioRange[MmioPair].Limit >= NewMmioRange.Limit) ? MmioRange[MmioPair].Limit : NewMmioRange.Limit;
               NewMmioIncluded = TRUE;
               Overlap = TRUE;
               break;
+            } else if (NewMmioRange.Limit < MmioRange[i + 1].Base) {
+              break;
             }
           }
-          MmioRange[MmioPair].Limit = (MmioRange[MmioPair + i - 1].Limit >= NewMmioRange.Limit) ? MmioRange[MmioPair + i - 1].Limit : NewMmioRange.Limit;
+          MmioRange[MmioPair].Limit = (MmioRange[i].Limit >= NewMmioRange.Limit) ? MmioRange[i].Limit : NewMmioRange.Limit;
           break;
         } else {
           // Overlapped MMIO regions with different attributes