[coreboot-gerrit] Patch set updated for coreboot: 3a8618c AMD IMC AGESA: Access the data in stack by correct length

Zheng Bao (zheng.bao@amd.com) gerrit at coreboot.org
Tue Dec 3 03:22:58 CET 2013


Zheng Bao (zheng.bao at amd.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/4297

-gerrit

commit 3a8618cae49b88cd8350aa5bfccb99ca0954a469
Author: Zheng Bao <fishbaozi at gmail.com>
Date:   Tue Dec 3 10:00:18 2013 +0800

    AMD IMC AGESA: Access the data in stack by correct length
    
    The bug is hard to find. We were adding the feature of fan control. We
    met some strange things which could not be explained. Like, sometimes
    adding printk let the error disappear. Then we traced the code by hardware
    debug tool (HDT). It turned out the data in stack was overwritten.
    
    The values of AccessWidthxx are
    { AccessWidth8 = 1,
      AccessWidth16,
      AccessWidth32,}
    For the case of AccessWidth8, we only need to access the index/data
    once. But ReadECmsg and WriteECmsg did the loop twice, 1 more time
    than they are supposed to do. The data in stack next to "Value" will
    be overwritten.
    
    For all the cases, the code should be
     OpFlag = OpFlag & 0x7f;
     switch (OpFlag) {
        case 1:              /* AccessWidth8 */
             OpFlag = 0;break;
        case 2:              /* AccessWidth16 */
             OpFlag = 1;break;
        case 3:              /* AccessWidth32 */
             OpFlag = 3;break;
        case 4:              /* AccessWidth64 */
             OpFlag = 7;break;
        default:
             error;
     }
    
    Actually, the caller only takes AccessWidth8 as the parameter. We can ignore other
    cases for now.
    
    That is an AGESA bug. AMD's AGESA team own this code. I presume let them
    decide the proper way to fix that. Before that, I change the code as little
    as possible to make it run without crash.
    
    Change-Id: I566f74c242ce93f4569eedf69ca07d2fb7fb368d
    Signed-off-by: Zheng Bao <zheng.bao at amd.com>
    Signed-off-by: Zheng Bao <fishbaozi at gmail.com>
---
 src/vendorcode/amd/agesa/f12/Proc/Fch/Imc/ImcLib.c   | 4 ++--
 src/vendorcode/amd/agesa/f15tn/Proc/Fch/Imc/ImcLib.c | 4 ++--
 src/vendorcode/amd/agesa/f16kb/Proc/Fch/Imc/ImcLib.c | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/vendorcode/amd/agesa/f12/Proc/Fch/Imc/ImcLib.c b/src/vendorcode/amd/agesa/f12/Proc/Fch/Imc/ImcLib.c
index 1a3f7dd..30aa59f 100644
--- a/src/vendorcode/amd/agesa/f12/Proc/Fch/Imc/ImcLib.c
+++ b/src/vendorcode/amd/agesa/f12/Proc/Fch/Imc/ImcLib.c
@@ -55,7 +55,7 @@ WriteECmsg (
 {
   UINT8   Index;
 
-  OpFlag = OpFlag & 0x7f;
+  OpFlag = (OpFlag - 1) & 0x7f;
   if (OpFlag == 0x02) OpFlag = 0x03;
 
   for (Index = 0; Index <= OpFlag; Index++) {
@@ -77,7 +77,7 @@ ReadECmsg (
 {
   UINT8 Index;
 
-  OpFlag = OpFlag & 0x7f;
+  OpFlag = (OpFlag - 1) & 0x7f;
   if (OpFlag == 0x02) OpFlag = 0x03;
 
   for (Index = 0; Index <= OpFlag; Index++) {
diff --git a/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Imc/ImcLib.c b/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Imc/ImcLib.c
index a451c41..0069970 100644
--- a/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Imc/ImcLib.c
+++ b/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Imc/ImcLib.c
@@ -55,7 +55,7 @@ WriteECmsg (
 {
   UINT8   Index;
 
-  OpFlag = OpFlag & 0x7f;
+  OpFlag = (OpFlag - 1) & 0x7f;
   if (OpFlag == 0x02) {
     OpFlag = 0x03;
   }
@@ -79,7 +79,7 @@ ReadECmsg (
 {
   UINT8 Index;
 
-  OpFlag = OpFlag & 0x7f;
+  OpFlag = (OpFlag - 1) & 0x7f;
   if (OpFlag == 0x02) {
     OpFlag = 0x03;
   }
diff --git a/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Imc/ImcLib.c b/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Imc/ImcLib.c
index 05f5727..5bef58e 100644
--- a/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Imc/ImcLib.c
+++ b/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Imc/ImcLib.c
@@ -66,7 +66,7 @@ WriteECmsg (
 {
   UINT8   Index;
 
-  OpFlag = OpFlag & 0x7f;
+  OpFlag = (OpFlag - 1) & 0x7f;
   if (OpFlag == 0x02) {
     OpFlag = 0x03;
   }
@@ -101,7 +101,7 @@ ReadECmsg (
 {
   UINT8 Index;
 
-  OpFlag = OpFlag & 0x7f;
+  OpFlag = (OpFlag - 1) & 0x7f;
   if (OpFlag == 0x02) {
     OpFlag = 0x03;
   }



More information about the coreboot-gerrit mailing list