New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 593912 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 593540
Owner: ----
Closed: Mar 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Unitialized warning when running curve25519 under MSan

Project Member Reported by sergeyu@chromium.org, Mar 10 2016

Issue description

When trying to land https://codereview.chromium.org/1759313002/ MSan bot started failing with a warning about uninitialized memory.

  Uninitialized value was stored to memory at
    #0 0x1a6801e in fe_cmov third_party/boringssl/src/crypto/curve25519/curve25519.c:723:10
    #1 0x1a6801e in cmov_cached third_party/boringssl/src/crypto/curve25519/curve25519.c:3559:0
    #2 0x1a6801e in x25519_ge_scalarmult third_party/boringssl/src/crypto/curve25519/curve25519.c:3610:0
    #3 0x19af4b0 in SPAKE2_process_msg third_party/boringssl/src/crypto/curve25519/spake25519.c:430:3
    #4 0x12d3093 in ProcessMessageInternal remoting/protocol/spake2_authenticator.cc:204:18
    ...............................

  Uninitialized value was created by an allocation of 'selected' in the stack frame of function 'x25519_ge_scalarmult'
    #0 0x1a67630 in x25519_ge_scalarmult third_party/boringssl/src/crypto/curve25519/curve25519.c:3567:0

The corresponding code snippet:
    unsigned j;
    ge_cached selected;
    for (j = 0; j < 16; j++) {
      cmov_cached(&selected, &Ai[j], equal(j, index));
    }

Here |selected| is not initialized explicitly, but it's guaranteed to be set by one of the 16 cmov_cached() calls, i.e. it always get initialized. MSan gets confused with how fe_cmov() is implemented:

/* Replace (f,g) with (g,g) if b == 1;
 * replace (f,g) with (f,g) if b == 0.
 *
 * Preconditions: b in {0,1}. */
static void fe_cmov(fe f, const fe g, unsigned b) {
  b = 0-b;
  unsigned i;
  for (i = 0; i < 10; i++) {
    int32_t x = f[i] ^ g[i];
    x &= b;
    f[i] ^= x;
  }
}

This should either be fixed in MSan or explicit initialization should be added to boringssl.
 
Mergedinto: 593540
Status: Duplicate (was: Untriaged)
Someone else beat you to it. :-P Only reason this hasn't gotten resolved yet is the CQ's been exploding.

Sign in to add a comment