[Patch 0/3] Fix UEFI and Tiano Decompression logic issue

[Patch 0/3] Fix UEFI and Tiano Decompression logic issue

[Liming Gao]

https://bugzilla.tianocore.org/show_bug.cgi?id=1317

This is a regression issue caused by previous change with more checkers in 
UefiDecompress. In Decode() function, once mOutBuf is fully filled, Decode() 
should return. Current logic misses the checker of mOutBuf after while() loop.

Liming Gao (3):
  BaseTools: Fix UEFI and Tiano Decompression logic issue
  MdePkg BaseUefiDecompressLib: Fix UEFI Decompression logic issue
  IntelFrameworkModulePkg: Fix UEFI and Tiano Decompression logic issue

 BaseTools/Source/C/Common/Decompress.c                                                              | 6 ++++++
 BaseTools/Source/C/TianoCompress/TianoCompress.c                                                    | 6 ++++++
 IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.c | 6 ++++++
 MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.c                                        | 6 ++++++
 4 files changed, 24 insertions(+)

-- 
2.16.2.windows.1

[Patch 1/3] BaseTools: Fix UEFI and Tiano Decompression logic issue

[Liming Gao]

https://bugzilla.tianocore.org/show_bug.cgi?id=1317

This is a regression issue caused by 041d89bc0f0119df37a5fce1d0f16495ff905089.
In Decode() function, once mOutBuf is fully filled, Decode() should return.
Current logic misses the checker of mOutBuf after while() loop.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
---
 BaseTools/Source/C/Common/Decompress.c           | 6 ++++++
 BaseTools/Source/C/TianoCompress/TianoCompress.c | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/BaseTools/Source/C/Common/Decompress.c b/BaseTools/Source/C/Common/Decompress.c
index 71313b1179..33e0f0d160 100644
--- a/BaseTools/Source/C/Common/Decompress.c
+++ b/BaseTools/Source/C/Common/Decompress.c
@@ -662,6 +662,12 @@ Returns: (VOID)
 
         BytesRemain--;
       }
+      //
+      // Once mOutBuf is fully filled, directly return
+      //
+      if (Sd->mOutBuf >= Sd->mOrigSize) {
+        return ;
+      }
     }
   }
 
diff --git a/BaseTools/Source/C/TianoCompress/TianoCompress.c b/BaseTools/Source/C/TianoCompress/TianoCompress.c
index 2d6fc4c952..a77f6798ec 100644
--- a/BaseTools/Source/C/TianoCompress/TianoCompress.c
+++ b/BaseTools/Source/C/TianoCompress/TianoCompress.c
@@ -2682,6 +2682,12 @@ Returns: (VOID)
 
         BytesRemain--;
       }
+      //
+      // Once mOutBuf is fully filled, directly return
+      //
+      if (Sd->mOutBuf >= Sd->mOrigSize) {
+        goto Done ;
+      }
     }
   }
 
-- 
2.16.2.windows.1

[Patch 2/3] MdePkg BaseUefiDecompressLib: Fix UEFI Decompression logic issue

[Liming Gao]

https://bugzilla.tianocore.org/show_bug.cgi?id=1317

This is a regression issue caused by 2ec7953d49677142c5f7552e9e3d96fb406ba0c4.
In Decode() function, once mOutBuf is fully filled, Decode() should return.
Current logic misses the checker of mOutBuf after while() loop.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao <liming.gao@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
---
 MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.c b/MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.c
index 9fc637e058..c1e8c5581a 100644
--- a/MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.c
+++ b/MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.c
@@ -641,6 +641,12 @@ Decode (
 
         BytesRemain--;
       }
+      //
+      // Once mOutBuf is fully filled, directly return
+      //
+      if (Sd->mOutBuf >= Sd->mOrigSize) {
+        goto Done;
+      }
     }
   }
 
-- 
2.16.2.windows.1

[Patch 3/3] IntelFrameworkModulePkg: Fix UEFI and Tiano Decompression logic issue

[Liming Gao]

https://bugzilla.tianocore.org/show_bug.cgi?id=1317

