There are some explicit timer(NULL) calls in openssl-1.1.0xx source,
but the dummy timer() wrapper in ConstantTimeClock.c (used by PEI
and SMM module) has no any checks on NULL parameter. This will
cause the memory access issue.
This patch adds the NULL parameter checking in timer() wrapper.

Cc: Ting Ye <ting.ye@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long <qin.long@intel.com>
---
 CryptoPkg/Library/BaseCryptLib/SysCall/ConstantTimeClock.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/CryptoPkg/Library/BaseCryptLib/SysCall/ConstantTimeClock.c b/CryptoPkg/Library/BaseCryptLib/SysCall/ConstantTimeClock.c
index 7f20164999..0cd90434ca 100644
--- a/CryptoPkg/Library/BaseCryptLib/SysCall/ConstantTimeClock.c
+++ b/CryptoPkg/Library/BaseCryptLib/SysCall/ConstantTimeClock.c
@@ -31,8 +31,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 time_t time (time_t *timer)
 {
-  *timer = 0;
-  return *timer;
+  if (timer != NULL) {
+    *timer = 0;
+  }
+  return 0;
 }
 
 struct tm * gmtime (const time_t *timer)
-- 
2.12.2.windows.1

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

On 03/31/17 19:05, Qin Long wrote:
> There are some explicit timer(NULL) calls in openssl-1.1.0xx source,
> but the dummy timer() wrapper in ConstantTimeClock.c (used by PEI
> and SMM module) has no any checks on NULL parameter. This will
> cause the memory access issue.
> This patch adds the NULL parameter checking in timer() wrapper.
> 
> Cc: Ting Ye <ting.ye@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Qin Long <qin.long@intel.com>
> ---
>  CryptoPkg/Library/BaseCryptLib/SysCall/ConstantTimeClock.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/CryptoPkg/Library/BaseCryptLib/SysCall/ConstantTimeClock.c b/CryptoPkg/Library/BaseCryptLib/SysCall/ConstantTimeClock.c
> index 7f20164999..0cd90434ca 100644
> --- a/CryptoPkg/Library/BaseCryptLib/SysCall/ConstantTimeClock.c
> +++ b/CryptoPkg/Library/BaseCryptLib/SysCall/ConstantTimeClock.c
> @@ -31,8 +31,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>  
>  time_t time (time_t *timer)
>  {
> -  *timer = 0;
> -  return *timer;
> +  if (timer != NULL) {
> +    *timer = 0;
> +  }
> +  return 0;
>  }
>  
>  struct tm * gmtime (const time_t *timer)
> 

This looks okay, except the function is called time(), not timer().
Please update the commit message (both subject line and body -- several
instances).

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

Got it. Will correct the commit message.
Thanks.

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Saturday, April 01, 2017 2:46 AM
> To: Long, Qin; edk2-devel@lists.01.org
> Cc: Ye, Ting; Wu, Hao A; Tian, Feng; Dong, Eric
> Subject: Re: [Patch 3/4] CryptoPkg/BaseCryptLib: Adding NULL checking in
> timer() wrapper.
> 
> On 03/31/17 19:05, Qin Long wrote:
> > There are some explicit timer(NULL) calls in openssl-1.1.0xx source,
> > but the dummy timer() wrapper in ConstantTimeClock.c (used by PEI and
> > SMM module) has no any checks on NULL parameter. This will cause the
> > memory access issue.
> > This patch adds the NULL parameter checking in timer() wrapper.
> >
> > Cc: Ting Ye <ting.ye@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Qin Long <qin.long@intel.com>
> > ---
> >  CryptoPkg/Library/BaseCryptLib/SysCall/ConstantTimeClock.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git
> > a/CryptoPkg/Library/BaseCryptLib/SysCall/ConstantTimeClock.c
> > b/CryptoPkg/Library/BaseCryptLib/SysCall/ConstantTimeClock.c
> > index 7f20164999..0cd90434ca 100644
> > --- a/CryptoPkg/Library/BaseCryptLib/SysCall/ConstantTimeClock.c
> > +++ b/CryptoPkg/Library/BaseCryptLib/SysCall/ConstantTimeClock.c
> > @@ -31,8 +31,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
> >
> >  time_t time (time_t *timer)
> >  {
> > -  *timer = 0;
> > -  return *timer;
> > +  if (timer != NULL) {
> > +    *timer = 0;
> > +  }
> > +  return 0;
> >  }
> >
> >  struct tm * gmtime (const time_t *timer)
> >
> 
> This looks okay, except the function is called time(), not timer().
> Please update the commit message (both subject line and body -- several
> instances).
> 
> Thanks,
> Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel