Discussion:
[edk2] [patch] CryptoPkg: Add some comments for API usage clarification.
Qin Long
2015-06-18 07:29:21 UTC
Permalink
This patch adds some comments for API usage clarification, and
adds one object initialization in X509ConstructCertificateStack
implementation to fix possible memory release issue.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long <***@intel.com>
---
CryptoPkg/Include/Library/BaseCryptLib.h | 4 +++-
CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaExt.c | 4 +++-
CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 3 ++-
CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c | 2 +-
CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c | 2 +-
5 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include/Library/BaseCryptLib.h
index 364fa3c..d3b211b 100644
--- a/CryptoPkg/Include/Library/BaseCryptLib.h
+++ b/CryptoPkg/Include/Library/BaseCryptLib.h
@@ -1631,6 +1631,8 @@ RsaGenerateKey (

/**
Validates key components of RSA context.
+ NOTE: This function performs integrity checks on all the RSA key material, so
+ the RSA key structure must contain all the private key data.

This function validates key compoents of RSA context in following aspects:
- Whether p is a prime
@@ -1859,7 +1861,7 @@ X509ConstructCertificate (
If X509Stack is NULL, then return FALSE.
If this interface is not supported, then return FALSE.

- @param[in, out] X509Stack On input, pointer to an existing X509 stack object.
+ @param[in, out] X509Stack On input, pointer to an existing or NULL X509 stack object.
On output, pointer to the X509 stack object with new
inserted X509 certificate.
@param ... A list of DER-encoded single certificate data followed
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaExt.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaExt.c
index 5c21d12..b890704 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaExt.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaExt.c
@@ -243,7 +243,9 @@ _Exit:
}

/**
- Validates key components of RSA context.
+ Validates key components of RSA context.
+ NOTE: This function performs integrity checks on all the RSA key material, so
+ the RSA key structure must contain all the private key data.

This function validates key compoents of RSA context in following aspects:
- Whether p is a prime
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
index 02851d5..70b135a 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
@@ -67,7 +67,7 @@ X509ConstructCertificate (

If X509Stack is NULL, then return FALSE.

- @param[in, out] X509Stack On input, pointer to an existing X509 stack object.
+ @param[in, out] X509Stack On input, pointer to an existing or NULL X509 stack object.
On output, pointer to the X509 stack object with new
inserted X509 certificate.
@param ... A list of DER-encoded single certificate data followed
@@ -132,6 +132,7 @@ X509ConstructCertificateStack (
//
// Construct X509 Object from the given DER-encoded certificate data.
//
+ X509Cert = NULL;
Status = X509ConstructCertificate (
(CONST UINT8 *) Cert,
CertSize,
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
index e1eb84d..51aa063 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
@@ -44,7 +44,7 @@ X509ConstructCertificate (

Return FALSE to indicate this interface is not supported.

- @param[in, out] X509Stack On input, pointer to an existing X509 stack object.
+ @param[in, out] X509Stack On input, pointer to an existing or NULL X509 stack object.
On output, pointer to the X509 stack object with new
inserted X509 certificate.
@param ... A list of DER-encoded single certificate data followed
diff --git a/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c b/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c
index c43ca07..f5d9aa1 100644
--- a/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c
+++ b/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c
@@ -44,7 +44,7 @@ X509ConstructCertificate (

Return FALSE to indicate this interface is not supported.

- @param[in, out] X509Stack On input, pointer to an existing X509 stack object.
+ @param[in, out] X509Stack On input, pointer to an existing or NULL X509 stack object.
On output, pointer to the X509 stack object with new
inserted X509 certificate.
@param ... A list of DER-encoded single certificate data followed
--
1.9.5.msysgit.1


------------------------------------------------------------------------------
Ye, Ting
2015-06-18 07:34:19 UTC
Permalink
Looks good.

Reviewed-by: Ye Ting <***@intel.com>



-----Original Message-----
From: Long, Qin
Sent: Thursday, June 18, 2015 3:29 PM
To: Ye, Ting
Cc: edk2-***@lists.sourceforge.net
Subject: [patch] CryptoPkg: Add some comments for API usage clarification.

This patch adds some comments for API usage clarification, and
adds one object initialization in X509ConstructCertificateStack
implementation to fix possible memory release issue.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long <***@intel.com>
---
CryptoPkg/Include/Library/BaseCryptLib.h | 4 +++-
CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaExt.c | 4 +++-
CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 3 ++-
CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c | 2 +-
CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c | 2 +-
5 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include/Library/BaseCryptLib.h
index 364fa3c..d3b211b 100644
--- a/CryptoPkg/Include/Library/BaseCryptLib.h
+++ b/CryptoPkg/Include/Library/BaseCryptLib.h
@@ -1631,6 +1631,8 @@ RsaGenerateKey (

/**
Validates key components of RSA context.
+ NOTE: This function performs integrity checks on all the RSA key material, so
+ the RSA key structure must contain all the private key data.

This function validates key compoents of RSA context in following aspects:
- Whether p is a prime
@@ -1859,7 +1861,7 @@ X509ConstructCertificate (
If X509Stack is NULL, then return FALSE.
If this interface is not supported, then return FALSE.

- @param[in, out] X509Stack On input, pointer to an existing X509 stack object.
+ @param[in, out] X509Stack On input, pointer to an existing or NULL X509 stack object.
On output, pointer to the X509 stack object with new
inserted X509 certificate.
@param ... A list of DER-encoded single certificate data followed
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaExt.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaExt.c
index 5c21d12..b890704 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaExt.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaExt.c
@@ -243,7 +243,9 @@ _Exit:
}

/**
- Validates key components of RSA context.
+ Validates key components of RSA context.
+ NOTE: This function performs integrity checks on all the RSA key material, so
+ the RSA key structure must contain all the private key data.

This function validates key compoents of RSA context in following aspects:
- Whether p is a prime
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
index 02851d5..70b135a 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
@@ -67,7 +67,7 @@ X509ConstructCertificate (

If X509Stack is NULL, then return FALSE.

- @param[in, out] X509Stack On input, pointer to an existing X509 stack object.
+ @param[in, out] X509Stack On input, pointer to an existing or NULL X509 stack object.
On output, pointer to the X509 stack object with new
inserted X509 certificate.
@param ... A list of DER-encoded single certificate data followed
@@ -132,6 +132,7 @@ X509ConstructCertificateStack (
//
// Construct X509 Object from the given DER-encoded certificate data.
//
+ X509Cert = NULL;
Status = X509ConstructCertificate (
(CONST UINT8 *) Cert,
CertSize,
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
index e1eb84d..51aa063 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
@@ -44,7 +44,7 @@ X509ConstructCertificate (

Return FALSE to indicate this interface is not supported.

- @param[in, out] X509Stack On input, pointer to an existing X509 stack object.
+ @param[in, out] X509Stack On input, pointer to an existing or NULL X509 stack object.
On output, pointer to the X509 stack object with new
inserted X509 certificate.
@param ... A list of DER-encoded single certificate data followed
diff --git a/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c b/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c
index c43ca07..f5d9aa1 100644
--- a/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c
+++ b/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c
@@ -44,7 +44,7 @@ X509ConstructCertificate (

Return FALSE to indicate this interface is not supported.

- @param[in, out] X509Stack On input, pointer to an existing X509 stack object.
+ @param[in, out] X509Stack On input, pointer to an existing or NULL X509 stack object.
On output, pointer to the X509 stack object with new
inserted X509 certificate.
@param ... A list of DER-encoded single certificate data followed
--
1.9.5.msysgit.1


------------------------------------------------------------------------------
Loading...