This is a regression issue caused by 684db6da64bc7b5faee4e1174e801c245f563b5c.
In Decode() function, once mOutBuf is fully filled, Decode() should return.
Current logic misses the checker of mOutBuf after while() loop.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao <liming.gao@intel.com>
---
 IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.c b/IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.c
index 39999a07c3..970795b1da 100644
--- a/IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.c
+++ b/IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.c
@@ -634,6 +634,12 @@ Decode (
 
         BytesRemain--;
       }
+      //
+      // Once mOutBuf is fully filled, directly return
+      //
+      if (Sd->mOutBuf >= Sd->mOrigSize) {
+        goto Done ;
+      }
     }
   }
 
-- 
2.16.2.windows.1

Re: [Patch 1/3] BaseTools: Fix UEFI and Tiano Decompression logic issue

[Zhu, Yonghong]

Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com> 

Best Regards,
Zhu Yonghong

-----Original Message-----
From: Gao, Liming 
Sent: Friday, November 09, 2018 7:58 AM
To: edk2-devel@lists.01.org
Cc: Zhu, Yonghong <yonghong.zhu@intel.com>
Subject: [Patch 1/3] BaseTools: Fix UEFI and Tiano Decompression logic issue

https://bugzilla.tianocore.org/show_bug.cgi?id=1317

This is a regression issue caused by 041d89bc0f0119df37a5fce1d0f16495ff905089.
In Decode() function, once mOutBuf is fully filled, Decode() should return.
Current logic misses the checker of mOutBuf after while() loop.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
---
 BaseTools/Source/C/Common/Decompress.c           | 6 ++++++
 BaseTools/Source/C/TianoCompress/TianoCompress.c | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/BaseTools/Source/C/Common/Decompress.c b/BaseTools/Source/C/Common/Decompress.c
index 71313b1179..33e0f0d160 100644
--- a/BaseTools/Source/C/Common/Decompress.c
+++ b/BaseTools/Source/C/Common/Decompress.c
@@ -662,6 +662,12 @@ Returns: (VOID)
 
         BytesRemain--;
       }
+      //
+      // Once mOutBuf is fully filled, directly return
+      //
+      if (Sd->mOutBuf >= Sd->mOrigSize) {
+        return ;
+      }
     }
   }
 
diff --git a/BaseTools/Source/C/TianoCompress/TianoCompress.c b/BaseTools/Source/C/TianoCompress/TianoCompress.c
index 2d6fc4c952..a77f6798ec 100644
--- a/BaseTools/Source/C/TianoCompress/TianoCompress.c
+++ b/BaseTools/Source/C/TianoCompress/TianoCompress.c
@@ -2682,6 +2682,12 @@ Returns: (VOID)
 
         BytesRemain--;
       }
+      //
+      // Once mOutBuf is fully filled, directly return
+      //
+      if (Sd->mOutBuf >= Sd->mOrigSize) {
+        goto Done ;
+      }
     }
   }
 
-- 
2.16.2.windows.1

Re: [Patch 0/3] Fix UEFI and Tiano Decompression logic issue

[Zhu, Yonghong]

Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com> 

Best Regards,
Zhu Yonghong

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Liming Gao
Sent: Friday, November 09, 2018 7:58 AM
To: edk2-devel@lists.01.org
Subject: [edk2] [Patch 0/3] Fix UEFI and Tiano Decompression logic issue

https://bugzilla.tianocore.org/show_bug.cgi?id=1317

This is a regression issue caused by previous change with more checkers in UefiDecompress. In Decode() function, once mOutBuf is fully filled, Decode() should return. Current logic misses the checker of mOutBuf after while() loop.

Liming Gao (3):
  BaseTools: Fix UEFI and Tiano Decompression logic issue
  MdePkg BaseUefiDecompressLib: Fix UEFI Decompression logic issue
  IntelFrameworkModulePkg: Fix UEFI and Tiano Decompression logic issue

 BaseTools/Source/C/Common/Decompress.c                                                              | 6 ++++++
 BaseTools/Source/C/TianoCompress/TianoCompress.c                                                    | 6 ++++++
 IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.c | 6 ++++++
 MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.c                                        | 6 ++++++
 4 files changed, 24 insertions(+)

