From 5ce8577bd77f97758a24cbfec8e90513a5abcb12 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Wed, 20 Sep 2023 14:35:55 +0200 Subject: [PATCH 1/4] lib: add dup() error check in logging code Mostly to make coverity happy, if dup() fails we're f*cked already. (Still useful to have a better error message...) Signed-off-by: David Lamparter --- lib/zlog_5424.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/zlog_5424.c b/lib/zlog_5424.c index c15bdece29..16f348369e 100644 --- a/lib/zlog_5424.c +++ b/lib/zlog_5424.c @@ -881,6 +881,11 @@ static int zlog_5424_open(struct zlog_cfg_5424 *zcf, int sock_type) case ZLOG_5424_DST_FD: fd = dup(zcf->fd); + if (fd < 0) { + flog_err_sys(EC_LIB_SYSTEM_CALL, + "failed to dup() log file descriptor: %m (FD limit too low?)"); + break; + } optlen = sizeof(sock_type); if (!getsockopt(fd, SOL_SOCKET, SO_TYPE, &sock_type, &optlen)) { From e408a915a3c6d808d6f522f9e124b7079b2a6e33 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Wed, 20 Sep 2023 14:46:10 +0200 Subject: [PATCH 2/4] lib: straight return on error on log open fail I think I originally had some other code at the tail end of that function, but that's not the case anymore, and dropping out of the function with a straight "return -1" is more useful than trucking on with an invalid fd. Signed-off-by: David Lamparter --- lib/zlog_5424.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/zlog_5424.c b/lib/zlog_5424.c index 16f348369e..9bc1c819a8 100644 --- a/lib/zlog_5424.c +++ b/lib/zlog_5424.c @@ -877,14 +877,14 @@ static int zlog_5424_open(struct zlog_cfg_5424 *zcf, int sock_type) switch (zcf->dst) { case ZLOG_5424_DST_NONE: - break; + return -1; case ZLOG_5424_DST_FD: fd = dup(zcf->fd); if (fd < 0) { flog_err_sys(EC_LIB_SYSTEM_CALL, "failed to dup() log file descriptor: %m (FD limit too low?)"); - break; + return -1; } optlen = sizeof(sock_type); @@ -896,7 +896,7 @@ static int zlog_5424_open(struct zlog_cfg_5424 *zcf, int sock_type) case ZLOG_5424_DST_FIFO: if (!zcf->filename) - break; + return -1; if (!zcf->file_nocreate) { frr_with_privs (lib_privs) { @@ -909,7 +909,7 @@ static int zlog_5424_open(struct zlog_cfg_5424 *zcf, int sock_type) if (err == 0) do_chown = true; else if (errno != EEXIST) - break; + return -1; } flags = O_NONBLOCK; @@ -917,7 +917,7 @@ static int zlog_5424_open(struct zlog_cfg_5424 *zcf, int sock_type) case ZLOG_5424_DST_FILE: if (!zcf->filename) - break; + return -1; frr_with_privs (lib_privs) { fd = open(zcf->filename, flags | O_WRONLY | O_APPEND | @@ -929,7 +929,7 @@ static int zlog_5424_open(struct zlog_cfg_5424 *zcf, int sock_type) flog_err_sys(EC_LIB_SYSTEM_CALL, "could not open log file %pSE: %m", zcf->filename); - break; + return -1; } frr_with_privs (lib_privs) { @@ -957,11 +957,11 @@ static int zlog_5424_open(struct zlog_cfg_5424 *zcf, int sock_type) flog_err_sys(EC_LIB_SYSTEM_CALL, "could not open or create log file %pSE: %m", zcf->filename); - break; + return -1; case ZLOG_5424_DST_UNIX: if (!zcf->filename) - break; + return -1; memset(&sa, 0, sizeof(sa)); sa.sun_family = AF_UNIX; @@ -993,6 +993,7 @@ static int zlog_5424_open(struct zlog_cfg_5424 *zcf, int sock_type) "could not connect to log unix path %pSE: %m", zcf->filename); need_reconnect = true; + /* no return -1 here, trigger retry code below */ } else { /* datagram sockets are connectionless, restarting * the receiver may lose some packets but will resume From 592011b25160ed6bc476e6855784ed2a7c8f3bc6 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Wed, 20 Sep 2023 14:49:22 +0200 Subject: [PATCH 3/4] lib: clippy ELF: check existence of string table Mostly to make coverity happy, no compiler/linker should produce broken ELF files like this (and if it does we can't process it anyway...) Signed-off-by: David Lamparter --- lib/elf_py.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/elf_py.c b/lib/elf_py.c index 81ca668e70..643495d8c7 100644 --- a/lib/elf_py.c +++ b/lib/elf_py.c @@ -1089,7 +1089,9 @@ static void elffile_add_dynreloc(struct elffile *w, Elf_Data *reldata, symidx = relw->symidx = GELF_R_SYM(rela->r_info); sym = relw->sym = gelf_getsym(symdata, symidx, &relw->_sym); if (sym) { - relw->symname = elfdata_strptr(strdata, sym->st_name); + if (strdata) + relw->symname = elfdata_strptr(strdata, + sym->st_name); relw->symvalid = GELF_ST_TYPE(sym->st_info) != STT_NOTYPE; relw->unresolved = sym->st_shndx == SHN_UNDEF; From 448d690a354c3ea481aba6254a285937d843cf81 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Wed, 20 Sep 2023 15:27:23 +0200 Subject: [PATCH 4/4] lib: random make-coverity-happy nits Signed-off-by: David Lamparter --- lib/event.c | 7 ++++++- lib/ipaddr.h | 1 + lib/northbound.c | 4 ++-- lib/prefix.c | 2 +- lib/printf/printfcommon.h | 2 +- lib/sha256.c | 2 +- lib/vty.c | 7 ++++--- 7 files changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/event.c b/lib/event.c index 37a30d2511..458e29f248 100644 --- a/lib/event.c +++ b/lib/event.c @@ -791,7 +791,12 @@ static struct event *thread_get(struct event_loop *m, uint8_t type, thread->master = m; thread->arg = arg; thread->yield = EVENT_YIELD_TIME_SLOT; /* default */ - thread->ref = NULL; + /* thread->ref is zeroed either by XCALLOC above or by memset before + * being put on the "unuse" list by thread_add_unuse(). + * Setting it here again makes coverity complain about a missing + * lock :( + */ + /* thread->ref = NULL; */ thread->ignore_timer_late = false; /* diff --git a/lib/ipaddr.h b/lib/ipaddr.h index e3ad14d7db..c86e38c867 100644 --- a/lib/ipaddr.h +++ b/lib/ipaddr.h @@ -28,6 +28,7 @@ struct ipaddr { enum ipaddr_type_t ipa_type; union { uint8_t addr; + uint8_t addrbytes[16]; struct in_addr _v4_addr; struct in6_addr _v6_addr; } ip; diff --git a/lib/northbound.c b/lib/northbound.c index ef2344ee11..69b96d3656 100644 --- a/lib/northbound.c +++ b/lib/northbound.c @@ -2691,7 +2691,6 @@ void nb_init(struct event_loop *tm, size_t nmodules, bool db_enabled) { struct yang_module *loaded[nmodules], **loadedp = loaded; - bool explicit_compile; /* * Currently using this explicit compile feature in libyang2 leads to @@ -2699,8 +2698,9 @@ void nb_init(struct event_loop *tm, * of modules until they have all been loaded into the context. This * avoids multiple recompiles of the same modules as they are * imported/augmented etc. + * (Done as a #define to make coverity happy) */ - explicit_compile = false; +#define explicit_compile false nb_db_enabled = db_enabled; diff --git a/lib/prefix.c b/lib/prefix.c index 0b8664411d..f342c4c1db 100644 --- a/lib/prefix.c +++ b/lib/prefix.c @@ -261,7 +261,7 @@ int evpn_type5_prefix_match(const struct prefix *n, const struct prefix *p) return 0; prefixlen = evp->prefix.prefix_addr.ip_prefix_length; - np = &evp->prefix.prefix_addr.ip.ip.addr; + np = evp->prefix.prefix_addr.ip.ip.addrbytes; /* If n's prefix is longer than p's one return 0. */ if (prefixlen > p->prefixlen) diff --git a/lib/printf/printfcommon.h b/lib/printf/printfcommon.h index b3a7ca0c13..f777be8805 100644 --- a/lib/printf/printfcommon.h +++ b/lib/printf/printfcommon.h @@ -68,7 +68,7 @@ io_print(struct io_state *iop, const CHAR * __restrict ptr, size_t len) { size_t copylen = len; - if (!iop->cb) + if (!iop->cb || !len) return 0; if (iop->avail < copylen) copylen = iop->avail; diff --git a/lib/sha256.c b/lib/sha256.c index ccf260fa7b..08e08eb06b 100644 --- a/lib/sha256.c +++ b/lib/sha256.c @@ -344,7 +344,7 @@ void HMAC__SHA256_Final(unsigned char digest[32], HMAC_SHA256_CTX *ctx) void PBKDF2_SHA256(const uint8_t *passwd, size_t passwdlen, const uint8_t *salt, size_t saltlen, uint64_t c, uint8_t *buf, size_t dkLen) { - HMAC_SHA256_CTX PShctx, hctx; + HMAC_SHA256_CTX PShctx = {}, hctx; size_t i; uint8_t ivec[4]; uint8_t U[32]; diff --git a/lib/vty.c b/lib/vty.c index 23aa2d1f38..8f8effa9cc 100644 --- a/lib/vty.c +++ b/lib/vty.c @@ -2368,8 +2368,7 @@ static void vtysh_read(struct event *thread) printf("result: %d\n", ret); printf("vtysh node: %d\n", vty->node); #endif /* VTYSH_DEBUG */ - - if (vty->pass_fd != -1) { + if (vty->pass_fd >= 0) { memset(vty->pass_fd_status, 0, 4); vty->pass_fd_status[3] = ret; vty->status = VTY_PASSFD; @@ -2387,7 +2386,9 @@ static void vtysh_read(struct event *thread) * => skip vty_event(VTYSH_READ, vty)! */ return; - } + } else + /* normalize other invalid values */ + vty->pass_fd = -1; /* hack for asynchronous "write integrated" * - other commands in "buf" will be ditched