Bug 1120937 - Properly initialize string fields from the PKCS#11 test module. r=keeler
authorMike Hommey <mh+mozilla@glandium.org>
Tue, 13 Jan 2015 18:11:29 +0900
changeset 228923 bd34e4330662fdbe9ffc897f055a62c2febff6ad
parent 228922 73b8180cfd7e09aec15204e10b345f283c46d70a
child 228924 e0a40aa0a2816247452a9f80d8f3e3e6d1c31074
push id28274
push usercbook@mozilla.com
push dateFri, 13 Feb 2015 16:03:46 +0000
treeherdermozilla-central@8f9d3dabd12e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1120937
milestone38.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1120937 - Properly initialize string fields from the PKCS#11 test module. r=keeler The string fields need to be padded with spaces, according to what PK11_MakeString does to find the end of the string. While here, factor all the string manipulations in the test module and use some C++ template magic to do the right thing. This changes the static asserts from (with clang): pkcs11testmodule.cpp:45:3: error: static_assert failed "TestManufacturerID too long - make it shorter" static_assert(sizeof(TestManufacturerID) <= sizeof(pInfo->manufacturerID), ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ to: pkcs11testmodule.cpp:46:3: error: static_assert failed "DestSize >= SrcSize - 1" static_assert(DestSize >= SrcSize - 1, "DestSize >= SrcSize - 1"); ^ ~~~~~~~~~~~~~~~~~~~~~~~ pkcs11testmodule.cpp:58:3: note: in instantiation of function template specialization 'CopyString<32, 63>' requested here CopyString(pInfo->manufacturerID, TestManufacturerID); ^ which actually gives more information than before: it gives the length of both buffers.
security/manager/ssl/tests/unit/pkcs11testmodule/pkcs11testmodule.cpp
--- a/security/manager/ssl/tests/unit/pkcs11testmodule/pkcs11testmodule.cpp
+++ b/security/manager/ssl/tests/unit/pkcs11testmodule/pkcs11testmodule.cpp
@@ -30,32 +30,39 @@ CK_RV Test_C_Finalize(CK_VOID_PTR)
   return CKR_OK;
 }
 
 static const CK_VERSION CryptokiVersion = { 2, 2 };
 static const CK_VERSION TestLibraryVersion = { 0, 0 };
 static const char TestLibraryDescription[] = "Test PKCS11 Library";
 static const char TestManufacturerID[] = "Test PKCS11 Manufacturer ID";
 
+/* The dest buffer is one in the CK_INFO or CK_TOKEN_INFO structs.
+ * Those buffers are padded with spaces. DestSize corresponds to the declared
+ * size for those buffers (e.g. 32 for `char foo[32]`).
+ * The src buffer is a string litteral. SrcSize includes the string
+ * termination character (e.g. 4 for `const char foo[] = "foo"` */
+template <size_t DestSize, size_t SrcSize>
+void CopyString(unsigned char (&dest)[DestSize], const char (&src)[SrcSize])
+{
+  static_assert(DestSize >= SrcSize - 1, "DestSize >= SrcSize - 1");
+  memcpy(dest, src, SrcSize - 1);
+  memset(dest + SrcSize - 1, ' ', DestSize - SrcSize + 1);
+}
+
 CK_RV Test_C_GetInfo(CK_INFO_PTR pInfo)
 {
   if (!pInfo) {
     return CKR_ARGUMENTS_BAD;
   }
 
   pInfo->cryptokiVersion = CryptokiVersion;
-  static_assert(sizeof(TestManufacturerID) <= sizeof(pInfo->manufacturerID),
-                "TestManufacturerID too long - make it shorter");
-  memcpy(pInfo->manufacturerID, TestManufacturerID, sizeof(TestManufacturerID));
+  CopyString(pInfo->manufacturerID, TestManufacturerID);
   pInfo->flags = 0; // must be 0
-  static_assert(sizeof(TestLibraryDescription) <=
-                sizeof(pInfo->libraryDescription),
-                "TestLibraryDescription too long - make it shorter");
-  memcpy(pInfo->libraryDescription, TestLibraryDescription,
-         sizeof(TestLibraryDescription));
+  CopyString(pInfo->libraryDescription, TestLibraryDescription);
   pInfo->libraryVersion = TestLibraryVersion;
   return CKR_OK;
 }
 
 CK_RV Test_C_GetFunctionList(CK_FUNCTION_LIST_PTR_PTR)
 {
   return CKR_OK;
 }