--
2.16.2.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [Patch 0/3] Fix UEFI and Tiano Decompression logic issue

[Gao, Liming]

Hi, all
  This patch fixes the Uefi and Tiano decompression issue. I will push it for edk2-stable201811 tag. If you have any comments, please let me know. 

Thanks
Liming
> -----Original Message-----
> From: Zhu, Yonghong
> Sent: Saturday, November 10, 2018 4:54 PM
> To: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org
> Cc: Zhu, Yonghong <yonghong.zhu@intel.com>
> Subject: RE: [edk2] [Patch 0/3] Fix UEFI and Tiano Decompression logic issue
> 
> Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>
> 
> Best Regards,
> Zhu Yonghong
> 
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Liming Gao
> Sent: Friday, November 09, 2018 7:58 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch 0/3] Fix UEFI and Tiano Decompression logic issue
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1317
> 
> This is a regression issue caused by previous change with more checkers in UefiDecompress. In Decode() function, once mOutBuf is fully
> filled, Decode() should return. Current logic misses the checker of mOutBuf after while() loop.
> 
> Liming Gao (3):
>   BaseTools: Fix UEFI and Tiano Decompression logic issue
>   MdePkg BaseUefiDecompressLib: Fix UEFI Decompression logic issue
>   IntelFrameworkModulePkg: Fix UEFI and Tiano Decompression logic issue
> 
>  BaseTools/Source/C/Common/Decompress.c                                                              | 6 ++++++
>  BaseTools/Source/C/TianoCompress/TianoCompress.c                                                    | 6 ++++++
>  IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.c | 6 ++++++
>  MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.c                                        | 6 ++++++
>  4 files changed, 24 insertions(+)
> 
> --
> 2.16.2.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [Patch 0/3] Fix UEFI and Tiano Decompression logic issue

[Leif Lindholm]

Hi Liming,

I have no issue with a fix for this issue going in during the hard
freeze, but neither the commit messages nor the cover letter contain
information of what problem was caused by the bug.

It also does not say anything about the likelihood of hitting this
bug, or what criteria need to be met. Nor does the bugzilla entry.

The bugzilla entry talks only about the TianoCompress utility, but we
the patches also modify runtime code.

Detail in commit messages are always important, but since this
modifies the behaviour of CVE fixes, there needs to be no room left
(for reviewers) to wonder whether the bugs have been sufficiently
understood and investigated.

Regards,

Leif

On Sat, Nov 10, 2018 at 02:18:51PM +0000, Gao, Liming wrote:
> Hi, all
>   This patch fixes the Uefi and Tiano decompression issue. I will
>   push it for edk2-stable201811 tag. If you have any comments,
>   please let me know.
> 
> Thanks
> Liming
> > -----Original Message-----
> > From: Zhu, Yonghong
> > Sent: Saturday, November 10, 2018 4:54 PM
> > To: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org
> > Cc: Zhu, Yonghong <yonghong.zhu@intel.com>
> > Subject: RE: [edk2] [Patch 0/3] Fix UEFI and Tiano Decompression logic issue
> > 
> > Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>
> > 
> > Best Regards,
> > Zhu Yonghong
> > 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Liming Gao
> > Sent: Friday, November 09, 2018 7:58 AM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] [Patch 0/3] Fix UEFI and Tiano Decompression logic issue
> > 
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1317
> > 
> > This is a regression issue caused by previous change with more checkers in UefiDecompress. In Decode() function, once mOutBuf is fully
> > filled, Decode() should return. Current logic misses the checker of mOutBuf after while() loop.
> > 
> > Liming Gao (3):
> >   BaseTools: Fix UEFI and Tiano Decompression logic issue
> >   MdePkg BaseUefiDecompressLib: Fix UEFI Decompression logic issue
> >   IntelFrameworkModulePkg: Fix UEFI and Tiano Decompression logic issue
> > 
> >  BaseTools/Source/C/Common/Decompress.c                                                              | 6 ++++++
> >  BaseTools/Source/C/TianoCompress/TianoCompress.c                                                    | 6 ++++++
> >  IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.c | 6 ++++++
> >  MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.c                                        | 6 ++++++
> >  4 files changed, 24 insertions(+)
> > 
> > --
> > 2.16.2.windows.1
> > 
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel

