libkcapi/libkcapi-1.1.1-Fix_various_issues_reported_by_Coverity.patch

524 lines
18 KiB
Diff
Raw Permalink Normal View History

2018-07-23 11:27:48 +00:00
From 4b4e7525123e236befec3168f3cecaa59f571621 Mon Sep 17 00:00:00 2001
From: Ondrej Mosnacek <omosnace@redhat.com>
Date: Mon, 23 Jul 2018 08:39:32 +0200
Subject: [PATCH 01/10] apps: Check return code of fstat()
Found by Coverity.
---
apps/app-internal.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/apps/app-internal.c b/apps/app-internal.c
index 25cef80..e80c304 100644
--- a/apps/app-internal.c
+++ b/apps/app-internal.c
@@ -255,7 +255,12 @@ int read_complete(int fd, uint8_t *buf, uint32_t buflen)
int check_filetype(int fd, struct stat *sb, const char *filename)
{
- fstat(fd, sb);
+ int ret = fstat(fd, sb);
+ if (ret) {
+ dolog(KCAPI_LOG_ERR,
+ "fstat() failed: %s", strerror(errno));
+ return -errno;
+ }
/* 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: Ondrej Mosnacek <omosnace@redhat.com>
Date: Mon, 23 Jul 2018 08:45:48 +0200
Subject: [PATCH 02/10] 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.
---
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 2fc3ddc..5769502 100644
--- a/apps/kcapi-hasher.c
+++ b/apps/kcapi-hasher.c
@@ -227,7 +227,7 @@ static int load_file(const char *filename, uint8_t **memory, uint32_t *size)
while ((rdbytes = read(fd, buffer + offset, buffer_size - offset)) != 0) {
if (rdbytes < 0) {
fprintf(stderr, "Error reading file %s: %s\n", filename,
- strerror((int)rdbytes));
+ strerror(errno));
ret = -EIO;
goto out;
}
From 1e0ef69512b1f1e7de99f812356749f5d7a09d90 Mon Sep 17 00:00:00 2001
From: Ondrej Mosnacek <omosnace@redhat.com>
Date: Mon, 23 Jul 2018 08:50:36 +0200
Subject: [PATCH 03/10] kcapi-hasher: Fix fd leak in load_file()
Found by Coverity.
---
apps/kcapi-hasher.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/apps/kcapi-hasher.c b/apps/kcapi-hasher.c
index 5769502..52fca78 100644
--- a/apps/kcapi-hasher.c
+++ b/apps/kcapi-hasher.c
@@ -258,6 +258,8 @@ static int load_file(const char *filename, uint8_t **memory, uint32_t *size)
*memory = buffer;
*size = (uint32_t)offset;
+
+ close(fd);
return 0;
out:
From f2eec27169c89bf0e8fb9338ed5390034c76bff4 Mon Sep 17 00:00:00 2001
From: Ondrej Mosnacek <omosnace@redhat.com>
Date: Mon, 23 Jul 2018 08:53:13 +0200
Subject: [PATCH 04/10] 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
separator is found at the very beginning of the line. Fix this by
starting to search for the separator at index 1 (it can't really be at
index 0 anyway).
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 52fca78..daab735 100644
--- a/apps/kcapi-hasher.c
+++ b/apps/kcapi-hasher.c
@@ -544,7 +544,7 @@ static int process_checkfile(const struct hash_params *params,
break;
}
- for (i = 0; i < linelen; i++) {
+ for (i = 1; i < linelen; i++) {
/*
* Check for BSD-style separator between file name and
* hash value.
From 4ec718f46d4199510d57043a5a483cf680ec69a3 Mon Sep 17 00:00:00 2001
From: Ondrej Mosnacek <omosnace@redhat.com>
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: Ondrej Mosnacek <omosnace@redhat.com>
Date: Mon, 23 Jul 2018 09:05:45 +0200
Subject: [PATCH 06/10] docproc: Use correct sizeof() argument for clarity
Found by Coverity.
---
lib/doc/bin/docproc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/doc/bin/docproc.c b/lib/doc/bin/docproc.c
index 4e52c1b..2313592 100644
--- a/lib/doc/bin/docproc.c
+++ b/lib/doc/bin/docproc.c
@@ -154,7 +154,8 @@ int symfilecnt = 0;
static void add_new_symbol(struct symfile *sym, char * symname)
{
sym->symbollist =
- realloc(sym->symbollist, (sym->symbolcnt + 1) * sizeof(char *));
+ realloc(sym->symbollist,
+ (sym->symbolcnt + 1) * sizeof(struct symbols));
sym->symbollist[sym->symbolcnt++].name = strdup(symname);
}
From 33380e413e031df50ecbd31e5280aaef76eb52a4 Mon Sep 17 00:00:00 2001
From: Ondrej Mosnacek <omosnace@redhat.com>
Date: Mon, 23 Jul 2018 09:09:44 +0200
Subject: [PATCH 07/10] docproc: Fail early on malloc/realloc failures
Found by Coverity.
---
lib/doc/bin/docproc.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/lib/doc/bin/docproc.c b/lib/doc/bin/docproc.c
index 2313592..9a0a931 100644
--- a/lib/doc/bin/docproc.c
+++ b/lib/doc/bin/docproc.c
@@ -156,6 +156,10 @@ static void add_new_symbol(struct symfile *sym, char * symname)
sym->symbollist =
realloc(sym->symbollist,
(sym->symbolcnt + 1) * sizeof(struct symbols));
+ if (!sym->symbollist) {
+ perror("realloc");
+ exit(1);
+ }
sym->symbollist[sym->symbolcnt++].name = strdup(symname);
}
@@ -391,12 +395,20 @@ static void find_all_symbols(char *filename)
default:
close(pipefd[1]);
data = malloc(4096);
+ if (!data) {
+ perror("malloc");
+ exit(1);
+ }
do {
while ((ret = read(pipefd[0],
data + data_len,
4096)) > 0) {
data_len += ret;
data = realloc(data, data_len + 4096);
+ if (!data) {
+ perror("realloc");
+ exit(1);
+ }
}
} while (ret == -EAGAIN);
if (ret != 0) {
@@ -421,6 +433,10 @@ static void find_all_symbols(char *filename)
start = all_list_len;
all_list_len += count;
all_list = realloc(all_list, sizeof(char *) * all_list_len);
+ if (!all_list) {
+ perror("realloc");
+ exit(1);
+ }
str = data;
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: Ondrej Mosnacek <omosnace@redhat.com>
Date: Mon, 23 Jul 2018 09:15:36 +0200
Subject: [PATCH 08/10] cryptoperf: Fix check of return value of open()
Found by Coverity.
---
speed-test/cryptoperf-base.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/speed-test/cryptoperf-base.c b/speed-test/cryptoperf-base.c
index 55cd7ea..b564e19 100644
--- a/speed-test/cryptoperf-base.c
+++ b/speed-test/cryptoperf-base.c
@@ -179,7 +179,7 @@ int cp_read_random(unsigned char *buf, size_t buflen)
size_t len = 0;
fd = open("/dev/urandom", O_RDONLY|O_CLOEXEC);
- if(0 >= fd)
+ if(0 > fd)
return fd;
do {
ret = read(fd, (buf + len), (buflen - len));
From 4a378fc0abba6c4e9ed648abfc2c661291d60ab6 Mon Sep 17 00:00:00 2001
From: Ondrej Mosnacek <omosnace@redhat.com>
Date: Mon, 23 Jul 2018 09:30:01 +0200
Subject: [PATCH 09/10] cryptoperf: Fix buffer overrun in cp_print_status()
Found by Coverity.
---
speed-test/cryptoperf-base.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/speed-test/cryptoperf-base.c b/speed-test/cryptoperf-base.c
index b564e19..c56c2ce 100644
--- a/speed-test/cryptoperf-base.c
+++ b/speed-test/cryptoperf-base.c
@@ -159,7 +159,7 @@ char *cp_print_status(struct cp_test *test, int raw)
memset(byteseconds, 0, sizeof(byteseconds));
cp_bytes2string((processed_bytes / totaltime), byteseconds,
- (VALLEN + 1));
+ VALLEN);
snprintf(str, 120, "%-24s|%s|%8lu bytes|%*s/s|%lu ops/s",
test->testname,
test->enc ? "e" : "d",
From 880b874a7304d54923471a3a5c4e8da08914a94c Mon Sep 17 00:00:00 2001
From: Ondrej Mosnacek <omosnace@redhat.com>
Date: Mon, 23 Jul 2018 10:05:50 +0200
Subject: [PATCH 10/10] test/cryptoperf: Check the return value of sysconf()
Found by Coverity.
---
speed-test/cryptoperf-aead.c | 10 ++++++--
speed-test/cryptoperf-skcipher.c | 8 +++++-
test/kcapi-main.c | 53 +++++++++++++++++++---------------------
3 files changed, 40 insertions(+), 31 deletions(-)
diff --git a/speed-test/cryptoperf-aead.c b/speed-test/cryptoperf-aead.c
index b2c0010..5a0446a 100644
--- a/speed-test/cryptoperf-aead.c
+++ b/speed-test/cryptoperf-aead.c
@@ -36,6 +36,12 @@ static int cp_aead_init_test(struct cp_test *test, int enc, int ccm)
unsigned char ivrand[MAX_KEYLEN];
unsigned char *ivdata = NULL;
uint32_t ivlen = 0;
+ long pagesize = sysconf(_SC_PAGESIZE);
+
+ if (pagesize < 0) {
+ printf(DRIVER_NAME": unable to determine the page size\n");
+ return -errno;
+ }
dbg("Initializing AEAD test %s\n", test->testname);
if (!test->driver_name) {
@@ -97,14 +103,14 @@ static int cp_aead_init_test(struct cp_test *test, int enc, int ccm)
test->u.aead.assoclen, TAGLEN);
}
- if (posix_memalign((void *)&input, sysconf(_SC_PAGESIZE),
+ if (posix_memalign((void *)&input, pagesize,
test->u.aead.indatalen *
(params->aio ? params->aio : 1))) {
printf(DRIVER_NAME": could not allocate input buffer for "
"%s\n", test->driver_name);
goto out;
}
- if (posix_memalign((void *)&output, sysconf(_SC_PAGESIZE),
+ if (posix_memalign((void *)&output, pagesize,
test->u.aead.outdatalen *
(params->aio ? params->aio : 1))) {
printf(DRIVER_NAME": could not allocate output buffer for "
diff --git a/speed-test/cryptoperf-skcipher.c b/speed-test/cryptoperf-skcipher.c
index a2db369..fb7123b 100644
--- a/speed-test/cryptoperf-skcipher.c
+++ b/speed-test/cryptoperf-skcipher.c
@@ -34,6 +34,12 @@ static int cp_skcipher_init_test(struct cp_test *test)
unsigned char *ivdata = NULL;
unsigned int bs;
int err;
+ long pagesize = sysconf(_SC_PAGESIZE);
+
+ if (pagesize < 0) {
+ printf(DRIVER_NAME": unable to determine the page size\n");
+ return -errno;
+ }
dbg("Initializing symmetric test %s\n", test->testname);
if (!test->driver_name) {
@@ -75,7 +81,7 @@ static int cp_skcipher_init_test(struct cp_test *test)
cp_read_random(ivdata, kcapi_cipher_blocksize(test->u.skcipher.handle));
test->u.skcipher.iv = ivdata;
- err = posix_memalign((void *)&scratchpad, sysconf(_SC_PAGESIZE),
+ err = posix_memalign((void *)&scratchpad, pagesize,
kcapi_cipher_blocksize(test->u.skcipher.handle) * params->len *
(params->aio ? params->aio : 1));
if (err) {
diff --git a/test/kcapi-main.c b/test/kcapi-main.c
index c167b7f..b0ec2ca 100644
--- a/test/kcapi-main.c
+++ b/test/kcapi-main.c
@@ -86,6 +86,8 @@ struct kcapi_cavs {
uint32_t outlen;
};
+static long pagesize;
+
static char hex_char_map_l[] = { '0', '1', '2', '3', '4', '5', '6', '7',
'8', '9', 'a', 'b', 'c', 'd', 'e', 'f' };
static char hex_char_map_u[] = { '0', '1', '2', '3', '4', '5', '6', '7',
@@ -808,8 +810,7 @@ static int cavs_sym(struct kcapi_cavs *cavs_test, uint32_t loops,
outbuflen = cavs_test->ctlen;
}
if (cavs_test->aligned) {
- if (posix_memalign((void *)&outbuf, sysconf(_SC_PAGESIZE),
- outbuflen))
+ if (posix_memalign((void *)&outbuf, pagesize, outbuflen))
goto out;
memset(outbuf, 0, outbuflen);
} else {
@@ -918,12 +919,10 @@ static int cavs_sym_stream(struct kcapi_cavs *cavs_test, uint32_t loops,
outbuflen = cavs_test->ctlen;
}
if (cavs_test->aligned) {
- if (posix_memalign((void *)&outbuf, sysconf(_SC_PAGESIZE),
- outbuflen))
+ if (posix_memalign((void *)&outbuf, pagesize, outbuflen))
goto out;
memset(outbuf, 0, outbuflen);
- if (posix_memalign((void *)&outbuf2, sysconf(_SC_PAGESIZE),
- outbuflen))
+ if (posix_memalign((void *)&outbuf2, pagesize, outbuflen))
goto out;
memset(outbuf2, 0, outbuflen);
} else {
@@ -1072,7 +1071,7 @@ static int cavs_sym_aio(struct kcapi_cavs *cavs_test, uint32_t loops,
return -ENOMEM;
if (cavs_test->aligned) {
- if (posix_memalign((void *)&outbuf, sysconf(_SC_PAGESIZE), outbuflen))
+ if (posix_memalign((void *)&outbuf, pagesize, outbuflen))
goto out;
memset(outbuf, 0, outbuflen);
} else {
@@ -1241,7 +1240,7 @@ static int cavs_aead(struct kcapi_cavs *cavs_test, uint32_t loops,
fullbuflen = (inbuflen > outbuflen) ? inbuflen : outbuflen;
if (cavs_test->aligned) {
- if (posix_memalign((void *)&inbuf, sysconf(_SC_PAGESIZE), fullbuflen))
+ if (posix_memalign((void *)&inbuf, pagesize, fullbuflen))
goto out;
memset(inbuf, 0, fullbuflen);
} else {
@@ -1425,8 +1424,7 @@ static int cavs_aead_aio(struct kcapi_cavs *cavs_test, uint32_t loops,
maxbuflen = (inbuflen > outbuflen) ? inbuflen : outbuflen;
if (cavs_test->aligned) {
- if (posix_memalign((void *)&inbuf, sysconf(_SC_PAGESIZE),
- loops * maxbuflen))
+ if (posix_memalign((void *)&inbuf, pagesize, loops * maxbuflen))
goto out;
memset(inbuf, 0, loops * maxbuflen);
} else {
@@ -1596,7 +1594,7 @@ static int cavs_aead_stream(struct kcapi_cavs *cavs_test, uint32_t loops,
maxbuflen = (inbuflen > outbuflen) ? inbuflen : outbuflen;
if (cavs_test->aligned) {
- if (posix_memalign((void *)&outbuf, sysconf(_SC_PAGESIZE), maxbuflen))
+ if (posix_memalign((void *)&outbuf, pagesize, maxbuflen))
goto out;
memset(outbuf, 0, maxbuflen);
} else {
@@ -1830,9 +1828,9 @@ static int cavs_aead_large(int stream, uint32_t loops, int splice)
test.keylen = len / 2;
len = strlen(aad);
- if (posix_memalign((void *)&test.assoc, sysconf(_SC_PAGESIZE), (16 * sysconf(_SC_PAGESIZE))))
+ if (posix_memalign((void *)&test.assoc, pagesize, (16 * pagesize)))
goto out;
- hex2bin(aad, len, test.assoc, (sysconf(_SC_PAGESIZE) * 16));
+ hex2bin(aad, len, test.assoc, (pagesize * 16));
test.assoclen = len / 2;
test.taglen = 16;
@@ -2052,8 +2050,7 @@ static int cavs_asym(struct kcapi_cavs *cavs_test, uint32_t loops,
}
if (cavs_test->aligned) {
- if (posix_memalign((void *)&outbuf, sysconf(_SC_PAGESIZE),
- maxsize))
+ if (posix_memalign((void *)&outbuf, pagesize, maxsize))
goto out;
memset(outbuf, 0, maxsize);
} else {
@@ -2164,11 +2161,10 @@ static int cavs_asym_aio(struct kcapi_cavs *cavs_test, uint32_t loops,
}
if (cavs_test->aligned) {
- if (posix_memalign((void *)&outbuf, sysconf(_SC_PAGESIZE),
- maxsize * loops))
+ if (posix_memalign((void *)&outbuf, pagesize, maxsize * loops))
goto out;
memset(outbuf, 0, maxsize * loops);
- if (posix_memalign((void *)&inbuf, sysconf(_SC_PAGESIZE),
+ if (posix_memalign((void *)&inbuf, pagesize,
cavs_test->ptlen * loops))
goto out;
memset(outbuf, 0, cavs_test->ptlen * loops);
@@ -2294,10 +2290,10 @@ static int cavs_asym_stream(struct kcapi_cavs *cavs_test, uint32_t loops,
}
if (cavs_test->aligned) {
- if (posix_memalign((void *)&outbuf, sysconf(_SC_PAGESIZE), maxsize * NUMIOVECS))
+ if (posix_memalign((void *)&outbuf, pagesize, maxsize * NUMIOVECS))
goto out;
memset(outbuf, 0, maxsize);
- if (posix_memalign((void *)&inbuf, sysconf(_SC_PAGESIZE), inbuflen))
+ if (posix_memalign((void *)&inbuf, pagesize, inbuflen))
goto out;
memset(inbuf, 0, inbuflen);
} else {
@@ -2489,8 +2485,7 @@ static int cavs_kdf_common(struct kcapi_cavs *cavs_test, uint32_t loops)
uint32_t i = 0;
if (cavs_test->aligned) {
- if (posix_memalign((void *)&outbuf, sysconf(_SC_PAGESIZE),
- cavs_test->outlen))
+ if (posix_memalign((void *)&outbuf, pagesize, cavs_test->outlen))
return -ENOMEM;
memset(outbuf, 0, cavs_test->outlen);
} else {
@@ -2571,8 +2566,7 @@ static int cavs_hkdf(struct kcapi_cavs *cavs_test, uint32_t loops)
}
if (cavs_test->aligned) {
- if (posix_memalign((void *)&outbuf, sysconf(_SC_PAGESIZE),
- cavs_test->outlen))
+ if (posix_memalign((void *)&outbuf, pagesize, cavs_test->outlen))
return -ENOMEM;
memset(outbuf, 0, cavs_test->outlen);
} else {
@@ -2671,8 +2665,7 @@ static int cavs_pbkdf(struct kcapi_cavs *cavs_test, uint32_t loops)
}
if (cavs_test->aligned) {
- if (posix_memalign((void *)&outbuf, sysconf(_SC_PAGESIZE),
- cavs_test->outlen))
+ if (posix_memalign((void *)&outbuf, pagesize, cavs_test->outlen))
return -ENOMEM;
memset(outbuf, 0, cavs_test->outlen);
} else {
@@ -2928,7 +2921,7 @@ static int kpp(struct kcapi_cavs *cavs_test, uint32_t loops, int splice)
outbuflen = ret;
if (cavs_test->aligned) {
- if (posix_memalign((void *)&outbuf, sysconf(_SC_PAGESIZE), ret))
+ if (posix_memalign((void *)&outbuf, pagesize, ret))
return -ENOMEM;
memset(outbuf, 0, ret);
} else {
@@ -3001,7 +2994,7 @@ static int kpp_aio(struct kcapi_cavs *cavs_test, uint32_t loops, int splice)
outbuflen = ret;
if (cavs_test->aligned) {
- if (posix_memalign((void *)&outbuf, sysconf(_SC_PAGESIZE), ret))
+ if (posix_memalign((void *)&outbuf, pagesize, ret))
return -ENOMEM;
memset(outbuf, 0, ret);
} else {
@@ -3072,6 +3065,10 @@ int main(int argc, char *argv[])
int splice = KCAPI_ACCESS_SENDMSG;
struct kcapi_cavs cavs_test;
+ pagesize = sysconf(_SC_PAGESIZE);
+ if (pagesize < 0)
+ return 1;
+
memset(&cavs_test, 0, sizeof(struct kcapi_cavs));
kcapi_set_verbosity(KCAPI_LOG_WARN);