commit 2df010e4f4eae6806cd1d855b63c3891116f065d
author David Benjamin <davidben@google.com> Wed Jan 04 07:28:28 2017 -0500
committer Adam Langley <agl@google.com> Thu Jan 12 16:45:23 2017 +0000
tree e78f9c0b3ff15423489244f74475e157f4745ea2
parent 0a211dfe91588d2986a8735e1969dd9202a8b025

Remove 'pivot element' from wNAF code.

Resolving the TODO here will be messier than the other implementations
but, to start with, remove this 'pivot element' thing. All that is just
to free some array contents without having to memset the whole thing to
zero.

Change-Id: Ifd6ee0b3815006d4f1f19c9db085cb842671c6dc
Reviewed-on: https://boringssl-review.googlesource.com/13057
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>

crypto/ec/wnaf.c

diff --git a/crypto/ec/wnaf.c b/crypto/ec/wnaf.c
index 1594354..67b7f34 100644
--- a/crypto/ec/wnaf.c
+++ b/crypto/ec/wnaf.c
@@ -242,7 +242,7 @@
   BN_CTX *new_ctx = NULL;
   const EC_POINT *generator = NULL;
   EC_POINT *tmp = NULL;
-  size_t total_num;
+  size_t total_num = 0;
   size_t i, j;
   int k;
   int r_is_inverted = 0;
@@ -251,7 +251,7 @@
   int8_t **wNAF = NULL; /* individual wNAFs */
   size_t *wNAF_len = NULL;
   size_t max_len = 0;
-  size_t num_val;
+  size_t num_val = 0;
   EC_POINT **val = NULL; /* precomputation */
   EC_POINT **v;
   EC_POINT ***val_sub = NULL; /* pointers to sub-arrays of 'val' */
@@ -284,15 +284,14 @@
   }
 
 
-  wsize = OPENSSL_malloc(total_num * sizeof wsize[0]);
-  wNAF_len = OPENSSL_malloc(total_num * sizeof wNAF_len[0]);
-  wNAF = OPENSSL_malloc((total_num + 1) *
-                        sizeof wNAF[0]); /* includes space for pivot */
-  val_sub = OPENSSL_malloc(total_num * sizeof val_sub[0]);
+  wsize = OPENSSL_malloc(total_num * sizeof(wsize[0]));
+  wNAF_len = OPENSSL_malloc(total_num * sizeof(wNAF_len[0]));
+  wNAF = OPENSSL_malloc(total_num * sizeof(wNAF[0]));
+  val_sub = OPENSSL_malloc(total_num * sizeof(val_sub[0]));
 
   /* Ensure wNAF is initialised in case we end up going to err. */
-  if (wNAF) {
-    wNAF[0] = NULL; /* preliminary pivot */
+  if (wNAF != NULL) {
+    OPENSSL_memset(wNAF, 0, total_num * sizeof(wNAF[0]));
   }
 
   if (!wsize || !wNAF_len || !wNAF || !val_sub) {
@@ -309,7 +308,6 @@
     bits = i < num ? BN_num_bits(scalars[i]) : BN_num_bits(g_scalar);
     wsize[i] = window_bits_for_scalar_size(bits);
     num_val += (size_t)1 << (wsize[i] - 1);
-    wNAF[i + 1] = NULL; /* make sure we always have a pivot */
     wNAF[i] =
         compute_wNAF((i < num ? scalars[i] : g_scalar), wsize[i], &wNAF_len[i]);
     if (wNAF[i] == NULL) {
@@ -322,12 +320,12 @@
 
   /* All points we precompute now go into a single array 'val'. 'val_sub[i]' is
    * a pointer to the subarray for the i-th point. */
-  val = OPENSSL_malloc((num_val + 1) * sizeof val[0]);
+  val = OPENSSL_malloc(num_val * sizeof(val[0]));
   if (val == NULL) {
     OPENSSL_PUT_ERROR(EC, ERR_R_MALLOC_FAILURE);
     goto err;
   }
-  val[num_val] = NULL; /* pivot element */
+  OPENSSL_memset(val, 0, num_val * sizeof(val[0]));
 
   /* allocate points for precomputation */
   v = val;
@@ -442,17 +440,15 @@
   OPENSSL_free(wsize);
   OPENSSL_free(wNAF_len);
   if (wNAF != NULL) {
-    int8_t **w;
-
-    for (w = wNAF; *w != NULL; w++) {
-      OPENSSL_free(*w);
+    for (i = 0; i < total_num; i++) {
+      OPENSSL_free(wNAF[i]);
     }
 
     OPENSSL_free(wNAF);
   }
   if (val != NULL) {
-    for (v = val; *v != NULL; v++) {
-      EC_POINT_clear_free(*v);
+    for (i = 0; i < num_val; i++) {
+      EC_POINT_clear_free(val[i]);
     }
 
     OPENSSL_free(val);