[edk2-devel] [PATCH v1][edk2-stable202002] MdeModulePkg/SdMmcPciHcDxe: Fix double PciIo Unmap in TRB creation (CVE-2019-14587)

Wu, Hao A posted 1 patch 4 years, 2 months ago
Failed in applying to current master (apply log)
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[edk2-devel] [PATCH v1][edk2-stable202002] MdeModulePkg/SdMmcPciHcDxe: Fix double PciIo Unmap in TRB creation (CVE-2019-14587)
Posted by Wu, Hao A 4 years, 2 months ago
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1989

The commit will avoid unmapping the same resource in error handling logic
for function BuildAdmaDescTable() and SdMmcCreateTrb().

For the error handling in BuildAdmaDescTable():
The error is directly related with the corresponding Map() operation
(mapped address beyond 4G, which is not supported in ADMA), so the Unmap()
operation is done in the error handling logic, and then setting
'Trb->AdmaMap' to NULL to avoid double Unmap.

For the error handling in SdMmcCreateTrb():
The error is not directly related with the corresponding Map() operation,
so the commit will update the code to left SdMmcFreeTrb() for the Unmap
operation to avoid double Unmap.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index da5559ae76..43626fff48 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -1544,6 +1544,8 @@ BuildAdmaDescTable (
       PciIo,
       Trb->AdmaMap
     );
+    Trb->AdmaMap = NULL;
+
     PciIo->FreeBuffer (
       PciIo,
       EFI_SIZE_TO_PAGES (TableSize),
@@ -1753,7 +1755,6 @@ SdMmcCreateTrb (
       }
       Status = BuildAdmaDescTable (Trb, Private->ControllerVersion[Slot]);
       if (EFI_ERROR (Status)) {
-        PciIo->Unmap (PciIo, Trb->DataMap);
         goto Error;
       }
     } else if (Private->Capability[Slot].Sdma != 0) {
-- 
2.12.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54523): https://edk2.groups.io/g/devel/message/54523
Mute This Topic: https://groups.io/mt/71343065/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1][edk2-stable202002] MdeModulePkg/SdMmcPciHcDxe: Fix double PciIo Unmap in TRB creation (CVE-2019-14587)
Posted by Wang, Jian J 4 years, 2 months ago
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>

Regards,
Jian

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao A
> Sent: Monday, February 17, 2020 11:52 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Ni, Ray <ray.ni@intel.com>
> Subject: [edk2-devel] [PATCH v1][edk2-stable202002]
> MdeModulePkg/SdMmcPciHcDxe: Fix double PciIo Unmap in TRB creation (CVE-
> 2019-14587)
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1989
> 
> The commit will avoid unmapping the same resource in error handling logic
> for function BuildAdmaDescTable() and SdMmcCreateTrb().
> 
> For the error handling in BuildAdmaDescTable():
> The error is directly related with the corresponding Map() operation
> (mapped address beyond 4G, which is not supported in ADMA), so the Unmap()
> operation is done in the error handling logic, and then setting
> 'Trb->AdmaMap' to NULL to avoid double Unmap.
> 
> For the error handling in SdMmcCreateTrb():
> The error is not directly related with the corresponding Map() operation,
> so the commit will update the code to left SdMmcFreeTrb() for the Unmap
> operation to avoid double Unmap.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index da5559ae76..43626fff48 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -1544,6 +1544,8 @@ BuildAdmaDescTable (
>        PciIo,
>        Trb->AdmaMap
>      );
> +    Trb->AdmaMap = NULL;
> +
>      PciIo->FreeBuffer (
>        PciIo,
>        EFI_SIZE_TO_PAGES (TableSize),
> @@ -1753,7 +1755,6 @@ SdMmcCreateTrb (
>        }
>        Status = BuildAdmaDescTable (Trb, Private->ControllerVersion[Slot]);
>        if (EFI_ERROR (Status)) {
> -        PciIo->Unmap (PciIo, Trb->DataMap);
>          goto Error;
>        }
>      } else if (Private->Capability[Slot].Sdma != 0) {
> --
> 2.12.0.windows.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54524): https://edk2.groups.io/g/devel/message/54524
Mute This Topic: https://groups.io/mt/71343065/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1][edk2-stable202002] MdeModulePkg/SdMmcPciHcDxe: Fix double PciIo Unmap in TRB creation (CVE-2019-14587)
Posted by Wu, Hao A 4 years, 2 months ago
> -----Original Message-----
> From: Wang, Jian J
> Sent: Monday, February 17, 2020 12:03 PM
> To: devel@edk2.groups.io; Wu, Hao A
> Cc: Ni, Ray
> Subject: RE: [edk2-devel] [PATCH v1][edk2-stable202002]
> MdeModulePkg/SdMmcPciHcDxe: Fix double PciIo Unmap in TRB creation
> (CVE-2019-14587)
> 
> 
> Reviewed-by: Jian J Wang <jian.j.wang@intel.com>


Thanks, pushed via commit e36d5ac7d1.

Best Regards,
Hao Wu


> 
> Regards,
> Jian
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao
> A
> > Sent: Monday, February 17, 2020 11:52 AM
> > To: devel@edk2.groups.io
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> > Ni, Ray <ray.ni@intel.com>
> > Subject: [edk2-devel] [PATCH v1][edk2-stable202002]
> > MdeModulePkg/SdMmcPciHcDxe: Fix double PciIo Unmap in TRB creation
> (CVE-
> > 2019-14587)
> >
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1989
> >
> > The commit will avoid unmapping the same resource in error handling logic
> > for function BuildAdmaDescTable() and SdMmcCreateTrb().
> >
> > For the error handling in BuildAdmaDescTable():
> > The error is directly related with the corresponding Map() operation
> > (mapped address beyond 4G, which is not supported in ADMA), so the
> Unmap()
> > operation is done in the error handling logic, and then setting
> > 'Trb->AdmaMap' to NULL to avoid double Unmap.
> >
> > For the error handling in SdMmcCreateTrb():
> > The error is not directly related with the corresponding Map() operation,
> > so the commit will update the code to left SdMmcFreeTrb() for the Unmap
> > operation to avoid double Unmap.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
> > ---
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > index da5559ae76..43626fff48 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > @@ -1544,6 +1544,8 @@ BuildAdmaDescTable (
> >        PciIo,
> >        Trb->AdmaMap
> >      );
> > +    Trb->AdmaMap = NULL;
> > +
> >      PciIo->FreeBuffer (
> >        PciIo,
> >        EFI_SIZE_TO_PAGES (TableSize),
> > @@ -1753,7 +1755,6 @@ SdMmcCreateTrb (
> >        }
> >        Status = BuildAdmaDescTable (Trb, Private->ControllerVersion[Slot]);
> >        if (EFI_ERROR (Status)) {
> > -        PciIo->Unmap (PciIo, Trb->DataMap);
> >          goto Error;
> >        }
> >      } else if (Private->Capability[Slot].Sdma != 0) {
> > --
> > 2.12.0.windows.1
> >
> >
> > 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54568): https://edk2.groups.io/g/devel/message/54568
Mute This Topic: https://groups.io/mt/71343065/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-