intel / intel-ipsec-mb

Intel(R) Multi-Buffer Crypto for IPSec
BSD 3-Clause "New" or "Revised" License
289 stars 87 forks source link

potential access of an uninitialized value #50

Closed ColinIanKing closed 4 years ago

ColinIanKing commented 4 years ago

Maybe a false positive but worth checking out: static analysis with Coverity detected a potential uninitialized scalar variable issue in lib/include/snow3g_common.h:

`` 3225static inline void 3226snow3g_8_buffer_ks_32_8(const snow3g_key_schedule_t pKey, 3227 const void const IV[], 3228 const uint8_t pBufferIn[], 3229 uint8_t pBufferOut[], 3230 uint32_t lengthInBytes) 3231{ 3232 const size_t num_lanes = 8; 3233 const size_t big_block_size = 32; 3234 const size_t small_block_size = SNOW3G_8_BYTES; 3235 const uint32_t bytes = length_find_min(lengthInBytes, num_lanes); 3236 snow3gKeyState8_t ctx; 3237 uint32_t i, bytes_left = bytes & (~(small_block_size - 1)); 3238 3239 / Initialize the schedule from the IV / 3240 snow3gStateInitialize_8(&ctx, pKey, IV[0], IV[1], IV[2], 3241 IV[3], IV[4], IV[5], IV[6], IV[7]); 3242 3243 / Clock FSM and LFSR once, ignore the key stream */ 3244 (void) snow3g_keystream_8_4(&ctx); 3245

  1. Condition bytes_left >= 32UL / big_block_size /, taking true branch.

3246 if (bytes_left >= big_block_size) { 3247 const uint32_t blocks = bytes_left / big_block_size;

  1. var_decl: Declaring variable ks without initializer.

3248 __m256i ks[8]; 3249 3250 length_sub(lengthInBytes, num_lanes, blocks big_block_size); 3251 bytes_left -= blocks big_block_size; 3252 3253 / generates 8 sets at a time on all streams /

  1. Condition i < blocks, taking true branch.

3254 for (i = 0; i < blocks; i++) { 3255 uint32_t j; 3256 3257 snow3g_keystream_8_32(&ctx, ks); 3258

  1. Condition j < 8UL / num_lanes /, taking true branch.

3259 for (j = 0; j < num_lanes; j++) { 3260 const __m256i in_ptr = 3261 (const __m256i )pBufferIn[j]; 3262 const __m256i in_val = 3263 _mm256_loadu_si256(in_ptr);

CID 98739 (#1 of 1): Uninitialized scalar variable (UNINIT)

  1. uninit_use_in_call: Using uninitialized value ks[j]. Field ks[j].elems is uninitialized when calling _mm256_xor_si256.

3264 const __m256i xor_val = 3265 _mm256_xor_si256(in_val, ks[j]);

pablodelara commented 4 years ago

Thanks for reporting! Looks like this is a false positive, as snow3g_keystream_8_32(&ctx, ks) should update ks. @tkanteck , do you agree?

tkanteck commented 4 years ago

Yes, @pablodelara it is a false positive. Yet it is always better to double check. Many thanks @ColinIanKing for reporting it.