CryptoPkg/BaseCryptLib: Fix mismatched memory allocation/free

The malloc/free (instead of AllocatePool/FreePool) were used directly
in some wrapper implementations, which was designed to leverage the
light-weight memory management routines at Runtime phase.
The malloc/free and AllocatePool/FreePool usages are required to be
matched, after extra memory size info header was introduced in malloc
wrapper.

This patch corrects two memory allocation cases, which requires the
caller to free the buffer with FreePool() outside the function call.

And some comments were also added to clarify the correct memory
release functions if it's the caller's responsibility to free the
memory buffer.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ting Ye <ting.ye@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long <qin.long@intel.com>
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>

diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include/Library/BaseCryptLib.h
index 5f67ecb709e06e92d2d2898833e1013cc3b37c25..e2b6a956664b96ee8e50f8c75d59a24c7b926f42 100644 (file)
--- a/CryptoPkg/Include/Library/BaseCryptLib.h
+++ b/CryptoPkg/Include/Library/BaseCryptLib.h
@@ -2388,10 +2388,12 @@ Pkcs5HashPassword (
   @param[in]  P7Data       Pointer to the PKCS#7 message to verify.\r
   @param[in]  P7Length     Length of the PKCS#7 message in bytes.\r
   @param[out] CertStack    Pointer to Signer's certificates retrieved from P7Data.\r
-                           It's caller's responsibility to free the buffer.\r
+                           It's caller's responsibility to free the buffer with\r
+                           Pkcs7FreeSigners().\r
   @param[out] StackLength  Length of signer's certificates in bytes.\r
   @param[out] TrustedCert  Pointer to a trusted certificate from Signer's certificates.\r
-                           It's caller's responsibility to free the buffer.\r
+                           It's caller's responsibility to free the buffer with\r
+                           Pkcs7FreeSigners().\r
   @param[out] CertLength   Length of the trusted certificate in bytes.\r
 \r
   @retval  TRUE            The operation is finished successfully.\r
@@ -2433,10 +2435,11 @@ Pkcs7FreeSigners (
   @param[in]  P7Data            Pointer to the PKCS#7 message.\r
   @param[in]  P7Length          Length of the PKCS#7 message in bytes.\r
   @param[out] SignerChainCerts  Pointer to the certificates list chained to signer's\r
-                                certificate. It's caller's responsibility to free the buffer.\r
+                                certificate. It's caller's responsibility to free the buffer\r
+                                with Pkcs7FreeSigners().\r
   @param[out] ChainLength       Length of the chained certificates list buffer in bytes.\r
   @param[out] UnchainCerts      Pointer to the unchained certificates lists. It's caller's\r
-                                responsibility to free the buffer.\r
+                                responsibility to free the buffer with Pkcs7FreeSigners().\r
   @param[out] UnchainLength     Length of the unchained certificates list buffer in bytes.\r
 \r
   @retval  TRUE         The operation is finished successfully.\r
@@ -2472,7 +2475,8 @@ Pkcs7GetCertificatesList (
   @param[in]  OtherCerts       Pointer to an optional additional set of certificates to\r
                                include in the PKCS#7 signedData (e.g. any intermediate\r
                                CAs in the chain).\r
-  @param[out] SignedData       Pointer to output PKCS#7 signedData.\r
+  @param[out] SignedData       Pointer to output PKCS#7 signedData. It's caller's\r
+                               responsibility to free the buffer with FreePool().\r
   @param[out] SignedDataSize   Size of SignedData in bytes.\r
 \r
   @retval     TRUE             PKCS#7 data signing succeeded.\r
@@ -2540,7 +2544,7 @@ Pkcs7Verify (
   @param[in]   P7Data       Pointer to the PKCS#7 signed data to process.\r
   @param[in]   P7Length     Length of the PKCS#7 signed data in bytes.\r
   @param[out]  Content      Pointer to the extracted content from the PKCS#7 signedData.\r
-                            It's caller's responsibility to free the buffer.\r
+                            It's caller's responsibility to free the buffer with FreePool().\r
   @param[out]  ContentSize  The size of the extracted content in bytes.\r
 \r
   @retval     TRUE          The P7Data was correctly formatted for processing.\r

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Sign.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Sign.c
index d3b1a907aad9c5ae8138f0c242f15e56e633da0b..0f61d4b4ad4b9ce1f6b919e95c26100fae24cf16 100644 (file)
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Sign.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Sign.c
@@ -34,7 +34,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
   @param[in]  OtherCerts       Pointer to an optional additional set of certificates to\r
                                include in the PKCS#7 signedData (e.g. any intermediate\r
                                CAs in the chain).\r
-  @param[out] SignedData       Pointer to output PKCS#7 signedData.\r
+  @param[out] SignedData       Pointer to output PKCS#7 signedData. It's caller's\r
+                               responsibility to free the buffer with FreePool().\r
   @param[out] SignedDataSize   Size of SignedData in bytes.\r
 \r
   @retval     TRUE             PKCS#7 data signing succeeded.\r
@@ -167,7 +168,7 @@ Pkcs7Sign (
   // is totally 19 bytes.\r
   //\r
   *SignedDataSize = P7DataSize - 19;\r
-  *SignedData     = malloc (*SignedDataSize);\r
+  *SignedData     = AllocatePool (*SignedDataSize);\r
   if (*SignedData == NULL) {\r
     OPENSSL_free (P7Data);\r
     goto _Exit;\r

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7SignNull.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7SignNull.c
index 539bb6b7d542e12f60e2b4d82b56620c7a33480b..1ce7202d911de2eb2caf9fbcfa4daeb54d99278e 100644 (file)
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7SignNull.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7SignNull.c
@@ -33,7 +33,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
   @param[in]  OtherCerts       Pointer to an optional additional set of certificates to\r
                                include in the PKCS#7 signedData (e.g. any intermediate\r
                                CAs in the chain).\r
-  @param[out] SignedData       Pointer to output PKCS#7 signedData.\r
+  @param[out] SignedData       Pointer to output PKCS#7 signedData. It's caller's\r
+                               responsibility to free the buffer with FreePool().\r
   @param[out] SignedDataSize   Size of SignedData in bytes.\r
 \r
   @retval FALSE  This interface is not supported.\r

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c
index bf67a1f569a2b664ecac8d4d3ab2e1a3d3afe15d..296df028b130a45d7f1562197ea1d3b31e2ff42d 100644 (file)
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c
@@ -240,10 +240,12 @@ _Exit:
   @param[in]  P7Data       Pointer to the PKCS#7 message to verify.\r
   @param[in]  P7Length     Length of the PKCS#7 message in bytes.\r
   @param[out] CertStack    Pointer to Signer's certificates retrieved from P7Data.\r
-                           It's caller's responsibility to free the buffer.\r
+                           It's caller's responsibility to free the buffer with\r
+                           Pkcs7FreeSigners().\r
   @param[out] StackLength  Length of signer's certificates in bytes.\r
   @param[out] TrustedCert  Pointer to a trusted certificate from Signer's certificates.\r
-                           It's caller's responsibility to free the buffer.\r
+                           It's caller's responsibility to free the buffer with\r
+                           Pkcs7FreeSigners().\r
   @param[out] CertLength   Length of the trusted certificate in bytes.\r
 \r
   @retval  TRUE            The operation is finished successfully.\r
@@ -438,10 +440,11 @@ Pkcs7FreeSigners (
   @param[in]  P7Data            Pointer to the PKCS#7 message.\r
   @param[in]  P7Length          Length of the PKCS#7 message in bytes.\r
   @param[out] SignerChainCerts  Pointer to the certificates list chained to signer's\r
-                                certificate. It's caller's responsibility to free the buffer.\r
+                                certificate. It's caller's responsibility to free the buffer\r
+                                with Pkcs7FreeSigners().\r
   @param[out] ChainLength       Length of the chained certificates list buffer in bytes.\r
   @param[out] UnchainCerts      Pointer to the unchained certificates lists. It's caller's\r
-                                responsibility to free the buffer.\r
+                                responsibility to free the buffer with Pkcs7FreeSigners().\r
   @param[out] UnchainLength     Length of the unchained certificates list buffer in bytes.\r
 \r
   @retval  TRUE         The operation is finished successfully.\r
@@ -921,7 +924,7 @@ _Exit:
   @param[in]   P7Data       Pointer to the PKCS#7 signed data to process.\r
   @param[in]   P7Length     Length of the PKCS#7 signed data in bytes.\r
   @param[out]  Content      Pointer to the extracted content from the PKCS#7 signedData.\r
-                            It's caller's responsibility to free the buffer.\r
+                            It's caller's responsibility to free the buffer with FreePool().\r
   @param[out]  ContentSize  The size of the extracted content in bytes.\r
 \r
   @retval     TRUE          The P7Data was correctly formatted for processing.\r
@@ -996,7 +999,7 @@ Pkcs7GetAttachedContent (
     OctStr = Pkcs7->d.sign->contents->d.data;\r
     if ((OctStr->length > 0) && (OctStr->data != NULL)) {\r
       *ContentSize = OctStr->length;\r
-      *Content     = malloc (*ContentSize);\r
+      *Content     = AllocatePool (*ContentSize);\r
       if (*Content == NULL) {\r
         *ContentSize = 0;\r
         goto _Exit;\r

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c
index 06602ec53572a39f1bb6cf892333b4edf243659a..d3e8ec89a74b95c4503272626154fbf3276e75d3 100644 (file)
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c
@@ -25,10 +25,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
   @param[in]  P7Data       Pointer to the PKCS#7 message to verify.\r
   @param[in]  P7Length     Length of the PKCS#7 message in bytes.\r
   @param[out] CertStack    Pointer to Signer's certificates retrieved from P7Data.\r
-                           It's caller's responsibility to free the buffer.\r
+                           It's caller's responsibility to free the buffer with\r
+                           Pkcs7FreeSigners().\r
   @param[out] StackLength  Length of signer's certificates in bytes.\r
   @param[out] TrustedCert  Pointer to a trusted certificate from Signer's certificates.\r
-                           It's caller's responsibility to free the buffer.\r
+                           It's caller's responsibility to free the buffer with\r
+                           Pkcs7FreeSigners().\r
   @param[out] CertLength   Length of the trusted certificate in bytes.\r
 \r
   @retval FALSE  This interface is not supported.\r
@@ -75,10 +77,11 @@ Pkcs7FreeSigners (
   @param[in]  P7Data            Pointer to the PKCS#7 message.\r
   @param[in]  P7Length          Length of the PKCS#7 message in bytes.\r
   @param[out] SignerChainCerts  Pointer to the certificates list chained to signer's\r
-                                certificate. It's caller's responsibility to free the buffer.\r
+                                certificate. It's caller's responsibility to free the buffer\r
+                                with Pkcs7FreeSigners().\r
   @param[out] ChainLength       Length of the chained certificates list buffer in bytes.\r
   @param[out] UnchainCerts      Pointer to the unchained certificates lists. It's caller's\r
-                                responsibility to free the buffer.\r
+                                responsibility to free the buffer with Pkcs7FreeSigners().\r
   @param[out] UnchainLength     Length of the unchained certificates list buffer in bytes.\r
 \r
   @retval  TRUE         The operation is finished successfully.\r
@@ -142,7 +145,7 @@ Pkcs7Verify (
   @param[in]   P7Data       Pointer to the PKCS#7 signed data to process.\r
   @param[in]   P7Length     Length of the PKCS#7 signed data in bytes.\r
   @param[out]  Content      Pointer to the extracted content from the PKCS#7 signedData.\r
-                            It's caller's responsibility to free the buffer.\r
+                            It's caller's responsibility to free the buffer with FreePool().\r
   @param[out]  ContentSize  The size of the extracted content in bytes.\r
 \r
   @retval     TRUE          The P7Data was correctly formatted for processing.\r