From e55cfcddecc6af2ec59c5c7f6174086ff56f6ed8 Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Fri, 27 Jul 2018 09:40:09 +0200 Subject: [PATCH] Add more Coverity fixes from upstream This commit updates the last PR patch (one of the commits was not accepted upstream) and adds new fixes that have been added in the meantime (including an alternative version of the patch that had been dropped). --- libkcapi-1.1.1-Coverity_PR_follow-up.patch | 272 +++++++++++++ ..._various_issues_reported_by_Coverity.patch | 368 +++++++++++++++--- libkcapi.spec | 9 +- 3 files changed, 602 insertions(+), 47 deletions(-) create mode 100644 libkcapi-1.1.1-Coverity_PR_follow-up.patch diff --git a/libkcapi-1.1.1-Coverity_PR_follow-up.patch b/libkcapi-1.1.1-Coverity_PR_follow-up.patch new file mode 100644 index 0000000..854481c --- /dev/null +++ b/libkcapi-1.1.1-Coverity_PR_follow-up.patch @@ -0,0 +1,272 @@ +From f24f3435be39cab2aa54a49d31968a023ab6d1d5 Mon Sep 17 00:00:00 2001 +From: Ondrej Mosnacek +Date: Thu, 26 Jul 2018 14:09:27 +0200 +Subject: [PATCH 1/3] kcapi-kdf: Clear the whole out buffer on error + +The KDF functions were decrementing the output length variable in the +loop, but on error they would clear the output buffer based on this +decremented value. This patch backs up the original length and uses it +when clearing the output buffer. + +The kcapi_pbkdf() function also used an incremented output buffer +pointer. This one is now also backed-up and the original value is used +when clearing the output. + +Signed-off-by: Stephan Mueller +--- + lib/kcapi-kdf.c | 16 +++++++++++----- + 1 file changed, 11 insertions(+), 5 deletions(-) + +diff --git a/lib/kcapi-kdf.c b/lib/kcapi-kdf.c +index 78a7e0d..6eccbe1 100644 +--- a/lib/kcapi-kdf.c ++++ b/lib/kcapi-kdf.c +@@ -99,6 +99,7 @@ int32_t kcapi_kdf_dpi(struct kcapi_handle *handle, + uint32_t h = kcapi_md_digestsize(handle); + int32_t err = 0; + uint8_t *dst_orig = dst; ++ uint32_t dlen_orig = dlen; + uint8_t Ai[h]; + uint32_t i = 1; + +@@ -161,7 +162,7 @@ int32_t kcapi_kdf_dpi(struct kcapi_handle *handle, + return 0; + + err: +- kcapi_memset_secure(dst_orig, 0, dlen); ++ kcapi_memset_secure(dst_orig, 0, dlen_orig); + kcapi_memset_secure(Ai, 0, h); + return err; + } +@@ -174,6 +175,7 @@ int32_t kcapi_kdf_fb(struct kcapi_handle *handle, + uint32_t h = kcapi_md_digestsize(handle); + int32_t err = 0; + uint8_t *dst_orig = dst; ++ uint32_t dlen_orig = dlen; + const uint8_t *label; + uint32_t labellen = 0; + uint32_t i = 1; +@@ -238,7 +240,7 @@ int32_t kcapi_kdf_fb(struct kcapi_handle *handle, + return 0; + + err: +- kcapi_memset_secure(dst_orig, 0, dlen); ++ kcapi_memset_secure(dst_orig, 0, dlen_orig); + return err; + } + +@@ -250,6 +252,7 @@ int32_t kcapi_kdf_ctr(struct kcapi_handle *handle, + uint32_t h = kcapi_md_digestsize(handle); + int32_t err = 0; + uint8_t *dst_orig = dst; ++ uint32_t dlen_orig = dlen; + uint32_t i = 1; + + if (dlen > INT_MAX) +@@ -295,7 +298,7 @@ int32_t kcapi_kdf_ctr(struct kcapi_handle *handle, + return 0; + + err: +- kcapi_memset_secure(dst_orig, 0, dlen); ++ kcapi_memset_secure(dst_orig, 0, dlen_orig); + return err; + } + +@@ -316,6 +319,7 @@ int32_t kcapi_hkdf(const char *hashname, + uint8_t *prev = NULL; + int32_t err = 0; + uint8_t *dst_orig = dst; ++ uint32_t dlen_orig = dlen; + uint8_t ctr = 0x01; + struct kcapi_handle *handle = NULL; + +@@ -415,7 +419,7 @@ int32_t kcapi_hkdf(const char *hashname, + goto out; + + err: +- kcapi_memset_secure(dst_orig, 0, dlen); ++ kcapi_memset_secure(dst_orig, 0, dlen_orig); + out: + kcapi_memset_secure(prk_tmp, 0, h); + kcapi_md_destroy(handle); +@@ -552,6 +556,8 @@ int32_t kcapi_pbkdf(const char *hashname, + uint8_t *key, uint32_t keylen) + { + struct kcapi_handle *handle; ++ uint8_t *key_orig = key; ++ uint32_t keylen_orig = keylen; + uint32_t h, i = 1; + #define MAX_DIGESTSIZE 64 + uint8_t u[MAX_DIGESTSIZE] __attribute__ ((aligned (sizeof(uint64_t)))); +@@ -633,7 +639,7 @@ int32_t kcapi_pbkdf(const char *hashname, + err: + kcapi_memset_secure(u, 0, h); + if (err) +- kcapi_memset_secure(key, 0, keylen); ++ kcapi_memset_secure(key_orig, 0, keylen_orig); + kcapi_md_destroy(handle); + + return err; + +From eacb82b193a94d46d2ea70c621176d79a5486008 Mon Sep 17 00:00:00 2001 +From: Ondrej Mosnacek +Date: Thu, 26 Jul 2018 14:12:51 +0200 +Subject: [PATCH 2/3] kcapi-kdf: Simplify handling of final blocks + +This patch avoids the use of temporary buffers when handling the last +block in the KDF functions, taking advantage of the fact that +kcapi_md_final() can be used to retrieve also a truncated hash directly. + +The new code no longer produces a false-positive warning with CLang +static analysis, so the workaround (which Coverity identifies as +unreachable code) can be removed. + +Signed-off-by: Stephan Mueller +--- + lib/kcapi-kdf.c | 43 +++++++++---------------------------------- + 1 file changed, 9 insertions(+), 34 deletions(-) + +diff --git a/lib/kcapi-kdf.c b/lib/kcapi-kdf.c +index 6eccbe1..afa6eb3 100644 +--- a/lib/kcapi-kdf.c ++++ b/lib/kcapi-kdf.c +@@ -140,13 +140,9 @@ int32_t kcapi_kdf_dpi(struct kcapi_handle *handle, + } + + if (dlen < h) { +- uint8_t tmpbuffer[h]; +- +- err = kcapi_md_final(handle, tmpbuffer, h); ++ err = kcapi_md_final(handle, dst, dlen); + if (err < 0) + goto err; +- memcpy(dst, tmpbuffer, dlen); +- kcapi_memset_secure(tmpbuffer, 0, h); + dlen = 0; + } else { + err = kcapi_md_final(handle, dst, h); +@@ -219,14 +215,10 @@ int32_t kcapi_kdf_fb(struct kcapi_handle *handle, + } + + if (dlen < h) { +- uint8_t tmpbuffer[h]; +- +- err = kcapi_md_final(handle, tmpbuffer, h); ++ err = kcapi_md_final(handle, dst, dlen); + if (err < 0) + goto err; +- memcpy(dst, tmpbuffer, dlen); +- kcapi_memset_secure(tmpbuffer, 0, h); +- return 0; ++ dlen = 0; + } else { + err = kcapi_md_final(handle, dst, h); + if (err < 0) +@@ -276,14 +268,10 @@ int32_t kcapi_kdf_ctr(struct kcapi_handle *handle, + } + + if (dlen < h) { +- uint8_t tmpbuffer[h]; +- +- err = kcapi_md_final(handle, tmpbuffer, h); ++ err = kcapi_md_final(handle, dst, dlen); + if (err < 0) + goto err; +- memcpy(dst, tmpbuffer, dlen); +- kcapi_memset_secure(tmpbuffer, 0, h); +- return 0; ++ dlen = 0; + } else { + err = kcapi_md_final(handle, dst, h); + if (err < 0) +@@ -392,16 +380,10 @@ int32_t kcapi_hkdf(const char *hashname, + goto err; + + if (dlen < h) { +- err = kcapi_md_final(handle, prk_tmp, h); ++ err = kcapi_md_final(handle, dst, dlen); + if (err < 0) + goto err; + +- /* Shut up Clang */ +- if (!dst) { +- err = -EFAULT; +- goto err; +- } +- memcpy(dst, prk_tmp, dlen); + dlen = 0; + } else { + err = kcapi_md_final(handle, dst, h); +@@ -561,8 +543,6 @@ int32_t kcapi_pbkdf(const char *hashname, + uint32_t h, i = 1; + #define MAX_DIGESTSIZE 64 + uint8_t u[MAX_DIGESTSIZE] __attribute__ ((aligned (sizeof(uint64_t)))); +- uint8_t T[MAX_DIGESTSIZE] __attribute__ ((aligned (sizeof(uint64_t)))) = +- { 0 }; + int32_t err = 0; + + if (keylen > INT_MAX) +@@ -617,17 +597,12 @@ int32_t kcapi_pbkdf(const char *hashname, + if (err < 0) + goto err; + +- if (keylen < h) +- kcapi_xor_64_aligned(T, u, h); +- else +- kcapi_xor_64(key, u, h); ++ kcapi_xor_64(key, u, keylen < h ? keylen : h); + } + +- if (keylen < h) { +- memcpy(key, T, keylen); +- kcapi_memset_secure(T, 0, keylen); ++ if (keylen < h) + keylen = 0; +- } else { ++ else { + keylen -= h; + key += h; + i++; + +From c9ed6b2c07026e9bafd99e6c288cfbd175fd237f Mon Sep 17 00:00:00 2001 +From: Ondrej Mosnacek +Date: Thu, 26 Jul 2018 14:28:53 +0200 +Subject: [PATCH 3/3] kcapi-kdf: Fix unused function warning on 32-bit + +The kcapi_xor_64_aligned() is now unused when compiling in 32-bit mode, +so we need to define it only in the 64-bit case, otherwise the build +fails under CLang due to an usnused function warning. + +Signed-off-by: Stephan Mueller +--- + lib/kcapi-kdf.c | 6 ++---- + 1 file changed, 2 insertions(+), 4 deletions(-) + +diff --git a/lib/kcapi-kdf.c b/lib/kcapi-kdf.c +index afa6eb3..a219d63 100644 +--- a/lib/kcapi-kdf.c ++++ b/lib/kcapi-kdf.c +@@ -503,10 +503,10 @@ static inline void kcapi_xor_32(uint8_t *dst, const uint8_t *src, uint32_t size) + kcapi_xor_8(dst, src, size); + } + ++#ifdef __LP64__ + static inline void kcapi_xor_64_aligned(uint8_t *dst, const uint8_t *src, + uint32_t size) + { +-#ifdef __LP64__ + uint64_t *dst_dword = (uint64_t *)dst; + uint64_t *src_dword = (uint64_t *)src; + +@@ -514,10 +514,8 @@ static inline void kcapi_xor_64_aligned(uint8_t *dst, const uint8_t *src, + *dst_dword++ ^= *src_dword++; + + kcapi_xor_32_aligned((uint8_t *)dst_dword, (uint8_t *)src_dword, size); +-#else +- kcapi_xor_32_aligned(dst, src, size); +-#endif + } ++#endif + + static inline void kcapi_xor_64(uint8_t *dst, const uint8_t *src, uint32_t size) + { diff --git a/libkcapi-1.1.1-Fix_various_issues_reported_by_Coverity.patch b/libkcapi-1.1.1-Fix_various_issues_reported_by_Coverity.patch index c5dc576..80f51cf 100644 --- a/libkcapi-1.1.1-Fix_various_issues_reported_by_Coverity.patch +++ b/libkcapi-1.1.1-Fix_various_issues_reported_by_Coverity.patch @@ -1,9 +1,11 @@ -From 4b4e7525123e236befec3168f3cecaa59f571621 Mon Sep 17 00:00:00 2001 +From 633569b273d63244fccf1a1e65acc8c8252c2f48 Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Mon, 23 Jul 2018 08:39:32 +0200 -Subject: [PATCH 01/10] apps: Check return code of fstat() +Subject: [PATCH 01/16] apps: Check return code of fstat() Found by Coverity. + +Signed-off-by: Stephan Mueller --- apps/app-internal.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) @@ -27,15 +29,17 @@ index 25cef80..e80c304 100644 /* Do not return an error in case we cannot validate the data. */ if ((sb->st_mode & S_IFMT) != S_IFREG && -From 2ffc5a5edebee6ba4984e4ef3ffe84c9116d328a Mon Sep 17 00:00:00 2001 +From bb1685801cf3f2c94c4591808a1a8499147b0249 Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Mon, 23 Jul 2018 08:45:48 +0200 -Subject: [PATCH 02/10] kcapi-hasher: Fix strerror() call +Subject: [PATCH 02/16] kcapi-hasher: Fix strerror() call strerror() expects a nonnegative error number. Here we can just pass errno instead of decoding the error from the return value of read(). Found by Coverity. + +Signed-off-by: Stephan Mueller --- apps/kcapi-hasher.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) @@ -54,12 +58,14 @@ index 2fc3ddc..5769502 100644 goto out; } -From 1e0ef69512b1f1e7de99f812356749f5d7a09d90 Mon Sep 17 00:00:00 2001 +From fadc3f42bbd44bd78f78f58c935ae7126b6eb2ce Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Mon, 23 Jul 2018 08:50:36 +0200 -Subject: [PATCH 03/10] kcapi-hasher: Fix fd leak in load_file() +Subject: [PATCH 03/16] kcapi-hasher: Fix fd leak in load_file() Found by Coverity. + +Signed-off-by: Stephan Mueller --- apps/kcapi-hasher.c | 2 ++ 1 file changed, 2 insertions(+) @@ -78,10 +84,10 @@ index 5769502..52fca78 100644 out: -From f2eec27169c89bf0e8fb9338ed5390034c76bff4 Mon Sep 17 00:00:00 2001 +From 5ee2bc94de5e70703ed6ad288b3c664a1cff4fcf Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Mon, 23 Jul 2018 08:53:13 +0200 -Subject: [PATCH 04/10] kcapi-hasher: Fix buffer overrun in process_checkfile() +Subject: [PATCH 04/16] kcapi-hasher: Fix buffer overrun in process_checkfile() The 'buf[(bsd_style - 4)]' access on line 593 can overrun the buffer if bsd_style is exactly 3, which can theoretically happen if the BSD-style @@ -90,6 +96,8 @@ starting to search for the separator at index 1 (it can't really be at index 0 anyway). Found by Coverity. + +Signed-off-by: Stephan Mueller --- apps/kcapi-hasher.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) @@ -108,39 +116,14 @@ index 52fca78..daab735 100644 * Check for BSD-style separator between file name and * hash value. -From 4ec718f46d4199510d57043a5a483cf680ec69a3 Mon Sep 17 00:00:00 2001 -From: Ondrej Mosnacek -Date: Mon, 23 Jul 2018 09:00:16 +0200 -Subject: [PATCH 05/10] kcapi-hasher: Ensure selfname is null-terminated - -Since readlink() does not null-terminate the returned string, we need to -pass BUFSIZE - 1 as the buffer size. - -Found by Coverity. ---- - apps/kcapi-hasher.c | 2 +- - 1 file changed, 1 insertion(+), 1 deletion(-) - -diff --git a/apps/kcapi-hasher.c b/apps/kcapi-hasher.c -index daab735..66bb794 100644 ---- a/apps/kcapi-hasher.c -+++ b/apps/kcapi-hasher.c -@@ -706,7 +706,7 @@ static int fipscheck_self(const struct hash_params *params_bin, - /* Integrity check of our application. */ - if (mode == SELFCHECK_CHECK || mode == SELFCHECK_PRINT_SELF) { - memset(selfname, 0, sizeof(selfname)); -- selfnamesize = readlink("/proc/self/exe", selfname, BUFSIZE); -+ selfnamesize = readlink("/proc/self/exe", selfname, BUFSIZE - 1); - if (selfnamesize >= BUFSIZE || selfnamesize < 0) { - fprintf(stderr, "Cannot obtain my filename\n"); - ret = -EFAULT; - -From d123a3a8f3e4468ed5fd74882cc841a058fe4aff Mon Sep 17 00:00:00 2001 +From 1520fca1f9b2231bcb5101eab32e8e859b33a66c Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Mon, 23 Jul 2018 09:05:45 +0200 -Subject: [PATCH 06/10] docproc: Use correct sizeof() argument for clarity +Subject: [PATCH 05/16] docproc: Use correct sizeof() argument for clarity Found by Coverity. + +Signed-off-by: Stephan Mueller --- lib/doc/bin/docproc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) @@ -160,12 +143,14 @@ index 4e52c1b..2313592 100644 } -From 33380e413e031df50ecbd31e5280aaef76eb52a4 Mon Sep 17 00:00:00 2001 +From ed6c64434d42ba43efd839d4b0c693623442968f Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Mon, 23 Jul 2018 09:09:44 +0200 -Subject: [PATCH 07/10] docproc: Fail early on malloc/realloc failures +Subject: [PATCH 06/16] docproc: Fail early on malloc/realloc failures Found by Coverity. + +Signed-off-by: Stephan Mueller --- lib/doc/bin/docproc.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) @@ -218,12 +203,14 @@ index 2313592..9a0a931 100644 for (i = 0; i < (int)data_len && start != all_list_len; i++) { if (data[i] == '\0') { -From be7c5d6d2f8c67e15aa77b24925a41ae280e1554 Mon Sep 17 00:00:00 2001 +From 1beccc4fa0af3ce57e0ff21d42907e774c4eb8fe Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Mon, 23 Jul 2018 09:15:36 +0200 -Subject: [PATCH 08/10] cryptoperf: Fix check of return value of open() +Subject: [PATCH 07/16] cryptoperf: Fix check of return value of open() Found by Coverity. + +Signed-off-by: Stephan Mueller --- speed-test/cryptoperf-base.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) @@ -242,12 +229,14 @@ index 55cd7ea..b564e19 100644 do { ret = read(fd, (buf + len), (buflen - len)); -From 4a378fc0abba6c4e9ed648abfc2c661291d60ab6 Mon Sep 17 00:00:00 2001 +From d41a21125e72e9ad611451bb9753489a1f96af5e Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Mon, 23 Jul 2018 09:30:01 +0200 -Subject: [PATCH 09/10] cryptoperf: Fix buffer overrun in cp_print_status() +Subject: [PATCH 08/16] cryptoperf: Fix buffer overrun in cp_print_status() Found by Coverity. + +Signed-off-by: Stephan Mueller --- speed-test/cryptoperf-base.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) @@ -266,12 +255,14 @@ index b564e19..c56c2ce 100644 test->testname, test->enc ? "e" : "d", -From 880b874a7304d54923471a3a5c4e8da08914a94c Mon Sep 17 00:00:00 2001 +From 5d17c564f7edae17b355f8cec7fa4c9685b10422 Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Mon, 23 Jul 2018 10:05:50 +0200 -Subject: [PATCH 10/10] test/cryptoperf: Check the return value of sysconf() +Subject: [PATCH 09/16] test/cryptoperf: Check the return value of sysconf() Found by Coverity. + +Signed-off-by: Stephan Mueller --- speed-test/cryptoperf-aead.c | 10 ++++++-- speed-test/cryptoperf-skcipher.c | 8 +++++- @@ -521,3 +512,290 @@ index c167b7f..b0ec2ca 100644 memset(&cavs_test, 0, sizeof(struct kcapi_cavs)); kcapi_set_verbosity(KCAPI_LOG_WARN); + +From 4c904fbf621b0fb01d79c1b01d28c296f36e6d8a Mon Sep 17 00:00:00 2001 +From: Ondrej Mosnacek +Date: Wed, 25 Jul 2018 11:10:01 +0200 +Subject: [PATCH 10/16] docproc: Fix memory leak + +Found by Coverity. + +Signed-off-by: Stephan Mueller +--- + lib/doc/bin/docproc.c | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/lib/doc/bin/docproc.c b/lib/doc/bin/docproc.c +index 9a0a931..ad8d3a0 100644 +--- a/lib/doc/bin/docproc.c ++++ b/lib/doc/bin/docproc.c +@@ -445,6 +445,7 @@ static void find_all_symbols(char *filename) + start++; + } + } ++ free(data); + } + + /* + +From 6092ff27886b7d40ea056f6c02a9c3fd5803df0d Mon Sep 17 00:00:00 2001 +From: Ondrej Mosnacek +Date: Wed, 25 Jul 2018 11:10:35 +0200 +Subject: [PATCH 11/16] kcapi-aead: Remove an unreachable statement + +Found by Coverity. + +Signed-off-by: Stephan Mueller +--- + lib/kcapi-aead.c | 2 -- + 1 file changed, 2 deletions(-) + +diff --git a/lib/kcapi-aead.c b/lib/kcapi-aead.c +index 7f8348f..d32c1e4 100644 +--- a/lib/kcapi-aead.c ++++ b/lib/kcapi-aead.c +@@ -249,8 +249,6 @@ int32_t kcapi_aead_encrypt_aio(struct kcapi_handle *handle, struct iovec *iniov, + + return _kcapi_aead_encrypt_aio_fallback(handle, iniov, outiov, iovlen, + iv); +- +- return ret; + } + + DSO_PUBLIC + +From 41a64a4363da4cce0f8de654f7dceef5c3fd6285 Mon Sep 17 00:00:00 2001 +From: Ondrej Mosnacek +Date: Wed, 25 Jul 2018 12:23:18 +0200 +Subject: [PATCH 12/16] kcapi-kdf: Fix buffer overruns in error paths + +Found by Coverity. + +Signed-off-by: Stephan Mueller +--- + lib/kcapi-kdf.c | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/lib/kcapi-kdf.c b/lib/kcapi-kdf.c +index bf150c1..78a7e0d 100644 +--- a/lib/kcapi-kdf.c ++++ b/lib/kcapi-kdf.c +@@ -336,6 +336,7 @@ int32_t kcapi_hkdf(const char *hashname, + if (h > HKDF_MAXHASH) { + kcapi_dolog(KCAPI_LOG_ERR, + "Null salt size too small for hash\n"); ++ h = HKDF_MAXHASH; + err = -EFAULT; + goto err; + } +@@ -570,6 +571,7 @@ int32_t kcapi_pbkdf(const char *hashname, + kcapi_dolog(KCAPI_LOG_ERR, + "Programming error in file %s at line %u\n", + __FILE__, __LINE__); ++ h = MAX_DIGESTSIZE; + err = -EFAULT; + goto err; + } + +From 33c3b71ba5577c0b2bcdf8eb880642e0ab461079 Mon Sep 17 00:00:00 2001 +From: Ondrej Mosnacek +Date: Wed, 25 Jul 2018 12:26:55 +0200 +Subject: [PATCH 13/16] kcapi-kernel-if: Simplify iovec validity check + +Current check is awkward, just checking iov for NULL seems to make CLang +happy. + +Found by Coverity. + +Signed-off-by: Stephan Mueller +--- + lib/kcapi-kernel-if.c | 8 ++++---- + 1 file changed, 4 insertions(+), 4 deletions(-) + +diff --git a/lib/kcapi-kernel-if.c b/lib/kcapi-kernel-if.c +index 807cbfe..595ce68 100644 +--- a/lib/kcapi-kernel-if.c ++++ b/lib/kcapi-kernel-if.c +@@ -257,11 +257,11 @@ int32_t _kcapi_common_vmsplice_iov(struct kcapi_handle *handle, + uint32_t inlen = 0; + unsigned long i; + +- for (i = 0; i < iovlen; i++) { +- if (!(iov + i)) +- return -EINVAL; ++ if (iovlen && !iov) ++ return -EINVAL; ++ ++ for (i = 0; i < iovlen; i++) + inlen += iov[i].iov_len; +- } + + /* kernel processes input data with max size of one page */ + handle->processed_sg += ((inlen + sysconf(_SC_PAGESIZE) - 1) / + +From c1f82d3b78031037f7098bd26b5da00eceecc00a Mon Sep 17 00:00:00 2001 +From: Ondrej Mosnacek +Date: Wed, 25 Jul 2018 12:37:15 +0200 +Subject: [PATCH 14/16] test: Allocate name even if size is zero + +We still need one byte for the terminating null character. + +Found by Coverity. + +Signed-off-by: Stephan Mueller +--- + test/kcapi-main.c | 10 ++++------ + 1 file changed, 4 insertions(+), 6 deletions(-) + +diff --git a/test/kcapi-main.c b/test/kcapi-main.c +index b0ec2ca..d20e74c 100644 +--- a/test/kcapi-main.c ++++ b/test/kcapi-main.c +@@ -275,13 +275,11 @@ static int fuzz_init_test(unsigned int size) + + kcapi_set_verbosity(KCAPI_LOG_NONE); + +- if (size) { +- name = calloc(1, size + 1); ++ name = calloc(1, size + 1); + +- if (!name) { +- printf("Allocation of %u bytes failed", size); +- return 1; +- } ++ if (!name) { ++ printf("Allocation of %u bytes failed", size); ++ return 1; + } + + if (get_random(name, size, 0)) { + +From 698fcb68572b5d315b27294bd3e9ee2c058920f6 Mon Sep 17 00:00:00 2001 +From: Ondrej Mosnacek +Date: Wed, 25 Jul 2018 12:41:37 +0200 +Subject: [PATCH 15/16] test: Fix resource leak and error handling + +The fuzz_cipher() and fuzz_aead() functions did not always return error +when it should and it did not always release the cipher handle on +return. This patch fixes both issues. + +Found by Coverity. + +Signed-off-by: Stephan Mueller +--- + test/kcapi-main.c | 16 ++++++++-------- + 1 file changed, 8 insertions(+), 8 deletions(-) + +diff --git a/test/kcapi-main.c b/test/kcapi-main.c +index d20e74c..b3f6ae9 100644 +--- a/test/kcapi-main.c ++++ b/test/kcapi-main.c +@@ -352,11 +352,11 @@ static int fuzz_cipher(struct kcapi_cavs *cavs_test, unsigned long flags, + uint8_t indata[4096]; + uint8_t outdata[4096]; + unsigned int i; +- int ret = 0; ++ int ret = 1; + + if (kcapi_cipher_init(&handle, cavs_test->cipher, 0)) { + printf("Allocation of %s cipher failed\n", cavs_test->cipher); +- return -EFAULT; ++ return 1; + } + + /* Set key */ +@@ -366,7 +366,7 @@ static int fuzz_cipher(struct kcapi_cavs *cavs_test, unsigned long flags, + for (i = 0; i < sizeof(key); i++) { + if (get_random(key, i, 0)) { + printf("get_random call failed\n"); +- return 1; ++ goto out; + } + kcapi_cipher_setkey(handle, key, i); + } +@@ -388,7 +388,7 @@ static int fuzz_cipher(struct kcapi_cavs *cavs_test, unsigned long flags, + + if (get_random(indata, i, 0)) { + printf("get_random call failed\n"); +- return 1; ++ goto out; + } + + if (flags & FUZZ_LESSOUT) +@@ -429,11 +429,11 @@ static int fuzz_aead(struct kcapi_cavs *cavs_test, unsigned long flags, + uint8_t indata[4096]; + uint8_t outdata[4096]; + unsigned int i; +- int ret = 0; ++ int ret = 1; + + if (kcapi_aead_init(&handle, cavs_test->cipher, 0)) { + printf("Allocation of %s cipher failed\n", cavs_test->cipher); +- return -EFAULT; ++ return 1; + } + + /* Set key */ +@@ -443,7 +443,7 @@ static int fuzz_aead(struct kcapi_cavs *cavs_test, unsigned long flags, + for (i = 0; i < sizeof(key); i++) { + if (get_random(key, i, 0)) { + printf("get_random call failed\n"); +- return 1; ++ goto out; + } + kcapi_aead_setkey(handle, key, i); + } +@@ -479,7 +479,7 @@ static int fuzz_aead(struct kcapi_cavs *cavs_test, unsigned long flags, + + if (get_random(indata, i, 0)) { + printf("get_random call failed\n"); +- return 1; ++ goto out; + } + + if (flags & FUZZ_LESSOUT) + +From ec9c36216623b94684c9e5ca8be26455b490bdef Mon Sep 17 00:00:00 2001 +From: Ondrej Mosnacek +Date: Wed, 25 Jul 2018 16:52:13 +0200 +Subject: [PATCH 16/16] test: Clean up after NULL string fix + +Signed-off-by: Stephan Mueller +--- + test/kcapi-main.c | 10 ++++------ + 1 file changed, 4 insertions(+), 6 deletions(-) + +diff --git a/test/kcapi-main.c b/test/kcapi-main.c +index b3f6ae9..3cba467 100644 +--- a/test/kcapi-main.c ++++ b/test/kcapi-main.c +@@ -271,14 +271,12 @@ static int fuzz_init_test(unsigned int size) + { + struct kcapi_handle *handle; + int ret = 0; +- uint8_t *name = NULL; ++ uint8_t *name = calloc(1, size + 1); + + kcapi_set_verbosity(KCAPI_LOG_NONE); + +- name = calloc(1, size + 1); +- + if (!name) { +- printf("Allocation of %u bytes failed", size); ++ printf("Allocation of %u bytes failed", size + 1); + return 1; + } + +@@ -317,10 +315,10 @@ static int fuzz_init_test(unsigned int size) + + fail: + fprintf(stdout, "allocation success of nonsense string "); +- if (name) ++ if (size) + bin2print(name, size); + else +- fprintf(stdout, "NULL\n"); ++ fprintf(stdout, "EMPTY\n"); + free(name); + return 1; + } diff --git a/libkcapi.spec b/libkcapi.spec index 3ef6d75..565c1c2 100644 --- a/libkcapi.spec +++ b/libkcapi.spec @@ -92,7 +92,7 @@ bin/kcapi-hasher -n fipshmac -d "$lib_path"/fipscheck \\\ Name: libkcapi Version: %{vmajor}.%{vminor}.%{vpatch} -Release: 7%{?dist} +Release: 8%{?dist} Summary: User space interface to the Linux Kernel Crypto API License: BSD or GPLv2 @@ -105,7 +105,8 @@ Patch1: %{giturl}/pull/61.patch#/%{name}-1.1.1-kcapi-hasher_Fix_off-by-o Patch2: %{giturl}/pull/64.patch#/%{name}-1.1.1-kcapi-hasher_Add_missing_-d_option_to_fipshmac.patch Patch3: %{giturl}/commit/3e388ac4eba63b466bf6b14b2088ea44c8a2bfe4.patch#/%{name}-1.1.1-Fix_possible_buffer_overflow_with_strncpy.patch Patch4: %{giturl}/commit/a10e5ff7f8f69e1ed5cd4151f3e71f4783c40c68.patch#/%{name}-1.1.1-test_Be_sure_to_terminate_strncpy_copied_string.patch -Patch5: %{giturl}/pull/65.patch#/%{name}-1.1.1-Fix_various_issues_reported_by_Coverity.patch +Patch5: %{giturl}/compare/decf850ab9bb...ec9c36216623.patch#/%{name}-1.1.1-Fix_various_issues_reported_by_Coverity.patch +Patch6: %{giturl}/compare/4a1a30f75e70...c9ed6b2c0702.patch#/%{name}-1.1.1-Coverity_PR_follow-up.patch # Workaround for failing builds on rawhide (F29). # To be removed when this issue is patched in the kernel: @@ -422,7 +423,11 @@ popd %{_bindir}/kcapi* %{_mandir}/man1/kcapi*.1.* + %changelog +* Fri Jul 27 2018 Ondrej Mosnáček - 1.1.1-8 +- Add more Coverity fixes from upstream + * Mon Jul 23 2018 Ondrej Mosnáček - 1.1.1-7 - Add various fixes from upstream - Drop the Requires on kernel package