intel / intel-ipsec-mb

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

static analysis warnings on today's tip #73

Closed ColinIanKing closed 3 years ago

ColinIanKing commented 3 years ago

Hi,

I just completed a build on today's tip with Coverity static analysis and it found some potential issues. I suspect these may be false positives, but I thought I should report them:

File: lib/include/kasumi_internal.h

 850 static inline void
 851 kasumi_f8_1_buffer_bit(const kasumi_key_sched_t *pCtx, const uint64_t IV,
 852                       const void *pIn, void *pOut,
 853                       const uint32_t lengthInBits,
 854                       const uint32_t offsetInBits)
 855 {
 856 #ifdef SAFE_DATA
 857        CLEAR_SCRATCH_SIMD_REGS();
 858 #endif /* SAFE_DATA */
 859
 860        const uint8_t *pBufferIn = (const uint8_t *) pIn;
 861        uint8_t *pBufferOut = (uint8_t *) pOut;
 862        uint32_t cipherLengthInBits = lengthInBits;
 863        uint32_t blkcnt;
 864        uint64_t shiftrem = 0;
 865        kasumi_union_t a, b, c; /* the modifier */
 866        const uint8_t *pcBufferIn = pBufferIn + (offsetInBits / 8);
 867        uint8_t *pcBufferOut = pBufferOut + (offsetInBits / 8);
 868        /* Offset into the first byte (0 - 7 bits) */
 869        uint32_t remainOffset = offsetInBits % 8;
 870        uint32_t byteLength = (cipherLengthInBits + 7) / 8;
 871        SafeBuf safeOutBuf = {0};

    1. var_decl: Declaring variable safeInBuf without initializer.

 872        SafeBuf safeInBuf;
 873
 874        /* IV Endianity  */
 875        a.b64[0] = BSWAP64(IV);
 876
 877        /* First encryption to create modifier */
 878        kasumi_1_block(pCtx->msk16, a.b16);
 879
 880        /* Final initialisation steps */
 881        blkcnt = 0;
 882        b.b64[0] = a.b64[0];
 883        /* Now run the block cipher */
 884
 885        /* Start with potential partial block (due to offset and length) */
 886        kasumi_1_block(pCtx->sk16, b.b16);
 887        c.b64[0] = b.b64[0] >> remainOffset;
 888        /* Only one block to encrypt */

    2. Condition cipherLengthInBits < 64 - remainOffset, taking true branch.

 889        if (cipherLengthInBits < (64 - remainOffset)) {
 890                byteLength = (cipherLengthInBits + 7) / 8;
 891                memcpy_keystrm(safeInBuf.b8, pcBufferIn, byteLength);
 892                /*
 893                 * If operation is Out-of-place and there is offset
 894                 * to be applied, "remainOffset" bits from the output buffer
 895                 * need to be preserved (only applicable to first byte,
 896                 * since remainOffset is up to 7 bits)
 897                 */

    3. Condition pIn != pOut, taking true branch.
    4. Condition remainOffset, taking true branch.

 898                if ((pIn != pOut) && remainOffset) {
 899                        const uint8_t mask8 =
 900                                (const uint8_t)(1 << (8 - remainOffset)) - 1;
 901

    CID 102314 (#1 of 1): Uninitialized scalar variable (UNINIT)5. uninit_use: Using uninitialized value safeInBuf.b8[0].

 902                        safeInBuf.b8[0] = (safeInBuf.b8[0] & mask8) |
 903                                        (pcBufferOut[0] & ~mask8);
 904                }
 905
 906                /* If last byte is a partial byte, the last bits of the output
 907                 * need to be preserved */
 908                const uint8_t bitlen_with_off = remainOffset +
 909                                        cipherLengthInBits;
 910
 911                if ((bitlen_with_off & 0x7) != 0) {
 912                        preserve_bits(&c, pcBufferOut, pcBufferIn, &safeOutBuf,
 913                                      &safeInBuf, bitlen_with_off, byteLength);
 914                }
 915                xor_keystrm_rev(safeOutBuf.b8, safeInBuf.b8, c.b64[0]);
 916                memcpy_keystrm(pcBufferOut, safeOutBuf.b8, byteLength);
 917                return;
 918        }

File: test/main.c:


123 int
124 main(int argc, char **argv)
125 {

   1. var_decl: Declaring variable arch_support without initializer.

126        uint8_t arch_support[IMB_ARCH_NUM];
127        int i, atype, auto_detect = 0;
128        uint64_t flags = 0;
129        int errors = 0;
130
131        /* Check version number */

   2. Condition imb_get_version() < 12800U /* (0 << 16) + (0x32 << 8) + 0 */, taking false branch.

132        if (imb_get_version() < IMB_VERSION(0, 50, 0))
133                printf("Library version detection unsupported!\n");
134        else
135                printf("Detected library version: %s\n", imb_get_version_str());
136
137        /* Print available CPU features */
138        print_hw_features();
139
140        /* Detect available architectures and features */

   3. Condition detect_arch(arch_support) < 0, taking false branch.

141        if (detect_arch(arch_support) < 0)
142                return EXIT_FAILURE;
143

   4. Condition i < argc, taking true branch.
   8. Condition i < argc, taking true branch.
   12. Condition i < argc, taking true branch.
   16. Condition i < argc, taking false branch.

144        for (i = 1; i < argc; i++) {

   5. Condition strcmp(argv[i], "-h") == 0, taking false branch.
   9. Condition strcmp(argv[i], "-h") == 0, taking false branch.
   13. Condition strcmp(argv[i], "-h") == 0, taking false branch.

145                if (strcmp(argv[i], "-h") == 0) {
146                        usage(argv[0]);
147                        return EXIT_SUCCESS;

    6. Condition update_flags_and_archs(argv[i], arch_support, &flags), taking true branch.
   10. Condition update_flags_and_archs(argv[i], arch_support, &flags), taking true branch.
   14. Condition update_flags_and_archs(argv[i], arch_support, &flags), taking true branch.

148                } else if (update_flags_and_archs(argv[i],
149                                                   arch_support,
150                                                   &flags))

   7. Continuing loop.
   11. Continuing loop.
   15. Continuing loop.

151                        continue;
152                else if (strcmp(argv[i], "--auto-detect") == 0)
153                        (void) auto_detect; /* legacy option - to be removed */
154                else {
155                        usage(argv[0]);
156                        return EXIT_FAILURE;
157                }
158        }
159
160        /* Go through architectures */

   17. Condition atype < IMB_ARCH_NUM, taking true branch.

161        for (atype = IMB_ARCH_NOAESNI; atype < IMB_ARCH_NUM; atype++) {
162                IMB_MGR *p_mgr = NULL;
163

   CID 106252 (#1 of 1): Uninitialized scalar variable (UNINIT)18. uninit_use: Using uninitialized value arch_support[atype].

164                if (!arch_support[atype])
165                        continue;

File: lib/avx512/zuc_avx512_top.c

261 static inline
262 void _zuc_eea3_16_buffer_avx512(const void * const pKey[NUM_AVX512_BUFS],
263                                const void * const pIv[NUM_AVX512_BUFS],
264                                const void * const pBufferIn[NUM_AVX512_BUFS],
265                                void *pBufferOut[NUM_AVX512_BUFS],
266                                const uint32_t length[NUM_AVX512_BUFS],
267                                const unsigned use_gfni)
268 {

   1. var_decl: Declaring variable state without initializer.

269        DECLARE_ALIGNED(ZucState16_t state, 64);
270        DECLARE_ALIGNED(ZucState_t singlePktState, 64);
271        unsigned int i = 0;
272        /* Calculate the minimum input packet size from all packets */
273        uint16_t bytes = (uint16_t) find_min_length32(length);
274
275        uint32_t numKeyStreamsPerPkt;
276        uint16_t remainBytes[NUM_AVX512_BUFS] = {0};
277        DECLARE_ALIGNED(uint8_t keyStr[NUM_AVX512_BUFS][64], 64);
278        /* structure to store the 16 keys */
279        DECLARE_ALIGNED(ZucKey16_t keys, 64);
280        /* structure to store the 16 IV's */
281        DECLARE_ALIGNED(ZucIv16_t ivs, 64);
282        uint32_t numBytesLeftOver = 0;
283        const uint8_t *pTempBufInPtr = NULL;
284        uint8_t *pTempBufOutPtr = NULL;
285
286        const uint64_t *pIn64[NUM_AVX512_BUFS]= {NULL};
287        uint64_t *pOut64[NUM_AVX512_BUFS] = {NULL};
288        uint64_t *pKeyStream64 = NULL;
289
290        /*
291         * Calculate the number of bytes left over for each packet,
292         * and setup the Keys and IVs
293         */

   2. Condition i < 16, taking true branch.
   4. Condition i < 16, taking true branch.
   6. Condition i < 16, taking false branch.

294        for (i = 0; i < NUM_AVX512_BUFS; i++) {
295                remainBytes[i] = length[i];
296                keys.pKeys[i] = pKey[i];
297                ivs.pIvs[i] = pIv[i];

   3. Jumping back to the beginning of the loop.
   5. Jumping back to the beginning of the loop.

298        }
299
300        init_16(&keys, &ivs, &state, 0xFFFF, use_gfni);
301

   7. Condition i < 16, taking true branch.
   9. Condition i < 16, taking true branch.
   11. Condition i < 16, taking false branch.

302        for (i = 0; i < NUM_AVX512_BUFS; i++) {
303                pOut64[i] = (uint64_t *) pBufferOut[i];
304                pIn64[i] = (const uint64_t *) pBufferIn[i];

   8. Jumping back to the beginning of the loop.
   10. Jumping back to the beginning of the loop.

305        }
306
307        cipher_16(&state, pIn64, pOut64, remainBytes, bytes, use_gfni);
308
309        /* process each packet separately for the remaining bytes */

   12. Condition i < 16, taking true branch.

310        for (i = 0; i < NUM_AVX512_BUFS; i++) {

   13. Condition remainBytes[i], taking true branch.

311                if (remainBytes[i]) {
312                        /* need to copy the zuc state to single packet state */

   CID 106253 (#3 of 3): Uninitialized scalar variable (UNINIT) [select issue]

313                        singlePktState.lfsrState[0] = state.lfsrState[0][i];
314                        singlePktState.lfsrState[1] = state.lfsrState[1][i];
315                        singlePktState.lfsrState[2] = state.lfsrState[2][i];
316                        singlePktState.lfsrState[3] = state.lfsrState[3][i];
317                        singlePktState.lfsrState[4] = state.lfsrState[4][i];
318                        singlePktState.lfsrState[5] = state.lfsrState[5][i];
319                        singlePktState.lfsrState[6] = state.lfsrState[6][i];
320                        singlePktState.lfsrState[7] = state.lfsrState[7][i];
321                        singlePktState.lfsrState[8] = state.lfsrState[8][i];
322                        singlePktState.lfsrState[9] = state.lfsrState[9][i];
323                        singlePktState.lfsrState[10] = state.lfsrState[10][i];
324                        singlePktState.lfsrState[11] = state.lfsrState[11][i];
325                        singlePktState.lfsrState[12] = state.lfsrState[12][i];
326                        singlePktState.lfsrState[13] = state.lfsrState[13][i];
327                        singlePktState.lfsrState[14] = state.lfsrState[14][i];
328                        singlePktState.lfsrState[15] = state.lfsrState[15][i];

File: lib/avx512/zuc_avx512_top.c

594 static inline
595 void _zuc_eia3_16_buffer_avx512(const void * const pKey[NUM_AVX512_BUFS],
596                                const void * const pIv[NUM_AVX512_BUFS],
597                                const void * const pBufferIn[NUM_AVX512_BUFS],
598                                const uint32_t lengthInBits[NUM_AVX512_BUFS],
599                                uint32_t *pMacI[NUM_AVX512_BUFS],
600                                const unsigned use_gfni)
601 {
602        unsigned int i = 0;

   1. var_decl: Declaring variable state without initializer.

603        DECLARE_ALIGNED(ZucState16_t state, 64);
604        DECLARE_ALIGNED(ZucState_t singlePktState, 64);
605        /* Calculate the minimum input packet size from all packets */
606        uint32_t commonBits = find_min_length32(lengthInBits);
607
608        DECLARE_ALIGNED(uint8_t keyStr[NUM_AVX512_BUFS][2*64], 64);
609        /* structure to store the 16 keys */
610        DECLARE_ALIGNED(ZucKey16_t keys, 64);
611        /* structure to store the 16 IV's */
612        DECLARE_ALIGNED(ZucIv16_t ivs, 64);
613        const uint8_t *pIn8[NUM_AVX512_BUFS] = {NULL};
614        uint32_t remainCommonBits = commonBits;
615        uint32_t numKeyStr = 0;
616        uint32_t T[NUM_AVX512_BUFS] = {0};
617        const uint32_t keyStreamLengthInBits = ZUC_KEYSTR_LEN * 8;
618        DECLARE_ALIGNED(uint32_t *pKeyStrArr0[NUM_AVX512_BUFS], 64) = {NULL};
619        DECLARE_ALIGNED(uint32_t *pKeyStrArr64[NUM_AVX512_BUFS], 64) = {NULL};
620        DECLARE_ALIGNED(uint16_t lens[NUM_AVX512_BUFS], 32);
621

   2. Condition i < 16, taking true branch.
   4. Condition i < 16, taking true branch.
   6. Condition i < 16, taking false branch.

622        for (i = 0; i < NUM_AVX512_BUFS; i++) {
623                pIn8[i] = (const uint8_t *) pBufferIn[i];
624                pKeyStrArr0[i] = (uint32_t *) &keyStr[i][0];
625                keys.pKeys[i] = pKey[i];
626                ivs.pIvs[i] = pIv[i];
627                lens[i] = (uint16_t) lengthInBits[i];

   3. Jumping back to the beginning of the loop.
   5. Jumping back to the beginning of the loop.

628        }
629
630        init_16(&keys, &ivs, &state, 0xFFFF, use_gfni);
631        /* Generate 64 bytes at a time */
632        keystr_64B_gen_16(&state, pKeyStrArr0, use_gfni);
633
634        /* Point at the next 64 bytes of the key */

   7. Condition i < 16, taking true branch.
   9. Condition i < 16, taking true branch.
   11. Condition i < 16, taking false branch.
635        for (i = 0; i < NUM_AVX512_BUFS; i++)
   8. Jumping back to the beginning of the loop.
   10. Jumping back to the beginning of the loop.

636                pKeyStrArr64[i] = (uint32_t *) &keyStr[i][64];
637
638        /* loop over the message bits */

   12. Condition remainCommonBits >= 512U /* keyStreamLengthInBits */, taking true branch.
   16. Condition remainCommonBits >= 512U /* keyStreamLengthInBits */, taking false branch.

639        while (remainCommonBits >= keyStreamLengthInBits) {
640                remainCommonBits -= keyStreamLengthInBits;
641                numKeyStr++;
642                /* Generate the next key stream 8 bytes or 64 bytes */

   13. Condition !remainCommonBits, taking true branch.

643                if (!remainCommonBits)
644                        keystr_8B_gen_16(&state, pKeyStrArr64, 0x0000FFFF,

   14. Falling through to end of if statement.

645                                     use_gfni);
646                else
647                        keystr_64B_gen_16(&state, pKeyStrArr64, use_gfni);
648                round64B_16(T, (const void * const *)pKeyStrArr0,
649                            (const void **)pIn8, lens, use_gfni);

   15. Jumping back to the beginning of the loop.

650        }
651
652        /* Process each packet separately for the remaining bits */

   17. Condition i < 16, taking true branch.

653        for (i = 0; i < NUM_AVX512_BUFS; i++) {
654                const uint32_t N = lengthInBits[i] + (2 * ZUC_WORD_BITS);
655                uint32_t L = ((N + 31) / ZUC_WORD_BITS) -
656                             numKeyStr*(keyStreamLengthInBits / 32);
657                uint32_t remainBits = lengthInBits[i] -
658                                      numKeyStr*keyStreamLengthInBits;
659                uint32_t *keyStr32 = (uint32_t *) keyStr[i];
660
661                /* If remaining bits are more than 56 bytes, we need to generate
662                 * at least 8B more of keystream, so we need to copy
663                 * the zuc state to single packet state first */

   18. Condition remainBits > 448U /* 14 * 32 */, taking true branch.

664                if (remainBits > (14*32)) {

   CID 106260 (#3 of 3): Uninitialized scalar variable (UNINIT) [select issue]

665                        singlePktState.lfsrState[0] = state.lfsrState[0][i];
666                        singlePktState.lfsrState[1] = state.lfsrState[1][i];
667                        singlePktState.lfsrState[2] = state.lfsrState[2][i];
668                        singlePktState.lfsrState[3] = state.lfsrState[3][i];
669                        singlePktState.lfsrState[4] = state.lfsrState[4][i];
670                        singlePktState.lfsrState[5] = state.lfsrState[5][i];
671                        singlePktState.lfsrState[6] = state.lfsrState[6][i];
672                        singlePktState.lfsrState[7] = state.lfsrState[7][i];
673                        singlePktState.lfsrState[8] = state.lfsrState[8][i];
674                        singlePktState.lfsrState[9] = state.lfsrState[9][i];
675                        singlePktState.lfsrState[10] = state.lfsrState[10][i];
676                        singlePktState.lfsrState[11] = state.lfsrState[11][i];
677                        singlePktState.lfsrState[12] = state.lfsrState[12][i];
678                        singlePktState.lfsrState[13] = state.lfsrState[13][i];
679                        singlePktState.lfsrState[14] = state.lfsrState[14][i];
680                        singlePktState.lfsrState[15] = state.lfsrState[15][i];
681

   CID 106260 (#2 of 3): Uninitialized scalar variable (UNINIT) [select issue]

682                        singlePktState.fR1 = state.fR1[i];

   CID 106260 (#1 of 3): Uninitialized scalar variable (UNINIT)19. uninit_use: Using uninitialized value state.fR2[i].

683                        singlePktState.fR2 = state.fR2[i];
684                }
tkanteck commented 3 years ago

Thanks @ColinIanKing. I'll have a look into it

tkanteck commented 3 years ago
ColinIanKing commented 3 years ago

Thanks for looking into these.