Re: [Patch 0/3] Fix UEFI and Tiano Decompression logic issue

[Gao, Liming]

Leif:
 Thanks for your comments. I add more detail in https://bugzilla.tianocore.org/show_bug.cgi?id=1317. I think we can add more information in this BZ. 

Thanks
Liming
>-----Original Message-----
>From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
>Sent: Sunday, November 11, 2018 8:19 PM
>To: Gao, Liming <liming.gao@intel.com>
>Cc: Zhu, Yonghong <yonghong.zhu@intel.com>; edk2-devel@lists.01.org;
>Laszlo Ersek (lersek@redhat.com) <lersek@redhat.com>; Kinney, Michael D
><michael.d.kinney@intel.com>; afish@apple.com
>Subject: Re: [edk2] [Patch 0/3] Fix UEFI and Tiano Decompression logic issue
>
>Hi Liming,
>
>I have no issue with a fix for this issue going in during the hard
>freeze, but neither the commit messages nor the cover letter contain
>information of what problem was caused by the bug.
>
>It also does not say anything about the likelihood of hitting this
>bug, or what criteria need to be met. Nor does the bugzilla entry.
>
>The bugzilla entry talks only about the TianoCompress utility, but we
>the patches also modify runtime code.
>
>Detail in commit messages are always important, but since this
>modifies the behaviour of CVE fixes, there needs to be no room left
>(for reviewers) to wonder whether the bugs have been sufficiently
>understood and investigated.
>
>Regards,
>
>Leif
>
>On Sat, Nov 10, 2018 at 02:18:51PM +0000, Gao, Liming wrote:
>> Hi, all
>>   This patch fixes the Uefi and Tiano decompression issue. I will
>>   push it for edk2-stable201811 tag. If you have any comments,
>>   please let me know.
>>
>> Thanks
>> Liming
>> > -----Original Message-----
>> > From: Zhu, Yonghong
>> > Sent: Saturday, November 10, 2018 4:54 PM
>> > To: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org
>> > Cc: Zhu, Yonghong <yonghong.zhu@intel.com>
>> > Subject: RE: [edk2] [Patch 0/3] Fix UEFI and Tiano Decompression logic
>issue
>> >
>> > Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>
>> >
>> > Best Regards,
>> > Zhu Yonghong
>> >
>> > -----Original Message-----
>> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>Liming Gao
>> > Sent: Friday, November 09, 2018 7:58 AM
>> > To: edk2-devel@lists.01.org
>> > Subject: [edk2] [Patch 0/3] Fix UEFI and Tiano Decompression logic issue
>> >
>> > https://bugzilla.tianocore.org/show_bug.cgi?id=1317
>> >
>> > This is a regression issue caused by previous change with more checkers in
>UefiDecompress. In Decode() function, once mOutBuf is fully
>> > filled, Decode() should return. Current logic misses the checker of
>mOutBuf after while() loop.
>> >
>> > Liming Gao (3):
>> >   BaseTools: Fix UEFI and Tiano Decompression logic issue
>> >   MdePkg BaseUefiDecompressLib: Fix UEFI Decompression logic issue
>> >   IntelFrameworkModulePkg: Fix UEFI and Tiano Decompression logic issue
>> >
>> >  BaseTools/Source/C/Common/Decompress.c
>| 6 ++++++
>> >  BaseTools/Source/C/TianoCompress/TianoCompress.c
>| 6 ++++++
>> >
>IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/Bas
>eUefiTianoCustomDecompressLib.c | 6 ++++++
>> >  MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.c
>| 6 ++++++
>> >  4 files changed, 24 insertions(+)
>> >
>> > --
>> > 2.16.2.windows.1
>> >
>> > _______________________________________________
>> > edk2-devel mailing list
>> > edk2-devel@lists.01.org
>> > https://lists.01.org/mailman/listinfo/edk2-devel