[PATCH] QuarkSocPkg/QNCSmmDispatcher: Fix use after free issue #2

[PATCH] QuarkSocPkg/QNCSmmDispatcher: Fix use after free issue #2

[Marvin Häuser]

As part of commit 5f82e02, ActiveRecordInDb was introduced as a copy
of RecordInDb as latter may be freed by the callback function. This
commit replaces an access of RecordInDb after the callback function
has been executed with an access to ActiveRecordInDb.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
---
 QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmmCore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmmCore.c b/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmmCore.c
index c2f75f86647a..29ad5f493466 100644
--- a/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmmCore.c
+++ b/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmmCore.c
@@ -758,7 +758,7 @@ QNCSmmCoreDispatcher (
             }
           }
 
-          if (RecordInDb->ClearSource == NULL) {
+          if (ActiveRecordInDb.ClearSource == NULL) {
             //
             // Clear the SMI associated w/ the source using the default function
             //
-- 
2.12.2.windows.2

Re: [PATCH] QuarkSocPkg/QNCSmmDispatcher: Fix use after free issue #2

[Kinney, Michael D]

Marvin,

Thanks for the fix!

Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>

I will push shortly.

Mike

> -----Original Message-----
> From: Marvin Häuser [mailto:Marvin.Haeuser@outlook.com]
> Sent: Thursday, July 20, 2017 11:12 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Steele,
> Kelly <kelly.steele@intel.com>
> Subject: [PATCH] QuarkSocPkg/QNCSmmDispatcher: Fix use after
> free issue #2
> 
> As part of commit 5f82e02, ActiveRecordInDb was introduced as
> a copy
> of RecordInDb as latter may be freed by the callback function.
> This
> commit replaces an access of RecordInDb after the callback
> function
> has been executed with an access to ActiveRecordInDb.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
> ---
> 
> QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCS
> mmCore.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git
> a/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QN
> CSmmCore.c
> b/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QN
> CSmmCore.c
> index c2f75f86647a..29ad5f493466 100644
> ---
> a/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QN
> CSmmCore.c
> +++
> b/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QN
> CSmmCore.c
> @@ -758,7 +758,7 @@ QNCSmmCoreDispatcher (
>              }
>            }
> 
> -          if (RecordInDb->ClearSource == NULL) {
> +          if (ActiveRecordInDb.ClearSource == NULL) {
>              //
>              // Clear the SMI associated w/ the source using
> the default function
>              //
> --
> 2.12.2.windows.2

Re: [PATCH] QuarkSocPkg/QNCSmmDispatcher: Fix use after free issue #2

[Kinney, Michael D]

Pushed as 4e33ff75d9

> -----Original Message-----
> From: Kinney, Michael D
> Sent: Wednesday, August 16, 2017 7:41 PM
> To: Marvin Häuser <Marvin.Haeuser@outlook.com>; edk2-
> devel@lists.01.org; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Steele, Kelly <kelly.steele@intel.com>
> Subject: RE: [PATCH] QuarkSocPkg/QNCSmmDispatcher: Fix use
> after free issue #2
> 
> Marvin,
> 
> Thanks for the fix!
> 
> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
> 
> I will push shortly.
> 
> Mike
> 
> > -----Original Message-----
> > From: Marvin Häuser [mailto:Marvin.Haeuser@outlook.com]
> > Sent: Thursday, July 20, 2017 11:12 PM
> > To: edk2-devel@lists.01.org
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Steele,
> > Kelly <kelly.steele@intel.com>
> > Subject: [PATCH] QuarkSocPkg/QNCSmmDispatcher: Fix use after
> > free issue #2
> >
> > As part of commit 5f82e02, ActiveRecordInDb was introduced
> as
> > a copy
> > of RecordInDb as latter may be freed by the callback
> function.
> > This
> > commit replaces an access of RecordInDb after the callback
> > function
> > has been executed with an access to ActiveRecordInDb.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
> > ---
> >
> >
> QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCS
> > mmCore.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git
> >
> a/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QN
> > CSmmCore.c
> >
> b/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QN
> > CSmmCore.c
> > index c2f75f86647a..29ad5f493466 100644
> > ---
> >
> a/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QN
> > CSmmCore.c
> > +++
> >
> b/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QN
> > CSmmCore.c
> > @@ -758,7 +758,7 @@ QNCSmmCoreDispatcher (
> >              }
> >            }
> >
> > -          if (RecordInDb->ClearSource == NULL) {
> > +          if (ActiveRecordInDb.ClearSource == NULL) {
> >              //
> >              // Clear the SMI associated w/ the source using
> > the default function
> >              //
> > --
> > 2.12.2.windows.2