@@ -83,23 +90,18 @@ CK_RV Test_C_GetSlotList(CK_BBOOL limitT
 static const char TestSlotDescription[] = "Test PKCS11 Slot";
 
 CK_RV Test_C_GetSlotInfo(CK_SLOT_ID, CK_SLOT_INFO_PTR pInfo)
 {
   if (!pInfo) {
     return CKR_ARGUMENTS_BAD;
   }
 
-  static_assert(sizeof(TestSlotDescription) <= sizeof(pInfo->slotDescription),
-                "TestSlotDescription too long - make it shorter");
-  memcpy(pInfo->slotDescription, TestSlotDescription,
-         sizeof(TestSlotDescription));
-  static_assert(sizeof(TestManufacturerID) <= sizeof(pInfo->manufacturerID),
-                "TestManufacturerID too long - make it shorter");
-  memcpy(pInfo->manufacturerID, TestManufacturerID, sizeof(TestManufacturerID));
+  CopyString(pInfo->slotDescription, TestSlotDescription);
+  CopyString(pInfo->manufacturerID, TestManufacturerID);
   pInfo->flags = (tokenPresent ? CKF_TOKEN_PRESENT : 0) | CKF_REMOVABLE_DEVICE;
   pInfo->hardwareVersion = TestLibraryVersion;
   pInfo->firmwareVersion = TestLibraryVersion;
   return CKR_OK;
 }
 
 // Deliberately include énye to ensure we're handling encoding correctly.
 // The PKCS #11 base specification v2.20 specifies that strings be encoded
@@ -108,25 +110,19 @@ static const char TestTokenLabel[] = "Test PKCS11 Tokeñ Label";
 static const char TestTokenModel[] = "Test Model";
 
 CK_RV Test_C_GetTokenInfo(CK_SLOT_ID, CK_TOKEN_INFO_PTR pInfo)
 {
   if (!pInfo) {
     return CKR_ARGUMENTS_BAD;
   }
 
-  static_assert(sizeof(TestTokenLabel) <= sizeof(pInfo->label),
-                "TestTokenLabel too long - make it shorter");
-  memcpy(pInfo->label, TestTokenLabel, sizeof(TestTokenLabel));
-  static_assert(sizeof(TestManufacturerID) <= sizeof(pInfo->manufacturerID),
-                "TestManufacturerID too long - make it shorter");
-  memcpy(pInfo->manufacturerID, TestManufacturerID, sizeof(TestManufacturerID));
-  static_assert(sizeof(TestTokenModel) <= sizeof(pInfo->model),
-                "TestTokenModel too long - make it shorter");
-  memcpy(pInfo->model, TestTokenModel, sizeof(TestTokenModel));
+  CopyString(pInfo->label, TestTokenLabel);
+  CopyString(pInfo->manufacturerID, TestManufacturerID);
+  CopyString(pInfo->model, TestTokenModel);
   memset(pInfo->serialNumber, 0, sizeof(pInfo->serialNumber));
   pInfo->flags = CKF_TOKEN_INITIALIZED;
   pInfo->ulMaxSessionCount = 1;
   pInfo->ulSessionCount = 0;
   pInfo->ulMaxRwSessionCount = 1;
   pInfo->ulRwSessionCount = 0;
   pInfo->ulMaxPinLen = 4;
   pInfo->ulMinPinLen = 4;